Skip to content
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

WIP: Working POC for buttons/buttons groups, PR up for commentary #3441

Closed

Conversation

johncosker
Copy link
Contributor

@johncosker johncosker commented Jun 2, 2023

Been working on buttons, this is the first working PoC. It allows wrapping a CycleHotKeyLabel in the new ButtonGroup class.

The GraphicButton and ButtonGroup classes will automatically load texpos graphics from sprite files and stitch them together to draw buttons. Some C++ is required to get the graphics loaded.

In the Lua, you need to pass in the name of the graphics (maps with the c++ code, which loads the texpos), and file_dims which is how many buttons there are and how many rows, so the following files are file_dims = { x = 7, y = 1 }, and file_dims = { x = 6, y = 1 }, respectively.
gui-design-modes
gui-design-shapes

It's also necessary to include the button_dims, which tell the classes what dimensions of texpos characters the buttons are, so for example the above icons are 4x3.

Eventually maybe we can come up with a way to automatically determine these things...

NOTE: There's work in progress to add more button states like hover, pressing, etc... so this will be expanded as work moves forward.

The ButtonGroup class will automatically keep the hotkey label and buttons in sync, and both controls will work. The hotkey_controller field is just the original widget, so it should be easy to upgrade existing controls and add graphics likes this.

Here's an example from gui/design

        widgets.ResizingPanel {
            subviews = {
                widgets.ButtonGroup {
                    view_id = "modes_button_group",
                    graphics = 'GuiDesignModes',
                    file_dims = { x = 7, y = 1 },
                    button_dims = { width = 4, height = 3 },
                    -- Original, unaltered hotkey label code
                    hotkey_controller = widgets.CycleHotkeyLabel {
                        view_id = "mode_name",
                        key = "CUSTOM_F",
                        key_back = "CUSTOM_SHIFT_F",
                        label = "Mode: ",
                        label_width = 8,
                        active = true,
                        enabled = true,
                        options = mode_options,
                        disabled = false,
                        show_tooltip = true,
                        on_change = function(new, old)
                            self.design_panel:updateLayout()
                        end,
                    },
                }
            }
        },
        widgets.ResizingPanel {
            subviews = {
                widgets.ButtonGroup {
                    orientation = 'vertical',
                    view_id = "shapes_button_group",
                    graphics = 'GuiDesignShapes',
                    file_dims = { x = 6, y = 1 },
                    button_dims = { width = 4, height = 3 },
                    -- Original, unaltered hotkey label code
                    hotkey_controller = widgets.CycleHotkeyLabel {
                        view_id = "shape_name",
                        key = "CUSTOM_Z",
                        key_back = "CUSTOM_SHIFT_Z",
                        label = "Shape: ",
                        label_width = 8,
                        active = true,
                        enabled = true,
                        options = options,
                        disabled = false,
                        show_tooltip = true,
                        on_change = self:callback("change_shape"),
                    },

                }
            }
        },

This code creates the following example:
graphics_buttons_demos

@johncosker johncosker marked this pull request as ready for review June 2, 2023 15:32
@johncosker
Copy link
Contributor Author

@myk002 idk if it's possible but it'd be cool if we could just pass in the png file path for the sprites in lua and have it dynamically load up the texpos values, so you don't have to hardcode in the stuff in the cpp files as seen in this change set, is that a thing we could theoretically do?

@myk002
Copy link
Member

myk002 commented Jun 4, 2023

Textures can be dynamically loaded, but their texpos values cannot be cached in Lua since the dynamic textures will be unloaded when the world unloads (or maybe it's when the next world loads). We'd have to write code to invalidate the texpos values stored in cpp as well.

@johncosker
Copy link
Contributor Author

the dynamic textures will be unloaded when the world unloads

This seems like a pretty infrequent occurrence, what would be needed to deal with it?

@myk002
Copy link
Member

myk002 commented Jun 6, 2023

So the background here is that the "texpos" value that the rendering system works with is an index into a DF array (std::vector). we can add textures to that array at any time and then use the index to render the tiles.

the lifecycle of the textures in that array is determined by the value of df.global.enabler.textures.init_texture_size. anything below that index is permanent. anything above that index will be wiped when a new world is loaded.

when DFHack starts, it add textures to the end of the vector and increments init_texture_size so our textures are permanent. however, if textures are added after a world is loaded, we can't adjust init_texture_size since our textures wouldn't be contiguous and in order to make our dynamically-added textures permanent, we'd have to make all of the loaded textures permanent. therefore, dynamically-added DFHack textures will have the same lifecycle as world textures.

This means that if a DFHack tool uses a dynamic texture, it must look up the texpos value every time it runs (which is not a terrible thing). The Textures module can likely handle dynamic reloading. It will definitely know when a world is unloaded (since we hook viewscreen changes and check for it) so it can invalidate textures properly.

@myk002
Copy link
Member

myk002 commented Jun 6, 2023

By the way, I am not attached to the current implementation of the Textures module. It was written quick and dirty to satisfy a need during the chaotic time of v50 migration. If you come up with a better replacement architecture/API, I will be nothing but pleased.

@myk002
Copy link
Member

myk002 commented Jun 6, 2023

more thoughts: "dynamic" is relative. we can create all the variations of a sprite sheet at load time and get them in at the same time as the other textures. this allows them to have the same lifecycle as all the other DFHack textures. this is "permanent" as long as DFHack itself is loaded before a world is loaded, which is currently always true, but may not always be the case, so it's still a good idea for scripts to look up the texpos values on each run.

anyway, there's no reason why we need to worry about non-contiguous texpos values like I was fearing above.

Copy link
Member

@myk002 myk002 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest that you take a look at TabBar and Tab to see if there are elements/strategies that you can adopt.

Comment on lines +2477 to +2489
self:addviews {
Label {
-- view_id = "button_but",
frame = { t = y, l = 0 },
text = tiles,
on_click = function()
if self.on_click_callback then self.on_click_callback({ group_index = self.group_index }) end
self.selected = not self.selected
self.tiles = nil
self:updateLayout()
end
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why multiple Labels instead of a single Label with multiple lines of tiles?

also note for later, once we get the tile variations in, that Label tokens now support a htile field for the tile to use when the Label is hovered. That will work much better if the entire button is a single Label

@@ -2433,4 +2433,131 @@ function RangeSlider:onRenderBody(dc, rect)
end
end

GraphicButton = defclass(GraphicButton, ResizingPanel)
GraphicButton.ATTRS {
tiles = DEFAULT_NIL,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this is not expected to be set by the parent, it shouldn't go into ATTRS. it can just be a regular self. field

Comment on lines +2439 to +2443
height = 0,
width = 0,
graphics = '',
graphic_file_dims = { x = 0, y = 0 },
pos = 0,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for parent-specified required values, it's better to default to DEFAULT_NIL and then validate in init

Comment on lines +2484 to +2486
self.selected = not self.selected
self.tiles = nil
self:updateLayout()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of re-initializing when the selected state changes, how about having two Labels with visible properties that are controlled by self.selected? that would allow all importing to happen in init, with display state handled by existing widget logic

function ButtonGroup:on_click(args)
if self.type == 'radio' then
for i, _ in ipairs(self.buttons) do
self.buttons[i].selected = false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the opposite choice that TabBar made, which keeps the selected state in the TabBar and the individual Tabs query the TabBar for "am I selected"?

I don't have a strong preference for which approach we take, but TabBar and ButtonGroup should be using the same architecture.

Actually, TabBar should be implemented as a radio-style ButtonGroup, and Tabs are just GraphicButtons with text also on them. Perhaps that's a good use case to make sure that your implementation here is generalized enough.

@johncosker
Copy link
Contributor Author

Thanks for the review, i'm hoping i'll be able to make more progress this weekend, nervous to start touching stuff like the tabs since i don't want to break everything, but i'll give it a shot

@myk002
Copy link
Member

myk002 commented Jun 13, 2023

nervous to start touching stuff like the tabs since i don't want to break everything, but i'll give it a shot

TabBar is only used by gui/control-panel and gui/launcher. if we make breaking changes, there won't be too much to update.

@johncosker
Copy link
Contributor Author

Good to know, still working on this just slowly

@shevernitskiy
Copy link
Contributor

shevernitskiy commented Jul 14, 2023

Textures can be dynamically loaded, but their texpos values cannot be cached in Lua since the dynamic textures will be unloaded when the world unloads (or maybe it's when the next world loads). We'd have to write code to invalidate the texpos values stored in cpp as well.

I made some crutch for similar case as a POC some time ago. During creating dynamic textures for chars (via sdl ttf) i needed to know, when i can use those created textures (cause as you said, game resetting caches), and when i needed to recreate new one. Well, i had made sort of game state tracker. Hooked into functions to render Main Menu, Loading New World and so. With help of that i can set callbacks to recreate textures.
Recently hooks was intoduced in libgraphics, maybe it wiil be reasonable to create hooks for such things like cache reset in a proper right places.

@myk002
Copy link
Member

myk002 commented May 19, 2024

I'm working on bringing this to life

vidcap-2024-05-18_17.54.13.mp4

@myk002 myk002 mentioned this pull request May 19, 2024
@myk002 myk002 closed this in #4615 May 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants