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

Precise Widget Recreation #125

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Conversation

LPGhatguy
Copy link
Member

@LPGhatguy LPGhatguy commented Dec 1, 2023

This PR currently started from work that was sitting in a Git stash on my computer for a long time. I'm opening this PR so that it doesn't grow too stale, and to raise awareness that this work is happening!

The Problem

This work spawned out of a bug report from @Ralith about textbox focus seeming to get lost between menus in a game he's working on. I pulled on the thread and found out that IDs were being reused for very different widgets. It seems like the only way that widgets ever got dropped were:

  1. If a widget changed type, that widget would be updated in-place with the new widget type, keeping the same ID
  2. If children were removed in an update, the list of children would get trimmed and dropped once the parent widget ended

The result of this is that IDs on the main branch of yakui are reused heavily. That... pretty much completely defeats the point of using a generational arena! The original bug occurs because the currently selected widget ID always points to a valid widget and never changes, causing it to get stuck on a widget with no focus handling code.

The Solution

To solve this problem, this PR changes our "tree shape" reconciliation code to completely tear down subtrees whenever widget types change, and to reallocate those nodes with new IDs. That will make any widget IDs invalid instead of letting them point at now-invalid widgets.

Current Status

It seems to work! This may have fixed some or all of #69 as a side effect from the increased visibility doing this.

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