Skip to content

Conversation

@WhyPenguins
Copy link
Contributor

@WhyPenguins WhyPenguins commented May 12, 2025

Description

Context

The current UI system automatically generates IDs for each element to handle focus/other stateful aspects. For many elements, the IDs are generated based on the address of the input variable passed in (for instance the text box's string, the checkbox state, slider value, etc). This doesn't work once the SplashKit translation layers are involved, leading to state being incorrectly shared between different UI elements.

This PR attempts to improve this by keeping track of each element's index within its context (panel/popup/inset/treenode). Rather than using the input variable's address for the ID, it just uses an incrementing number that is contained to that context.

How it works

This interacts with MicroUI's own context stack to work properly. MicroUI already contains a context stack, which contains an ID for each panel/popup/inset/treenode in the current stack, generated from their label (which is required).

When we generate IDs for the UI elements via mu_get_id, it uses the ID for the current context as a base.

So all we need to do is keep track of an integer incrementing from 0 within the context, and provide that to mu_get_id rather than the input value's address - MicroUI will handle making this integer unique in each context.

Known Issues

This works well for most types of interfaces - the only edge cases I can think of are when the interface changes during an interaction. For instance when a text box is focused (let's say its ID is 2), if a different text box appears above it within the same context while the user is typing, focus will switch to that text box (since that text box now has ID 2, while the one the user was typing in has become 3). This can be seen in the ID Test 2 section of the UI test.

Future work can involve anchoring the IDs when labels are known (since text inputs, checkboxes, etc, may have a label).

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

The existing UI test has been checked to ensure things still work. Checking that the state of unrelated elements don't affect each other (unless in the case above) has been tested. The fail case above has also been added as a test to demonstrate it.

This hasn't been tested when ran through translation layers however - I can't foresee any issues (since it isn't using the variable addresses anymore), but that doesn't mean there aren't any 😅

Testing Checklist

  • Tested with sktest
  • Tested with skunit_tests

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have requested a review from ... on the Pull Request

@WhyPenguins WhyPenguins force-pushed the sb/fix_UI_ID_generation branch from 43062f5 to 5368b7f Compare May 12, 2025 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant