Clear out the blocks in dispose. Fixes #1758 where old monitored vari… #1818
Conversation
…ld monitored variables were getting kept across project loads
… loaded so was hand editing the monitorBlocks before loading the default project. The code I added to dispose was then clearing that out.
There was a problem hiding this comment.
Hmmm it looks like there's something weird going on with the monitor blocks that have a menu.
You can observe via:
- Create a monitor for backdrop number
- Create a monitor for backdrop name; observe that the backdrop name monitor appears empty
- Change the selected option in the menu of the costume block in the toolbox; observe that the backdrop name monitor got filled in
It's not immediately clear what's going on here, but I think we should investigate and then decide what to do about it.
…ting all of the monitor blocks for each sprite and the global variables.
|
Fixed @kchadha's bug (great fine!). Since we ended up working together and tried multiple approaches, looking for a new set of eyes on this PR. |
There was a problem hiding this comment.
Meeps, needing to tip-toe around certain recycled blocks makes me scared. But if it works!
We might want to file an "ergonomics" issue for clarifying what "dispose" means for the runtime, since it is becoming pretty specialized. It doesn't, for instance, stop the stepping loop for threads, which may or may not be an oversight...
|
@paulkaplan, that sounds like a good idea. Yeah, runtime dispose becoming suddenly populated with a lot of different things is definitely worrisome and something to keep an eye on. This was a great example of us not realizing that we're making assumptions in other places that certain things (e.g. stage monitor blocks in the monitor block container) are going to stick around and things break if they are cleared out. It would be good to track these kinds of things in |
|
Filed #1829 and labeled it both tech debt and ergonomics. If there are other dispose-y things that show up soon, please pile them on. |
picklesrus
dismissed
kchadha’s stale review
Because we resolved stuff but somehow github doesn't think so
…ables were getting kept across project loads
Resolves
Fixes #1758
Proposed Changes
The original approach of clearing out all of monitor blocks didn't work (see the note below) because we recycle stage monitor blocks.
This way only deletes sprite specific monitors and global variables but leaves stage monitors (like backdrop number) alone.
Test Coverage
Added integration test and verified that it fails without the change to dispose.