build!: add amalgamation system #44

Merged
abart27 merged 15 commits from amalgamation into main 2026-02-28 09:34:49 +00:00
abart27 commented 2026-02-22 22:29:29 +00:00 (Migrated from github.com)

Summary

Splits BreitbandGraphics, ugui, and ugui-ext into multiple scripts which are unified into breitbandgraphics-amalgamated and ugui-amalgamated using a new amalgamation system.

Breaking Changes

  • The built library filenames have changed.

    mupen-lua-ugui.lua -> ugui-amalgamated.lua
    mupen-lua-ugui-ext.lua -> ugui-amalgamated.lua
    breitbandgraphics.lua -> breitbandgraphics-amalgamated.lua

    -BreitbandGraphics = dofile("breitbandgraphics.lua")
    -ugui = dofile("mupen-lua-ugui.lua")
    -ugui_ext = dofile("mupen-lua-ugui-ext.lua")
    
    +BreitbandGraphics = dofile("breitbandgraphics-amalgamated.lua")
    +ugui = dofile("ugui-amalgamated.lua")
    
  • mupen-lua-ugui-ext.lua doesn't exist anymore - it's now integrated in ugui.

    Consequently, all other members of ugui_ext (e.g. ugui_ext.internal.rectangle_to_key) have now been moved into ugui. If you are overriding or otherwise hacking around with these, make sure to adjust that code as well.

    -ugui_ext.apply_nineslice()
    +ugui.apply_nineslice()
    
  • the global math shims were removed due to them being non-standard and now part of Mupen64.

    If you are seeing errors like attempt to call a nil value (field 'pow'), you should update Mupen64.

## Summary Splits BreitbandGraphics, ugui, and ugui-ext into multiple scripts which are unified into `breitbandgraphics-amalgamated` and `ugui-amalgamated` using a new amalgamation system. ## Breaking Changes - The built library filenames have changed. `mupen-lua-ugui.lua` -> `ugui-amalgamated.lua` `mupen-lua-ugui-ext.lua` -> `ugui-amalgamated.lua` `breitbandgraphics.lua` -> `breitbandgraphics-amalgamated.lua` ```diff -BreitbandGraphics = dofile("breitbandgraphics.lua") -ugui = dofile("mupen-lua-ugui.lua") -ugui_ext = dofile("mupen-lua-ugui-ext.lua") +BreitbandGraphics = dofile("breitbandgraphics-amalgamated.lua") +ugui = dofile("ugui-amalgamated.lua") ``` - `mupen-lua-ugui-ext.lua` doesn't exist anymore - it's now integrated in `ugui`. Consequently, all other members of `ugui_ext` (e.g. `ugui_ext.internal.rectangle_to_key`) have now been moved into `ugui`. If you are overriding or otherwise hacking around with these, make sure to adjust that code as well. ```diff -ugui_ext.apply_nineslice() +ugui.apply_nineslice() ``` - the global `math` shims were removed due to them being non-standard and now part of Mupen64. If you are seeing errors like `attempt to call a nil value (field 'pow')`, you should update Mupen64.
copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed 2026-02-22 22:49:27 +00:00
copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull request overview

This PR introduces an amalgamation build system that splits BreitbandGraphics and ugui into multiple source files which are combined during build. The system uses a Python script to merge files with proper headers.

Changes:

  • Splits monolithic Lua files into modular source files under src/ directory
  • Adds build.py to amalgamate source files into build/ directory
  • Updates test runner and demos to use amalgamated files
  • Removes mupen-lua-ugui-ext.lua - now returned as second value from ugui-amalgamated.lua

Reviewed changes

Copilot reviewed 30 out of 50 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
build.py Python script that amalgamates source files with copyright headers
src/BreitbandGraphics/* Split BreitbandGraphics into types, core, internal, backends, and epilogue
src/ugui/* Split ugui into types, core, internal, styler, controls, ugui-ext, and epilogue
test/* New test files for various UI controls
test/runner.lua Updated to load from amalgamated build files
demos/* Updated to load from amalgamated build files
breitbandgraphics.lua Deleted (replaced by amalgamation)
.gitignore Added build/ directory

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

## Pull request overview This PR introduces an amalgamation build system that splits BreitbandGraphics and ugui into multiple source files which are combined during build. The system uses a Python script to merge files with proper headers. **Changes:** - Splits monolithic Lua files into modular source files under `src/` directory - Adds `build.py` to amalgamate source files into `build/` directory - Updates test runner and demos to use amalgamated files - Removes `mupen-lua-ugui-ext.lua` - now returned as second value from ugui-amalgamated.lua ### Reviewed changes Copilot reviewed 30 out of 50 changed files in this pull request and generated 3 comments. <details> <summary>Show a summary per file</summary> | File | Description | | ---- | ----------- | | build.py | Python script that amalgamates source files with copyright headers | | src/BreitbandGraphics/* | Split BreitbandGraphics into types, core, internal, backends, and epilogue | | src/ugui/* | Split ugui into types, core, internal, styler, controls, ugui-ext, and epilogue | | test/* | New test files for various UI controls | | test/runner.lua | Updated to load from amalgamated build files | | demos/* | Updated to load from amalgamated build files | | breitbandgraphics.lua | Deleted (replaced by amalgamation) | | .gitignore | Added build/ directory | </details> --- 💡 <a href="/mupen64/ugui/new/main/.github/instructions?filename=*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.
@ -0,0 +39,4 @@
return {
primary = selected_index,
meta = {
{ signal_change = data.signal_change },
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-22 22:49:26 +00:00

The meta table is incorrectly wrapped in an extra table. It should be signal_change = data.signal_change directly, not { signal_change = data.signal_change }. This will cause the meta field to be a table containing a table instead of the expected Meta structure.

                signal_change = data.signal_change,
The meta table is incorrectly wrapped in an extra table. It should be `signal_change = data.signal_change` directly, not `{ signal_change = data.signal_change }`. This will cause the meta field to be a table containing a table instead of the expected Meta structure. ```suggestion signal_change = data.signal_change, ```
@ -0,0 +103,4 @@
data.signal_change = ugui.internal.process_signal_changes(data.signal_change, control.value ~= data.value)
return {
value = data.value,
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-22 22:49:27 +00:00

The numberbox logic returns an object with field value instead of primary. This is inconsistent with all other controls which return { primary = ..., meta = ... }. The calling code expects result.primary, which will be nil here.

            primary = data.value,
The numberbox logic returns an object with field `value` instead of `primary`. This is inconsistent with all other controls which return `{ primary = ..., meta = ... }`. The calling code expects result.primary, which will be nil here. ```suggestion primary = data.value, ```
@ -0,0 +55,4 @@
is_checked = selected_index == i,
})
if not previous == new then
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-22 22:49:27 +00:00

The comparison logic is inverted. The condition not previous == new should be previous ~= new. The current logic treats the boolean result of previous == new as a table/value to negate, which will likely always evaluate to false (since any value negated is false), causing the selected_index to always be updated.

        if previous ~= new then
The comparison logic is inverted. The condition `not previous == new` should be `previous ~= new`. The current logic treats the boolean result of `previous == new` as a table/value to negate, which will likely always evaluate to false (since any value negated is false), causing the selected_index to always be updated. ```suggestion if previous ~= new then ```
copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed 2026-02-27 16:54:38 +00:00
copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull request overview

Copilot reviewed 31 out of 51 changed files in this pull request and generated 14 comments.

Comments suppressed due to low confidence (1)

src/ugui/ugui-ext.lua:59

  • ugui_ext.internal.cached_draw (and ugui_ext.free) are only defined when d2d.draw_to_image exists. If d2d.draw_to_image is absent but d2d.create_render_target is present, the fallback branch (if not d2d.create_render_target and not d2d.draw_to_image) won't run either, leaving cached_draw/free nil and causing runtime errors when apply_nineslice calls them. Either reintroduce the create_render_target implementation, or make the fallback trigger whenever draw_to_image is unavailable so cached_draw/free are always defined.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

## Pull request overview Copilot reviewed 31 out of 51 changed files in this pull request and generated 14 comments. <details> <summary>Comments suppressed due to low confidence (1)</summary> **src/ugui/ugui-ext.lua:59** * `ugui_ext.internal.cached_draw` (and `ugui_ext.free`) are only defined when `d2d.draw_to_image` exists. If `d2d.draw_to_image` is absent but `d2d.create_render_target` is present, the fallback branch (`if not d2d.create_render_target and not d2d.draw_to_image`) won't run either, leaving `cached_draw`/`free` nil and causing runtime errors when `apply_nineslice` calls them. Either reintroduce the `create_render_target` implementation, or make the fallback trigger whenever `draw_to_image` is unavailable so `cached_draw`/`free` are always defined. </details> --- 💡 <a href="/mupen64/ugui/new/main?filename=.github/instructions/*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.
@ -0,0 +1,443 @@
--
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-27 16:54:38 +00:00

BreitbandGraphics.draw_image calls color_source_to_float_color(color) but that function isn't in scope here (the implementation lives at BreitbandGraphics.internal.color_source_to_float_color). Also the else branch assigns BreitbandGraphics.colors.white (a Color) to float_color and later uses float_color.a, which will be nil. Use BreitbandGraphics.internal.color_source_to_float_color for both branches (or otherwise ensure float_color is a proper {r,g,b,a} float table).

        float_color = BreitbandGraphics.internal.color_source_to_float_color(color)
    else
        float_color = BreitbandGraphics.internal.color_source_to_float_color(BreitbandGraphics.colors.white)
`BreitbandGraphics.draw_image` calls `color_source_to_float_color(color)` but that function isn't in scope here (the implementation lives at `BreitbandGraphics.internal.color_source_to_float_color`). Also the `else` branch assigns `BreitbandGraphics.colors.white` (a Color) to `float_color` and later uses `float_color.a`, which will be nil. Use `BreitbandGraphics.internal.color_source_to_float_color` for both branches (or otherwise ensure `float_color` is a proper `{r,g,b,a}` float table). ```suggestion float_color = BreitbandGraphics.internal.color_source_to_float_color(color) else float_color = BreitbandGraphics.internal.color_source_to_float_color(BreitbandGraphics.colors.white) ```
@ -0,0 +30,4 @@
r = tonumber(hex:sub(2, 3), 16),
g = tonumber(hex:sub(4, 5), 16),
b = tonumber(hex:sub(6, 7), 16),
}
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-27 16:54:37 +00:00

hex_to_color currently uses return on its own line followed by a table on the next line. In Lua this returns nil (the table is treated as a separate statement), so both branches will return nil instead of a color table. Put the table literal on the same line as return (or wrap in parentheses) so the function actually returns the constructed color.

`hex_to_color` currently uses `return` on its own line followed by a table on the next line. In Lua this returns `nil` (the table is treated as a separate statement), so both branches will return `nil` instead of a color table. Put the table literal on the same line as `return` (or wrap in parentheses) so the function actually returns the constructed color.
@ -0,0 +42,4 @@
r = value,
g = value,
b = value,
}
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-27 16:54:37 +00:00

repeated_to_color has the same Lua issue as hex_to_color: return is on its own line, so the function returns nil instead of the table. Keep the returned table on the same line as return so callers receive a valid {r,g,b} color.

`repeated_to_color` has the same Lua issue as `hex_to_color`: `return` is on its own line, so the function returns `nil` instead of the table. Keep the returned table on the same line as `return` so callers receive a valid `{r,g,b}` color.
@ -0,0 +36,4 @@
r = (source >> 24) & 0xFF,
g = (source >> 16) & 0xFF,
b = (source >> 8) & 0xFF,
a = source & 0xFF,
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-27 16:54:36 +00:00

color_source_to_float_color's RawColor branch returns channel values in the 0–255 integer range, but the function claims to return a FloatColor (0.0–1.0) and its callers pass the result directly to APIs like d2d.create_brush which expect floats. Convert these extracted bytes to floats (divide by 255.0) to avoid incorrect colors/alpha.

                r = ((source >> 24) & 0xFF) / 255.0,
                g = ((source >> 16) & 0xFF) / 255.0,
                b = ((source >> 8) & 0xFF) / 255.0,
                a = (source & 0xFF) / 255.0,
`color_source_to_float_color`'s RawColor branch returns channel values in the 0–255 integer range, but the function claims to return a FloatColor (0.0–1.0) and its callers pass the result directly to APIs like `d2d.create_brush` which expect floats. Convert these extracted bytes to floats (divide by 255.0) to avoid incorrect colors/alpha. ```suggestion r = ((source >> 24) & 0xFF) / 255.0, g = ((source >> 16) & 0xFF) / 255.0, b = ((source >> 8) & 0xFF) / 255.0, a = (source & 0xFF) / 255.0, ```
@ -0,0 +39,4 @@
return {
primary = selected_index,
meta = {
{ signal_change = data.signal_change },
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-27 16:54:38 +00:00

carrousel_button's registry logic returns meta = { { signal_change = ... } } (an array containing a table) instead of a plain Meta table. This makes result.meta.signal_change nil for callers. Return meta = { signal_change = data.signal_change } to match the ControlReturnValue/Meta contract used by other controls.

                signal_change = data.signal_change,
`carrousel_button`'s registry `logic` returns `meta = { { signal_change = ... } }` (an array containing a table) instead of a plain `Meta` table. This makes `result.meta.signal_change` nil for callers. Return `meta = { signal_change = data.signal_change }` to match the `ControlReturnValue`/`Meta` contract used by other controls. ```suggestion signal_change = data.signal_change, ```
@ -0,0 +10,4 @@
validate = function(control)
ugui.internal.assert(type(control.items) == 'table', 'expected items to be table')
ugui.internal.assert(type(control.selected_index) == 'number', 'expected selected_index to be number')
end,
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-27 16:54:38 +00:00

ugui.registry.combobox.validate requires selected_index to be a number, but the public type (ComboBox.selected_index) is documented as optional/nil-able and the styler handles nil as "no selection". Allow nil here (or update the type/docs) so callers can create an unselected combobox without validation errors.

`ugui.registry.combobox.validate` requires `selected_index` to be a number, but the public type (`ComboBox.selected_index`) is documented as optional/nil-able and the styler handles `nil` as "no selection". Allow `nil` here (or update the type/docs) so callers can create an unselected combobox without validation errors.
@ -0,0 +150,4 @@
end
return result, result.meta
end
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-27 16:54:37 +00:00

ugui.menu treats the return value of ugui.control(control, 'menu') as if it were a MenuResult (result.item, result.dismissed), but ugui.control returns a ControlReturnValue wrapper { primary = MenuResult, meta = ... }. This breaks both submenu propagation and the public return type (callers will receive the wrapper table). Mirror the other controls: work with result.primary and return result.primary, result.meta.

`ugui.menu` treats the return value of `ugui.control(control, 'menu')` as if it were a `MenuResult` (`result.item`, `result.dismissed`), but `ugui.control` returns a `ControlReturnValue` wrapper `{ primary = MenuResult, meta = ... }`. This breaks both submenu propagation and the public return type (callers will receive the wrapper table). Mirror the other controls: work with `result.primary` and return `result.primary, result.meta`.
@ -0,0 +103,4 @@
data.signal_change = ugui.internal.process_signal_changes(data.signal_change, control.value ~= data.value)
return {
value = data.value,
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-27 16:54:36 +00:00

ugui.registry.numberbox.logic returns { value = data.value, meta = ... }, but ugui.control and all other registry entries use { primary = ..., meta = ... }. Returning value means callers will see result.primary == nil and breaks the control contract. Rename the field to primary (and keep meta as-is) so ugui.control wrappers behave consistently.

            primary = data.value,
`ugui.registry.numberbox.logic` returns `{ value = data.value, meta = ... }`, but `ugui.control` and all other registry entries use `{ primary = ..., meta = ... }`. Returning `value` means callers will see `result.primary == nil` and breaks the control contract. Rename the field to `primary` (and keep `meta` as-is) so `ugui.control` wrappers behave consistently. ```suggestion primary = data.value, ```
@ -0,0 +196,4 @@
end
return math.floor(data.value), data.meta
end
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-27 16:54:37 +00:00

ugui.numberbox returns data.meta, but the registry logic never stores a meta field into control_data (only signal_change). This will return nil for Meta even when the control changes. Capture the ControlReturnValue from ugui.control(control, 'numberbox') and return its .meta (updating signal_change if you mutate data.value afterward).

`ugui.numberbox` returns `data.meta`, but the registry logic never stores a `meta` field into `control_data` (only `signal_change`). This will return `nil` for Meta even when the control changes. Capture the `ControlReturnValue` from `ugui.control(control, 'numberbox')` and return its `.meta` (updating `signal_change` if you mutate `data.value` afterward).
@ -0,0 +55,4 @@
is_checked = selected_index == i,
})
if not previous == new then
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-27 16:54:38 +00:00

if not previous == new then is parsed as (not previous) == new, so it won't reliably detect changes in selection state. Use if previous ~= new (or if new and not previous) to correctly update selected_index when a different tab becomes checked.

        if previous ~= new then
`if not previous == new then` is parsed as `(not previous) == new`, so it won't reliably detect changes in selection state. Use `if previous ~= new` (or `if new and not previous`) to correctly update `selected_index` when a different tab becomes checked. ```suggestion if previous ~= new then ```
@ -0,0 +14,4 @@
---@param control ToggleButton
---@return ControlReturnValue
logic = function(control, data)
data.is_checked = control.is_checked
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-27 16:54:37 +00:00

ugui.registry.toggle_button.validate asserts control.is_checked is a boolean, but the public type (ToggleButton.is_checked) is optional and documented as defaulting to unchecked when nil. Accept nil here (treat as false) to match the documented API and avoid forcing callers to always pass is_checked.

        ugui.internal.assert(control.is_checked == nil or type(control.is_checked) == 'boolean', 'expected is_checked to be boolean or nil')
    end,
    ---@param control ToggleButton
    ---@return ControlReturnValue
    logic = function(control, data)
        -- Treat nil is_checked as false (unchecked) per API documentation.
        data.is_checked = control.is_checked or false
`ugui.registry.toggle_button.validate` asserts `control.is_checked` is a boolean, but the public type (`ToggleButton.is_checked`) is optional and documented as defaulting to unchecked when nil. Accept `nil` here (treat as false) to match the documented API and avoid forcing callers to always pass `is_checked`. ```suggestion ugui.internal.assert(control.is_checked == nil or type(control.is_checked) == 'boolean', 'expected is_checked to be boolean or nil') end, ---@param control ToggleButton ---@return ControlReturnValue logic = function(control, data) -- Treat nil is_checked as false (unchecked) per API documentation. data.is_checked = control.is_checked or false ```
@ -0,0 +513,4 @@
end
color = result
end
table.insert(segments, { type = 'icon', value = icon_name, color = color ~= '' and color or nil })
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-27 16:54:38 +00:00

parse_rich_text uses color:find('.') to detect whether the color is a param-path, but in Lua patterns . matches any character, so this condition is true for any non-empty color (including hex colors like #FFFF00). That will incorrectly treat hex colors as paths and likely resolve to nil. Use an escaped dot pattern ('%.') and/or explicitly detect hex strings (e.g. color:sub(1,1) == '#').

`parse_rich_text` uses `color:find('.')` to detect whether the color is a param-path, but in Lua patterns `.` matches any character, so this condition is true for any non-empty color (including hex colors like `#FFFF00`). That will incorrectly treat hex colors as paths and likely resolve to nil. Use an escaped dot pattern (`'%.')` and/or explicitly detect hex strings (e.g. `color:sub(1,1) == '#'`).
@ -0,0 +579,4 @@
-- Determine the hovered control if we haven't already
if ugui.internal.hovered_control == nil then
if is_point_inside_rectangle(ugui.internal.environment.mouse_position, control.rectangle) then
ugui.internal.hovered_control = control.uid
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-27 16:54:36 +00:00

do_input_processing uses a local is_point_inside_rectangle that ignores ugui.internal.is_point_inside_control (enabled state + window bounds). As a result, controls can be hovered/clicked even when the mouse is outside the window (negative coords), which contradicts the intended offscreen hittest behavior and makes is_point_inside_control unused for core input routing. Consider using ugui.internal.is_point_inside_control (or replicating its checks) when determining hovered/clicked controls.

`do_input_processing` uses a local `is_point_inside_rectangle` that ignores `ugui.internal.is_point_inside_control` (enabled state + window bounds). As a result, controls can be hovered/clicked even when the mouse is outside the window (negative coords), which contradicts the intended offscreen hittest behavior and makes `is_point_inside_control` unused for core input routing. Consider using `ugui.internal.is_point_inside_control` (or replicating its checks) when determining hovered/clicked controls.
@ -0,0 +717,4 @@
height = rectangle.height,
}, -7)
ugui.standard_styler.draw_icon(icon_rect, ugui.standard_styler.params.menu_item.height, nil, 'arrow_right')
end
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-27 16:54:36 +00:00

draw_menu_item passes ugui.standard_styler.params.menu_item.height (a number) as the color argument to draw_icon for both the checkmark and submenu arrow. This will be treated as a RawColor integer and produce incorrect rendering. Pass an actual color (e.g. ugui.standard_styler.params.menu_item.text[visual_state] or another dedicated icon color) instead of the height.

`draw_menu_item` passes `ugui.standard_styler.params.menu_item.height` (a number) as the `color` argument to `draw_icon` for both the checkmark and submenu arrow. This will be treated as a RawColor integer and produce incorrect rendering. Pass an actual color (e.g. `ugui.standard_styler.params.menu_item.text[visual_state]` or another dedicated icon color) instead of the height.
Sign in to join this conversation.
No description provided.