fix: caller args disconnecting shareable procedures

google · Sep 7, 2023 · 81d1b09 · 81d1b09
1 parent af95f13
commit 81d1b09
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 35 deletions.
diff --git a/plugins/block-shareable-procedures/src/blocks.ts b/plugins/block-shareable-procedures/src/blocks.ts
@@ -1003,13 +1003,29 @@ const procedureCallerUpdateShapeMixin = {
    * data model.
    */
   updateParameters_: function() {
+    this.ensureArgsMapIsInitialized_();
     this.deleteAllArgInputs_();
     this.addParametersLabel__();
     this.createArgInputs_();
     this.reattachBlocks_();
     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
@@ -45,7 +45,7 @@
 <body>
   <div id="root">
     <div class="blockly" id="blockly1"></div>
-    <div class="blockly" id="blockly2"></div>
+    <!-- <div class="blockly" id="blockly2"></div> -->
     <div id="saveArea">
       <h2>Saved State</h2>
       <input id="load" type="button" value="Reload"/>

diff --git a/plugins/block-shareable-procedures/test/index.js b/plugins/block-shareable-procedures/test/index.js
@@ -10,34 +10,33 @@
 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);
 
 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
@@ -76,6 +76,7 @@ suite('Procedures', function() {
         .callsFake((callback) => {
           callback();
         });
+    window.cancelAnimationFrame = this.sandbox.stub();
   });
 
   teardown(function() {