From 81d1b09cb599737caf524340c683c90d4e5ff9e4 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Thu, 7 Sep 2023 20:53:03 +0000 Subject: [PATCH] fix: caller args disconnecting shareable procedures --- .../block-shareable-procedures/src/blocks.ts | 18 ++++- .../test/index.html | 2 +- .../block-shareable-procedures/test/index.js | 66 +++++++++---------- .../test/procedure_blocks.mocha.js | 1 + 4 files changed, 52 insertions(+), 35 deletions(-) diff --git a/plugins/block-shareable-procedures/src/blocks.ts b/plugins/block-shareable-procedures/src/blocks.ts index d7084bcbef..4e7e3ff9cf 100644 --- a/plugins/block-shareable-procedures/src/blocks.ts +++ b/plugins/block-shareable-procedures/src/blocks.ts @@ -1003,6 +1003,7 @@ const procedureCallerUpdateShapeMixin = { * data model. */ updateParameters_: function() { + this.ensureArgsMapIsInitialized_(); this.deleteAllArgInputs_(); this.addParametersLabel__(); this.createArgInputs_(); @@ -1010,6 +1011,21 @@ const procedureCallerUpdateShapeMixin = { this.prevParams_ = [...this.getProcedureModel().getParameters()]; }, + /** + * Makes sure that if we are updating the parameters before any move events + * have happened, the args map records the current state of the block. Does + * not remove entries from the array, since blocks can be disconnected + * temporarily during mutation (which triggers this method). + */ + ensureArgsMapIsInitialized_: function() { + // We look at the prevParams array because the current state of the block + // matches the old params, not the new params state. + for (const [i, p] of this.prevParams_.entries()) { + const target = this.getInputTargetBlock(`ARG${i}`); + if (target) this.argsMap_.set(p.getId(), target); + } + }, + /** * Saves a map of parameter IDs to target blocks attached to the inputs * of this caller block. @@ -1112,7 +1128,7 @@ const procedureCallerOnChangeMixin = { if (this.disposed || this.workspace.isFlyout) return; // Blockly.Events not generated by user. Skip handling. if (!event.recordUndo) return; - if (event.type === Blockly.Events.BLOCK_MOVE) this.updateArgsMap_(); + if (event.type === Blockly.Events.BLOCK_MOVE) this.updateArgsMap_(true); if (event.type !== Blockly.Events.BLOCK_CREATE) return; if (event.blockId !== this.id && event.ids.indexOf(this.id) === -1) return; // We already found our model, which means we don't need to create a block. diff --git a/plugins/block-shareable-procedures/test/index.html b/plugins/block-shareable-procedures/test/index.html index de969229e2..c0731becd6 100644 --- a/plugins/block-shareable-procedures/test/index.html +++ b/plugins/block-shareable-procedures/test/index.html @@ -45,7 +45,7 @@
-
+

Saved State

diff --git a/plugins/block-shareable-procedures/test/index.js b/plugins/block-shareable-procedures/test/index.js index be65ca1d96..d53c567d61 100644 --- a/plugins/block-shareable-procedures/test/index.js +++ b/plugins/block-shareable-procedures/test/index.js @@ -10,8 +10,7 @@ import * as Blockly from 'blockly'; import {toolboxCategories} from '@blockly/dev-tools'; import {blocks, unregisterProcedureBlocks, registerProcedureSerializer} from '../src/index'; -import {ProcedureBase} from '../src/events_procedure_base'; - +// import {ProcedureBase} from '../src/events_procedure_base'; unregisterProcedureBlocks(); Blockly.common.defineBlocks(blocks); @@ -19,25 +18,25 @@ Blockly.common.defineBlocks(blocks); registerProcedureSerializer(); let workspace1; -let workspace2; +// let workspace2; document.addEventListener('DOMContentLoaded', function() { const options = { toolbox: toolboxCategories, }; workspace1 = Blockly.inject('blockly1', options); - workspace2 = Blockly.inject('blockly2', options); + // workspace2 = Blockly.inject('blockly2', options); // If we allow undoing operations, it can create event loops when sharing // events between workspaces. - workspace1.MAX_UNDO = 0; - workspace2.MAX_UNDO = 0; - workspace1.addChangeListener(createEventSharer(workspace2)); - workspace2.addChangeListener(createEventSharer(workspace1)); + // workspace1.MAX_UNDO = 0; + // workspace2.MAX_UNDO = 0; + // workspace1.addChangeListener(createEventSharer(workspace2)); + // workspace2.addChangeListener(createEventSharer(workspace1)); workspace1.addChangeListener( createSerializationListener(workspace1, 'save1')); - workspace2.addChangeListener( - createSerializationListener(workspace2, 'save2')); + // workspace2.addChangeListener( + // createSerializationListener(workspace2, 'save2')); document.getElementById('load') .addEventListener('click', () => reloadWorkspaces()); @@ -49,25 +48,25 @@ document.addEventListener('DOMContentLoaded', function() { * @param {!Blockly.Workspace} otherWorkspace The workspace to share events to. * @returns {function(Blockly.Events.Abstract)} The listener. */ -function createEventSharer(otherWorkspace) { - return (e) => { - if (!(e instanceof ProcedureBase) && - !(e instanceof Blockly.Events.VarBase)) { - return; - } - let event; - try { - event = Blockly.Events.fromJson(e.toJson(), otherWorkspace); - } catch (e) { - // Could not deserialize event. This is expected to happen. E.g. - // when round-tripping parameter deletes, the delete in the - // secondary workspace cannot be deserialized into the original - // workspace. - return; - } - event.run(true); - }; -} +// function createEventSharer(otherWorkspace) { +// return (e) => { +// if (!(e instanceof ProcedureBase) && +// !(e instanceof Blockly.Events.VarBase)) { +// return; +// } +// let event; +// try { +// event = Blockly.Events.fromJson(e.toJson(), otherWorkspace); +// } catch (e) { +// // Could not deserialize event. This is expected to happen. E.g. +// // when round-tripping parameter deletes, the delete in the +// // secondary workspace cannot be deserialized into the original +// // workspace. +// return; +// } +// event.run(true); +// }; +// } /** * Returns an event listener that serializes the given workspace to JSON and @@ -92,12 +91,13 @@ function createSerializationListener(workspace, textAreaId) { * Reloads both workspaces to showcase serialization working. */ async function reloadWorkspaces() { - const save1 = Blockly.serialization.workspaces.save(workspace1); - const save2 = Blockly.serialization.workspaces.save(workspace2); + const save1 = JSON.parse(document.getElementById('save1').value); + // const save1 = Blockly.serialization.workspaces.save(workspace1); + // const save2 = Blockly.serialization.workspaces.save(workspace2); workspace1.clear(); - workspace2.clear(); + // workspace2.clear(); // Show people the cleared workspace so they know the reload did something. await new Promise((resolve) => setTimeout(resolve, 500)); Blockly.serialization.workspaces.load(save1, workspace1); - Blockly.serialization.workspaces.load(save2, workspace2); + // Blockly.serialization.workspaces.load(save2, workspace2); } diff --git a/plugins/block-shareable-procedures/test/procedure_blocks.mocha.js b/plugins/block-shareable-procedures/test/procedure_blocks.mocha.js index b2c770a8b2..5f7e429706 100644 --- a/plugins/block-shareable-procedures/test/procedure_blocks.mocha.js +++ b/plugins/block-shareable-procedures/test/procedure_blocks.mocha.js @@ -76,6 +76,7 @@ suite('Procedures', function() { .callsFake((callback) => { callback(); }); + window.cancelAnimationFrame = this.sandbox.stub(); }); teardown(function() {