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

fix several function bugs in new blockly #10402

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

riknoll
Copy link
Member

@riknoll riknoll commented Feb 27, 2025

this fixes a few bugs with functions in new blockly that were discovered on livestream. namely:

  1. the function toolbox flyout was being cached too aggressively causing it to not update when existing functions were edited
  2. undoing/redoing edits to function definitions (e.g. right click + edit function) were not always working correctly
  3. editing functions was causing any attached shadow blocks to have their state reset in the calls to those functions

to fix all these issues, i've overhauled how the mutation of function blocks works. firstly, our old implementation disposed all of the argument inputs when a function was edited and then recreated them which led to the shadow issue mentioned above. now, only the edited arguments are created/destroyed. the rest of the inputs on the block are simply reordered.

secondly, we no longer rely on the blockly events spawned by editing the inputs on all of the changed blocks to handle undo/redo because keeping all of those events in sync was causing a lot of issues with edge cases. instead, i've created a custom blockly event that wraps up all of the edits for the mutation in one tidy package.

finally, the function flyout is now cached on the hash of its blocks rather than by its name. since we're essentially busting the cache here, i also tweaked our flyout so that the cache doesn't grow indefinitely. old cache entries now get evicted once we get to 20

there were a ton of tiny bugs here, so i look forward to everyone really hammering on this code in testing!

@riknoll riknoll requested a review from a team February 27, 2025 00:26
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