Description

This addresses a class of bugs related computing the "last measure" of a student's song, which is used to determine several things such as when to stop, how long to render the timeline, and when to show validation. There have been a few bugs with this in the past, most recently reported by @dancodedotorg (Slack thread) where an incorrect calculation of the last measure would cause playback to stop unexpected early. This ended up being a bigger refactor that aims to simplify the contract between the view (MusicView) and the code compilation/execution unit (MusicBlocklyWorkspace).

Current State

Currently, MusicView owns a Sequencer component that it passes to MusicBlocklyWorkspace to provide as global scope for compiled Blockly JS code. The blockly code calls this sequencer which assembles a list of playback events, from which is derived the last measure. MusicView reads data off of the sequencer after calling MusicBlocklyWorkspace to compile and execute code.

The issue is that because MusicView manages the sequencer, it needs to have internal knowledge of how exactly the sequencer gets used by the underlying Blockly execution code; i.e. when it's reset, when it will have the latest events, etc.

For these specific bugs, the issue was that in MusicBlocklyWorkspace's executeCompiledCode function, we would execute each code hook one after another - first the "when run" code, and then any active triggers. This is specifically to address the case where a trigger is currently playing but the code inside of it has changed. Because we executed these all one after another, when MusicView finally read the last measure from the sequencer, it would return the last measure of the last executed code hook specifically, and not the last measure overall. So if the full song had events going for 30 measure, but the last trigger executed only ran for 1 measure starting at measure 9, we would mistakenly compute the last measure as measure 10, and end early. This is true even if the when_run code was shorter than the triggers; for example, if two triggers had been executed but one ran longer than the other, if the shorter one executed second, we would treat the last measure of the shorter trigger as the last measure of the song and end early.

Changes

Rather than trying to fix each bug one by one, the main refactor here is to simplify the interface between MusicView and MusicBlocklyWorkspace into something like: "given these blocks/code, give me the computed events and last measure". In order to do this, we move the Sequencer out of MusicView, and into the MusicBlocklyWorkspace as an internal component. MusicBlocklyWorkspace manages resetting the sequencer, merging events, and computing the last measure if there are multiple back to back executions, and simply returns the computed playback events, ordered functions, and last measure whenever it executes code. MusicView doesn't need to manage executing code and trying to manage the sequencer/knowing when it should read data from it. As a result, the last measure we get back should always be the true last measure (because MusicBlocklyWorkspace handles returning the max value after multiple executions), and we get a simpler interface between these two core components overall.

Screen Recordings

BEFORE

• Dan's repro: https://codedotorg.slack.com/archives/C07UT279KM1/p1745325344479719?thread_ts=1745325204.304879&cid=C07UT279KM1
• Alternate repro - long triggered followed by short trigger getting cut off early:

AFTER

• Long when run, and long triggered - notice that the song doesn't stop even though the last trigger (update code-dot-org README #3) has only one short sound

Links

• Jira: https://codedotorg.atlassian.net/browse/LABS-1443

Testing story

Tested locally (see screen recordings)