fix(TabControl): broken `selected_index` diffing #46

Merged

abart27 merged 1 commit from fix-tabcontrol-broken-check into main

2026-02-28 12:01:05 +00:00

abart27 commented

2026-02-28 11:44:43 +00:00

(Migrated from github.com)

Fixes broken diffing of selected_index when clicking the internal togglebuttons.

Fixes broken diffing of `selected_index` when clicking the internal togglebuttons.

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed

2026-02-28 11:48:18 +00:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull request overview

Fixes TabControl’s selected_index update logic when interacting with the per-tab internal toggle_button controls so selection changes propagate correctly through the control’s diffing / signal_change mechanism.

Changes:

Switches TabControl’s selection-update trigger from comparing toggle boolean states to using the toggle button’s meta.signal_change.
Updates selected_index when the toggle button reports a signal_change_states.started event.

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

## Pull request overview Fixes TabControl’s `selected_index` update logic when interacting with the per-tab internal `toggle_button` controls so selection changes propagate correctly through the control’s diffing / `signal_change` mechanism. **Changes:** - Switches TabControl’s selection-update trigger from comparing toggle boolean states to using the toggle button’s `meta.signal_change`. - Updates `selected_index` when the toggle button reports a `signal_change_states.started` event. --- 💡 <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>.

src/ugui/controls/tabcontrol.lua

meta.signal_change == ugui.signal_change_states.started is not a reliable way to detect a fresh click here because process_signal_changes advances started -> ongoing (and ended -> none) before considering the current pressed flag. That means a quick re-click on the same tab (or switching back to a tab you clicked in the last couple frames, especially at low FPS) can yield ongoing/none and this block won’t run, so selected_index won’t update. Prefer detecting the click directly (e.g., comparing the returned boolean to selected_index == i using ~= like the prior intent, or checking ugui.internal.clicked_control == control.uid + i) so every click is handled regardless of prior signal_change state.

`meta.signal_change == ugui.signal_change_states.started` is not a reliable way to detect a fresh click here because `process_signal_changes` advances `started -> ongoing` (and `ended -> none`) before considering the current `pressed` flag. That means a quick re-click on the same tab (or switching back to a tab you clicked in the last couple frames, especially at low FPS) can yield `ongoing`/`none` and this block won’t run, so `selected_index` won’t update. Prefer detecting the click directly (e.g., comparing the returned boolean to `selected_index == i` using `~=` like the prior intent, or checking `ugui.internal.clicked_control == control.uid + i`) so every click is handled regardless of prior `signal_change` state.

There are existing tabcontrol tests, but they currently only assert that rendering with no items doesn’t crash. Since this change alters click/selection behavior, please add a test that simulates clicking a tab and asserts the returned selected_index changes and meta.signal_change is updated accordingly (and ideally covers rapid switching between tabs).

There are existing tabcontrol tests, but they currently only assert that rendering with no items doesn’t crash. Since this change alters click/selection behavior, please add a test that simulates clicking a tab and asserts the returned `selected_index` changes and `meta.signal_change` is updated accordingly (and ideally covers rapid switching between tabs).