-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[NO SQUASH] Create new UI API (Formspec/HUD replacement) #14263
base: master
Are you sure you want to change the base?
Conversation
7968d0b
to
e3a4f26
Compare
Sizers were easier than I'd hoped, so I added them in. There's a flex and grid sizer, bearing no little resemblance to CSS grid and flexbox, albeit somewhat simplified. The documentation also has pretty ASCII images to document how layout works in more detail. There's also a few other minor changes based on preliminary feedback. Codeui.set_default_theme(ui.Style{
ui.Style{
sel = "#root",
},
ui.Style{
sel = "#base",
rel_pos = {0.5, 0.5},
rel_anchor = {0.5, 0.5},
rel_size = {0, 0},
bg_fill = "black#8C",
},
})
local function flex_builder(id, player, cx)
return ui.Window{
type = "gui",
style = ui.Style{
noclip = true,
ui.Style{
sel = ".green",
size = {100, 100},
bg_fill = "green#8C",
},
ui.Style{
sel = ".blue",
size = {150, 100},
bg_fill = "blue#8C",
},
ui.Style{
sel = ".purple",
size = {100, 120},
bg_fill = "purple#8C",
},
ui.Style{
sel = ".red",
size = {100, 100},
bg_fill = "red#8C",
weight = 1,
},
ui.Style{
sel = ".cyan",
size = {100, 100},
bg_fill = "cyan#8C",
weight = 2,
},
ui.Style{
sel = ".white",
size = {700, 100},
bg_fill = "white#8C",
},
ui.Style{
sel = ".black",
size = {100, 300},
bg_fill = "black#8C",
},
ui.Style{
sel = ".sneaky",
size = {0, 0},
},
},
root = ui.Elem{
id = "root",
ui.Flex{
id = "base",
dir = "right",
wrap = "forward",
--hspacing = "around",
--vspacing = "remove",
rel_size = {0.5, 0.9},
padding = {5, 5, 5, 5},
gap = {5, 10},
ui.Elem{groups = {"green"}},
ui.Elem{groups = {"red"}},
ui.Elem{groups = {"sneaky"}},
ui.Elem{groups = {"cyan"}},
ui.Elem{groups = {"purple"}},
ui.Elem{groups = {"white"}},
ui.Elem{groups = {"green"}},
ui.Elem{groups = {"red"}},
ui.Elem{groups = {"blue"}},
ui.Elem{groups = {"red"}},
ui.Elem{groups = {"cyan"}},
ui.Elem{groups = {"black"}},
ui.Elem{groups = {"purple"}},
--[[
id = "base",
dir = "down",
wrap = "forward",
hspacing = "remove",
vspacing = "evenly",
rel_size = {0.5, 0.9},
padding = {5, 5, 5, 5},
gap = {10, 5},
ui.Elem{groups = {"black"}},
ui.Elem{groups = {"green"}},
ui.Elem{groups = {"blue"}},
ui.Elem{groups = {"blue"}},
ui.Elem{groups = {"purple"}},
]]
},
},
}
end
local function grid_builder(id, player, cx)
return ui.Window{
type = "gui",
style = ui.Style{
noclip = true,
ui.Style{
sel = ".green",
size = {100, 100},
bg_fill = "green#8C",
},
ui.Style{
sel = ".blue",
size = {150, 100},
bg_fill = "blue#8C",
},
ui.Style{
sel = ".purple",
size = {100, 120},
bg_fill = "purple#8C",
},
ui.Style{
sel = ".white",
size = {600, 100},
bg_fill = "white#8C",
},
ui.Style{
sel = ".black",
size = {100, 300},
bg_fill = "black#8C",
},
ui.Style{
sel = ".sneaky",
size = {0, 0},
},
},
root = ui.Elem{
id = "root",
ui.Grid{
id = "base",
hsizes = {100, 0, 150},
vsizes = {100, 120},
hweights = {0, 1, 0},
--vweights = {1, 5},
--hspacing = "around",
vspacing = "remove",
rel_size = {0.5, 0.9},
padding = {5, 5, 5, 5},
gap = {5, 10},
ui.Elem{groups = {"white"}, pos = {0, 0}, span = {3, 1}},
ui.Elem{groups = {"green"}, pos = {1, 1}, span = {1, 1}},
ui.Elem{groups = {"purple"}, pos = {0, 1}, span = {1, 1}},
ui.Elem{groups = {"blue"}, pos = {2, 1}, span = {1, 1}},
ui.Elem{groups = {"black"}, pos = {1, 2}, span = {1, 1}},
ui.Elem{groups = {"sneaky"}, pos = {0, 0}, span = {3, 3}},
},
},
}
end
core.register_on_joinplayer(function(player)
local id = ui.open(flex_builder, player:get_player_name())
end)
|
babe696
to
b2f28c9
Compare
I beefed up style selectors in power, so they have things similar to child/parent selectors and stuff now. In some respects, they have a few features that are more convenient than CSS itself, such as the ability to group things like As usual, here's some example code and a screenshot of the output (nothing amazing to look at, but shows the expected output): Codeui.set_default_theme(ui.Style{
ui.Style{
sel = "?root",
},
ui.Style{
sel = "?<(?root)",
rel_pos = {0.5, 0.5},
rel_anchor = {0.5, 0.5},
rel_size = {0, 0},
bg_fill = "black#8C",
},
})
local function style_builder(id, player, cx)
return ui.Window{
type = "gui",
style = ui.Style{
ui.Style{
sel = "?<(flex)",
size = {100, 100},
bg_fill = "green#8C",
},
ui.Style{
sel = "flex",
fg_image = "cdb_queued.png",
fg_scale = 1,
ui.Style{
sel = "?horizontal",
fg_valign = "bottom",
},
ui.Style{
sel = "?vertical",
fg_halign = "right",
},
},
ui.Style{
sel = ".blue?<(flex)",
bg_fill = "blue#8C",
},
ui.Style{
sel = "flex!(/hud/, elem, #that)",
bg_fill = "white#8C",
},
ui.Style{
sel = "?nth_child(3)?nth_last_child(6)",
bg_fill = "purple#8C",
},
ui.Style{
sel = "flex?>(.blue)",
rel_anchor = {0.4, 0.4},
},
ui.Style{
sel = "?root?>>(.blue)",
fg_image = "air.png",
fg_scale = 1,
},
ui.Style{
sel = ".what?<<(#bob)",
bg_fill = "red#8C",
},
ui.Style{
sel = ".eh?<>(.what)",
bg_fill = "black#8C",
},
ui.Style{
sel = ".maybe_empty?empty",
bg_fill = "yellow#8C",
},
ui.Style{
sel = "(?root, ?<(.maybe_empty))?only_child",
fg_fill = "blue#3",
fg_halign = "right",
},
ui.Style{
sel = "(?first_child, ?last_child)?<(flex)",
fg_image = "cdb_add.png",
},
},
root = ui.Elem{
id = "bob",
groups = {"blue"},
ui.Flex{
id = "base",
dir = "down",
wrap = "forward",
rel_size = {0.5, 0.5},
padding = {5, 5, 5, 5},
gap = {5, 5},
ui.Elem{},
ui.Elem{groups = {"blue"}},
ui.Elem{},
ui.Elem{groups = {"what", "eh"}},
ui.Elem{groups = {"eh"}},
ui.Elem{groups = {"maybe_empty"}},
ui.Elem{
groups = {"maybe_empty"},
ui.Elem{
fg_image = "cdb_update.png",
fg_scale = 1,
},
},
ui.Elem{},
}
},
}
end
core.register_on_joinplayer(function(player)
local id = ui.open(style_builder, player:get_player_name())
core.after(2, function() ui.update(id) end)
end) |
6e94043
to
40c1f65
Compare
This looks like excellent work, from my reading of the conversation. I'm new to both Luanti and C++ but I have a ton of experience at Microsoft with UX design. Not much to say yet, but just want to congratulate and thank @v-rob and the team for their persistent efforts here, this new UI API will unlock a LOT of technological doors for Luanti game designers ❤️ Thanks everyone, merry Christmas, and I look forward to contributing to this PR however and whenever I can :) |
Ok, seems like everyone disagrees with me on clien't side. I still want texturpack support though. It's important for texturepacks to be able to provide a consistent look. |
@mruncreative I appreciate your concern, but the focus of this PR is a bit different. Maybe you can find or file a better issue for "texture packs + formspecs" specifically to focus the discussion? |
@v-rob reminder |
``` | ||
|
||
Lastly, state precedences combine. A style for pressed and hovered buttons will | ||
override styles for only pressed or only hovered buttons. The highest state |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So basically lexicographical ordering based on state precedences?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically, yes, where higher precedence means listed later in lexicographical ordering.
Very Minor Nitpick I see ipairs used extensively. In general, since this is falling to the main-thread lua, we should aggressively optimize our limited compute here. A small, easy optimization is to instead use #indexing, typically a 10-15% perf boost for iterations for minimal effort: Demonstration lua script#!/usr/bin/env luajit
--[[
Adapted from https://gist.github.com/dpino/647b9d2e12aa8ff859b4
Environment: Lenovo Legion / 16GB RAM / i7-9750H CPU @ 2.60GHz
| pairs | ipairs | indexing |
pairs | 1.000000 | 3.007634 | 3.549550 |
ipairs | 0.332487 | 1.000000 | 1.180180 |
indexing | 0.281726 | 0.847328 | 1.000000 |
]]--
local ITERATIONS = 1000000
local x
local function array(n)
local result = {}
for i=1, n do
table.insert(result, tostring(i))
end
return result
end
local function execute_times(f, times)
local a = array(100)
local begin = os.clock()
for i=1,times do
f(a)
end
local finish = os.clock()
return finish - begin
end
local times = {}
-- Pairs
local elapse_time1 = execute_times(function(a)
for j,v in pairs(a) do
x=v
end
end, ITERATIONS)
times["pairs"] = elapse_time1
-- iPairs
local elapse_time2 = execute_times(function(a)
for _,v in ipairs(a) do
x=v
end
end, ITERATIONS)
times["ipairs"] = elapse_time2
-- Indexing
local elapse_time3 = execute_times(function(a)
local len = #a
for j=1,len do
x=a[j]
end
end, ITERATIONS)
times["indexing"] = elapse_time3
local results = {
{ times["pairs"]/times["pairs"], times["pairs"]/times["ipairs"], times["pairs"]/times["indexing"] },
{ times["ipairs"]/times["pairs"], times["ipairs"]/times["ipairs"], times["ipairs"]/times["indexing"] },
{ times["indexing"]/times["pairs"], times["indexing"]/times["ipairs"], times["indexing"]/times["indexing"] },
}
print("| pairs | ipairs | indexing |")
for _, row in ipairs(results) do
io.write(string.format("| %.6f ", row[1]))
io.write(string.format("| %.6f |", row[2]))
io.write(string.format(" %.6f |", row[3]))
io.write("\n")
end |
@@ -36,6 +36,7 @@ set(BUILD_SERVER FALSE CACHE BOOL "Build server") | |||
set(BUILD_UNITTESTS TRUE CACHE BOOL "Build unittests") | |||
set(BUILD_BENCHMARKS FALSE CACHE BOOL "Build benchmarks") | |||
set(BUILD_DOCUMENTATION TRUE CACHE BOOL "Build documentation") | |||
set(BUILD_UI FALSE CACHE BOOL "Build experimental UI API") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please specify that Build Client
and USE_SDL2
are required here, rather than just fatal erroring when they try it.
return rect[1], rect[2], rect[1], rect[2] | ||
elseif #rect == 1 then | ||
return rect[1], rect[1], rect[1], rect[1] | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might want to make #rect a local len
variable, rather than calling #rect 3 times.
return default_theme | ||
end | ||
|
||
function ui.set_default_theme(theme) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error check here to ensure theme is of type ui.Style {}
Also does this use table.copy or direct table pass through?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style objects are immutable, so no need to copy.
There's a lot of places I could do type assertions, but I currently don't. Arguably that's not the best, and I could add it everywhere.
@@ -14,6 +14,7 @@ General options and their default values: | |||
BUILD_UNITTESTS=TRUE - Build unittest sources | |||
BUILD_BENCHMARKS=FALSE - Build benchmark sources | |||
BUILD_DOCUMENTATION=TRUE - Build doxygen documentation | |||
BUILD_UI=TRUE - Build experimental UI API |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mention Requiring SDL2 and BUILD_CLIENT
end) | ||
``` | ||
|
||
Note that, since the UI API has no theme bundled by default at this point, a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have the prelude theme bundled at this point, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically, yeah. Here, I meant there's no theme that gives default visuals.
checkbox, which has a dynamic `main` box, the checkbox will have the `hovered` | ||
state whereas the parent button will not. However, if the mouse hovers over the | ||
label, which has a static `main` box, the event will pass through the label and | ||
the parent button will have the `hovered` state instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You cover it later, but at this point in the readme, it's not clear that a touch/click event will propagate downward to each underneath dynamic box until handled. After reading this section, it sounds like they only hit the top-most dynamic box.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They do only hit the topmost dynamic box, which is the intended behavior. Does it say somewhere that it hits all of them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When mouse or touch input is received by the window, it first allows the
focused element and its parents to peek at the event, such as to let buttons
become unpressed. Then, it sends the input to the hovered element. If it
ignores the input, the input passes to the element directly below it (which is
not necessarily its parent element), and so on. If none of them used the input,
the window again gets to use the input, possibly sending it to the server.
Here you imply that, if an event goes unhandled by that topmost dynamic box, it will be distributed downward. Or is this only the case for hover? I would expect it to also apply in the case of clicks
pressed, the window sets the focused element to the topmost focusable element | ||
under the cursor. Lastly, if the mouse is double clicked outside of any box | ||
except the backdrop of the root element, then the window will be closed if it | ||
is not uncloseable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make double click/double touch a disable-able option/ I mean, I guess we could put an invisible box there, but seems like a hack
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would you want to disable it? Formspecs always do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just because it's available for formspecs to be always there, in this rewrite I would hope that would be dehardcoded. Had my fair share of mistaken double clicks or double taps, personally. I'm sure others have too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it should be an option, since that's just going to end up with inconsistencies between mods. I'd prefer to either require the behavior or do away with it entirely. Either way is fine for me.
The tab key is used for changing which element has user focus via the keyboard. | ||
Pressing tab will cause the next focusable element in the element tree to | ||
become focused, whereas pressing shift+tab will transfer focus to the previous | ||
focusable element. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Next adjacent element? or next child element?
What if I'm on layer 3, and the next element is at layer 2?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be precise, repeatedly hitting tab will do a preorder traversal of focusable elements in the element tree. I will clarify that.
no other purpose than being a base for other themes. The prelude theme sets | ||
style properties that are essential to the functionality of the element, or are | ||
at least a basic part of the element's intended design. It can be accessed with | ||
the `ui.get_prelude_theme()` function. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Core theme vs prelude theme, will they be one in the same or no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are separate. The prelude is designed to be the base for every theme (including the core theme), and doesn't give any visuals. Imagine the prelude theme as a web browser's default stylesheet after a CSS reset has been used--things like <head>
tags are still hidden, but other things like buttons don't have a distinct look from any other element.
on_scroll = function(ev) | ||
state.scroll = ev.value | ||
context:update() | ||
end, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, scrolling is server side? or will it be both?
I'm thinking that both would allow lazy loading, I guess, can't think of many other use cases yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scrolling will be client-side; I'm just storing the scroll value on the server for the sole purpose of displaying a label.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or will it be both?
but the server does have an on_scroll callback, then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will when scrollbars exist :) They don't yet, as mentioned in the paragraph above the example.
will automatically generate IDs when none is required. | ||
* The format of this ID is not precisely specified, but it will have the | ||
format of an engine reserved ID and will not conflict with any other IDs | ||
generated during this session. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ui.new_id() gives a number of 0->large. What If I also give an id that is just a number "1"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it gives an ID of _0
, which is an engine-reserved ID. Mods are not allowed to use IDs with leading underscores or dashes.
Defaults to the theme provided by `ui.get_default_theme()`. | ||
* `style` (`ui.Style`): Specifies a style to apply across the entire element | ||
tree. Defaults to an empty style. | ||
* `uncloseable` (boolean): Indicates whether the user is able to close the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love that this is an option, what if there's a bug and a modder/game maker gives them no buttons to close with? How do they exit to the main menu?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the future, ESC should likely open the pause menu, but for now you'd have to Alt+F4.
A key feature I expected from unifying HUD and formspecs into this new UI interface was control of when the mouse is captured vs not, and keyboard is captured vs not. In my head, I expected the ability to create "hud" or "gui" windows/contexts where I specify if that window/context captures the mouse, or keyboard explicitly. For example, say I want to leave the keyboard for walking/interacting with the game (not the window), but have the mouse available for button/clicking? Right now, that keyboard event could only be passed through if I do so on the server and replicate the action on the client, which will definitely have lag and be an issue. Some example things we can do by having these two captures (keyboard, mouse, heck individual keys) separated and controllable:
Going one step further, allow passthrough to other contexts/not "gui" window contexts when the event goes unhandled. That would give us a TON more flexibility. Of course, all this means a modder/game designer needs to consider mobile vs. non-mobile users. Which adds complexity but is doable as of today. |
This sounds like a premature optimization; I doubt
Capturing keyboard buttons in the UI layer as a means of implementing custom keybindings is a hack, not really a good example. I don't understand the use for capturing the mouse but leaving the keyboard controls in the third example--do you just mean WASD for moving around? The only really useful example I've thought of for this is pressing buttons on the HUD with a touchscreen, e.g. for custom controls, while still allowing interaction with the game itself. That is a planned feature in the future, although I was planning on just adding a new window type for that express purpose. |
Absolutely, more than that, but at least that, yes. movement while selecting things, walking with a minimap overlay I can click on, RTS overlay buttons , etc.
Oh then no worries, I would just put that feature into base "hub" and "gui" contexts, but sure another breakout if needed, sure.
This comment and the others -> I think you are thinking too traditional MT games specific, and I'm thinking more along the lines of games, where we want more complete control in the game designers hands That said: this is your baby, you've done 99% of the work. Let's not bike shed flexibility features in what is an initial, experimental UI. I can make the case for the proposal via my own PR rather than asking for it here. |
So, I finished adding proper support for labels two weeks ago (all the basic stuff, with font, size, bold, color, etc.). I was also considering As a proposal, I think something akin to CSS ui.Style "#ex" {
before = {
box_image = "cancel.png",
-- Probably introduce a property that factors the size of the
-- background image into the minimum size of the box.
},
after = {
text = "Example text",
-- Labels would become a style property, not an element property.
},
}
ui.Button "ex" {
ui.Elem "a" {},
ui.Elem "b" {},
} The
Writing out that kind of styling would be extremely verbose. Adding support for custom attributes could solve that problem: ui.Style "button" {
before = {
box_image = ui.attr "icon",
},
after = {
text = ui.attr "label",
},
}
ui.Button "ex" {
icon = "cancel.png",
label = "Example text",
} Custom attributes need not be difficult to implement, and honestly could remove the need for groups and group selectors entirely. I do like how much more general and extensible this solution would be, particularly since it can take advantage of the powerful layout sizers. Nevertheless, there's a fair amount of complexity involved, so I'm not entirely sold on the idea. |
under the cursor. Lastly, if the mouse is double clicked outside of any box | ||
except the backdrop of the root element, then the window will be closed if it | ||
is not uncloseable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This behavior is not necessarily needed on desktop since you have ESC there, but on mobile it's often the most convenient way to exit a formsec (in reply to ExeVirus)
Formspecs have since a few versions changed from "double-tap outside to exit" to "single tap outside to exit" because that's a common convention for mobile UIs and makes it more discoverable, so the new UI API should do the same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will remove the double click to exit functionality for desktop then.
Discussion from Matrix:
|
Closes #6527. Continued from #12926, but reworked stuff like serialization, added full styling support, and made a Lua API for the UI.
This PR is
officially Ready for Reviewundergoing some changes right now.DO NOT SQUASH. The code is already in nice logical commits.