fix(scratch-vm): create missing variable and broadcast definitions on import#554
Merged
cwillisf merged 1 commit intohotfix/scratch-blocks-release-triagefrom Apr 30, 2026
Conversation
Pasting blocks from the backpack (no source target) and importing sprites both rely on `Target.fixUpVariableReferences` to reconcile variable references against the project. Two gaps caused references to dangle: `shareBlocksToTarget` only ran reconciliation when called with a source target, and `fixUpVariableReferences` walked variable and list references but not broadcast references. `fixUpVariableReferences` now includes broadcasts via `getAllVariableAndListReferences(null, true)` and is safe to run on the stage; the local-vs-global rename branches that don't apply to the global scope are guarded with `!this.isStage`. `shareBlocksToTarget` calls it after creating the new blocks when no source target was provided. Refs #533
Contributor
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes missing variable/list/broadcast definitions when blocks/sprites are imported into a project (notably via backpack paste and sprite import), by reconciling referenced IDs against project state in scratch-vm and creating/remapping the needed stage-scoped definitions.
Changes:
- Extend
Target.fixUpVariableReferences()to include broadcast references and to safely run when the receiving target is the stage. - Update
VirtualMachine.shareBlocksToTarget()to calltarget.fixUpVariableReferences()when there is no source target (e.g., blocks originating outside the project such as backpack paste). - Add unit tests covering broadcast reconciliation, stage-as-target behavior, backpack-paste-like sharing without a source target, and the sprite-import broadcast path.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/scratch-vm/src/engine/target.js | Reconciles broadcast references alongside variables/lists and allows reconciliation to run on stage targets while guarding rename-only logic. |
| packages/scratch-vm/src/virtual-machine.js | Runs reference reconciliation after block sharing when blocks come from outside the project (no source target). |
| packages/scratch-vm/test/unit/engine_target.js | Adds targeted tests for broadcast reconciliation behavior and stage-as-target behavior in fixUpVariableReferences(). |
| packages/scratch-vm/test/unit/virtual-machine.js | Adds tests for shareBlocksToTarget without a source target and for sprite-import broadcast reconciliation via installTargets. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
f687340
into
hotfix/scratch-blocks-release-triage
17 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves
Part of #533. Fixes four related items in the spork tracker:
Proposed Changes
Two gaps in
scratch-vmhad the same shape:VirtualMachine.shareBlocksToTargetonly ran reference reconciliation when called with a source target ID. The backpack-paste path passes no source target, so pasted scripts referenced ids that weren't defined anywhere in the receiving project.Target.fixUpVariableReferenceswalked variable and list references but skipped broadcasts, becausegetAllVariableAndListReferenceswas called withoutoptIncludeBroadcast. The sprite-import path (installTargetswithwholeProject=false) callsfixUpVariableReferences, so imported sprites left broadcast references undefined.This PR:
fixUpVariableReferencesto walk broadcast references and to run safely on the stage. The local-vs-global rename branches that don't apply on the global scope are guarded with!this.isStage. Updates the JSDoc.shareBlocksToTargetto calltarget.fixUpVariableReferences()after creating the new blocks when there is no source target.Reason for Changes
Pre-spork, the forked
scratch-blocksauto-created Blockly variable-model entries for unknown ids referenced by blocks added to the workspace. Those auto-creations firedvar_createevents thatscratch-vm'sBlocks.blocklyListenconsumed, materializing the missing definitions on the right target. Upstream Blockly does not auto-create from unknown ids the same way, so the unfork removed that implicit path and the references began to dangle.Doing the reconciliation in
scratch-vmitself makes it self-sufficient: the authoritative project state lives there, the existingfixUpVariableReferencesalready handles the merge / create / remap cases for variables and lists, and including broadcasts is a small extension to that logic. The change is also idempotent against any futurescratch-blocksbehavior.Test Coverage
packages/scratch-vm/test/unit/engine_target.js: 5 new Tap tests covering broadcast handling infixUpVariableReferences(undefined ref creates a stage broadcast, same-name remap, idempotency) and stage-as-target (no rename of existing stage variables, broadcast creation when stage is the receiving target).packages/scratch-vm/test/unit/virtual-machine.js: 4 new Tap tests forshareBlocksToTargetwithout a source target (variable creation, broadcast creation, name-collision remap, stage as target) and 1 forinstallTargetscovering the sprite-import broadcast path.npm test --workspace=packages/scratch-vmpasses (3698 / 3698).npm run buildandnpm testfrom the repo root pass on every workspace.Manual Testing
.sprite3that references broadcasts; broadcasts dropdown is populated on the receiving project.when I receiveactually fires at runtime.