Merge pull request #3133 from BeksOmega/fixes/ConnectionTracking

Connection Tracking Pt 3: Changed connections to use 'tracked_' property
2026-01-08 09:30:06 +01:00 · 2019-10-07 11:05:10 -07:00
parent ddd0cb5632 e09a7b8165
commit 608ce7b93c
8 changed files with 195 additions and 164 deletions
--- a/core/block_svg.js
+++ b/core/block_svg.js
@@ -130,6 +130,14 @@ Blockly.BlockSvg = function(workspace, prototypeName, opt_id) {
   * @private
   */
  this.markerSvg_ = null;
+
+  /**
+   * Should the block tell its connections to start tracking inside the render
+   * method?
+   * @type {boolean}
+   * @private
+   */
+  this.callTrackConnections_ = true;
 };
 Blockly.utils.object.inherits(Blockly.BlockSvg, Blockly.Block);

@@ -138,6 +146,7 @@ Blockly.utils.object.inherits(Blockly.BlockSvg, Blockly.Block);
 * Height is in workspace units.
 */
 Blockly.BlockSvg.prototype.height = 0;
+
 /**
 * Width of this block, including any connected value blocks.
 * Width is in workspace units.
@@ -1488,35 +1497,58 @@ Blockly.BlockSvg.prototype.appendInput_ = function(type, name) {
 };

 /**
- * Set whether the connections are hidden (not tracked in a database) or not.
- * Recursively walk down all child blocks (except collapsed blocks).
- * @param {boolean} hidden True if connections are hidden.
+ * Tell the block to wait for an outside source to call
+ * startTrackingConnections, rather than starting connection
+ * tracking automatically.
+ *
+ * Also tells children of this block to wait.
 * @package
 */
-Blockly.BlockSvg.prototype.setConnectionsHidden = function(hidden) {
-  if (!hidden && this.isCollapsed()) {
-    if (this.outputConnection) {
-      this.outputConnection.setHidden(hidden);
+Blockly.BlockSvg.prototype.waitToTrackConnections = function() {
+  this.callTrackConnections_ = false;
+  var children = this.getChildren();
+  for (var i = 0, child; child = children[i]; i++) {
+    child.waitToTrackConnections();
+  }
+};
+
+/**
+ * Tell this block's connections to add themselves to the connection
+ * database (i.e. start tracking).
+ *
+ * All following/next blocks will be told to start tracking. Inner blocks
+ * (i.e. blocks attached to value/statement inputs) will be told to start
+ * tracking if this block is not collapsed.
+ * @package
+ */
+Blockly.BlockSvg.prototype.startTrackingConnections = function() {
+  if (this.previousConnection) {
+    this.previousConnection.setTracking(true);
+  }
+  if (this.outputConnection) {
+    this.outputConnection.setTracking(true);
+  }
+  if (this.nextConnection) {
+    this.nextConnection.setTracking(true);
+    var child = this.nextConnection.targetBlock();
+    if (child) {
+      child.startTrackingConnections();
    }
-    if (this.previousConnection) {
-      this.previousConnection.setHidden(hidden);
-    }
-    if (this.nextConnection) {
-      this.nextConnection.setHidden(hidden);
-      var child = this.nextConnection.targetBlock();
-      if (child) {
-        child.setConnectionsHidden(hidden);
-      }
-    }
-  } else {
-    var myConnections = this.getConnections_(true);
-    for (var i = 0, connection; connection = myConnections[i]; i++) {
-      connection.setHidden(hidden);
-      if (connection.isSuperior()) {
-        var child = connection.targetBlock();
-        if (child) {
-          child.setConnectionsHidden(hidden);
-        }
+  }
+
+  if (this.collapsed_) {
+    return;
+  }
+
+  for (var i = 0; i < this.inputList.length; i++) {
+    var conn = this.inputList[i].connection;
+    if (conn) {
+      conn.setTracking(true);
+
+      // Pass tracking on down the chain.
+      var block = conn.targetBlock();
+      if (block) {
+        block.startTrackingConnections();
      }
    }
  }
@@ -1665,6 +1697,13 @@ Blockly.BlockSvg.prototype.render = function(opt_bubble) {
  (/** @type {!Blockly.WorkspaceSvg} */ (this.workspace)).getRenderer().render(this);
  // No matter how we rendered, connection locations should now be correct.
  this.updateConnectionLocations_();
+  // TODO: This should be handled inside a robust init method, because it would
+  //  make it a lot cleaner, but for now it's handled here for backwards
+  //  compatibility.
+  if (this.callTrackConnections_) {
+    this.startTrackingConnections();
+    this.callTrackConnections_ = false;
+  }
  if (opt_bubble !== false) {
    // Render all blocks above this one (propagate a reflow).
    var parentBlock = this.getParent();
--- a/core/connection.js
+++ b/core/connection.js
@@ -201,8 +201,7 @@ Blockly.Connection.prototype.connect_ = function(childConnection) {
 };

 /**
- * Dispose of this connection. Deal with connected blocks and remove this
- * connection from the database.
+ * Dispose of this connection and deal with connected blocks.
 * @package
 */
 Blockly.Connection.prototype.dispose = function() {
--- a/core/connection_db.js
+++ b/core/connection_db.js
@@ -16,14 +16,16 @@
 */

 /**
- * @fileoverview Components for managing connections between blocks.
+ * @fileoverview A database of all the rendered connections that could
+ *    possibly be connected to (i.e. not collapsed, etc).
+ *    Sorted by y coordinate.
 * @author fraser@google.com (Neil Fraser)
 */
 'use strict';

 goog.provide('Blockly.ConnectionDB');

-goog.require('Blockly.Connection');
+goog.require('Blockly.RenderedConnection');


 /**
@@ -34,44 +36,42 @@ goog.require('Blockly.Connection');
 */
 Blockly.ConnectionDB = function() {
  /**
-   * Array of connections sorted by y coordinate.
-   * @type {!Array.<!Blockly.Connection>}
+   * Array of connections sorted by y position in workspace units.
+   * @type {!Array.<!Blockly.RenderedConnection>}
   * @private
   */
  this.connections_ = [];
 };

 /**
- * Add a connection to the database.  Must not already exist in DB.
- * @param {!Blockly.Connection} connection The connection to be added.
+ * Add a connection to the database. Should not already exist in the database.
+ * @param {!Blockly.RenderedConnection} connection The connection to be added.
+ * @param {number} yPos The y position used to decide where to insert the
+ *    connection.
+ * @package
 */
-Blockly.ConnectionDB.prototype.addConnection = function(connection) {
-  if (connection.inDB_) {
-    throw Error('Connection already in database.');
-  }
-  if (connection.getSourceBlock().isInFlyout) {
-    // Don't bother maintaining a database of connections in a flyout.
-    return;
-  }
-  var position = this.findPositionForConnection_(connection);
-  this.connections_.splice(position, 0, connection);
-  connection.inDB_ = true;
+Blockly.ConnectionDB.prototype.addConnection = function(connection, yPos) {
+  var index = this.calculateIndexForYPos_(yPos);
+  this.connections_.splice(index, 0, connection);
 };

 /**
- * Find the given connection.
+ * Finds the index of the given connection.
+ *
 * Starts by doing a binary search to find the approximate location, then
- *     linearly searches nearby for the exact connection.
- * @param {!Blockly.Connection} conn The connection to find.
+ * linearly searches nearby for the exact connection.
+ * @param {!Blockly.RenderedConnection} conn The connection to find.
+ * @param {number} yPos The y position used to find the index of the connection.
 * @return {number} The index of the connection, or -1 if the connection was
 *     not found.
+ * @private
 */
-Blockly.ConnectionDB.prototype.findConnection = function(conn) {
+Blockly.ConnectionDB.prototype.findIndexOfConnection_ = function(conn, yPos) {
  if (!this.connections_.length) {
    return -1;
  }

-  var bestGuess = this.findPositionForConnection_(conn);
+  var bestGuess = this.calculateIndexForYPos_(yPos);
  if (bestGuess >= this.connections_.length) {
    // Not in list
    return -1;
@@ -99,15 +99,13 @@ Blockly.ConnectionDB.prototype.findConnection = function(conn) {
 };

 /**
- * Finds a candidate position for inserting this connection into the list.
- * This will be in the correct y order but makes no guarantees about ordering in
- *     the x axis.
- * @param {!Blockly.Connection} connection The connection to insert.
+ * Finds the correct index for the given y position.
+ * @param {number} yPos The y position used to decide where to
+ *    insert the connection.
 * @return {number} The candidate index.
 * @private
 */
-Blockly.ConnectionDB.prototype.findPositionForConnection_ = function(
-    connection) {
+Blockly.ConnectionDB.prototype.calculateIndexForYPos_ = function(yPos) {
  if (!this.connections_.length) {
    return 0;
  }
@@ -115,9 +113,9 @@ Blockly.ConnectionDB.prototype.findPositionForConnection_ = function(
  var pointerMax = this.connections_.length;
  while (pointerMin < pointerMax) {
    var pointerMid = Math.floor((pointerMin + pointerMax) / 2);
-    if (this.connections_[pointerMid].y_ < connection.y_) {
+    if (this.connections_[pointerMid].y_ < yPos) {
      pointerMin = pointerMid + 1;
-    } else if (this.connections_[pointerMid].y_ > connection.y_) {
+    } else if (this.connections_[pointerMid].y_ > yPos) {
      pointerMax = pointerMid;
    } else {
      pointerMin = pointerMid;
@@ -129,28 +127,25 @@ Blockly.ConnectionDB.prototype.findPositionForConnection_ = function(

 /**
 * Remove a connection from the database.  Must already exist in DB.
- * @param {!Blockly.Connection} connection The connection to be removed.
- * @private
+ * @param {!Blockly.RenderedConnection} connection The connection to be removed.
+ * @param {number} yPos The y position used to find the index of the connection.
+ * @throws {Error} If the connection cannot be found in the database.
 */
-Blockly.ConnectionDB.prototype.removeConnection_ = function(connection) {
-  if (!connection.inDB_) {
-    throw Error('Connection not in database.');
-  }
-  var removalIndex = this.findConnection(connection);
-  if (removalIndex == -1) {
+Blockly.ConnectionDB.prototype.removeConnection = function(connection, yPos) {
+  var index = this.findIndexOfConnection_(connection, yPos);
+  if (index == -1) {
    throw Error('Unable to find connection in connectionDB.');
  }
-  connection.inDB_ = false;
-  this.connections_.splice(removalIndex, 1);
+  this.connections_.splice(index, 1);
 };

 /**
 * Find all nearby connections to the given connection.
 * Type checking does not apply, since this function is used for bumping.
- * @param {!Blockly.Connection} connection The connection whose neighbours
- *     should be returned.
+ * @param {!Blockly.RenderedConnection} connection The connection whose
+ *     neighbours should be returned.
 * @param {number} maxRadius The maximum radius to another connection.
- * @return {!Array.<!Blockly.Connection>} List of connections.
+ * @return {!Array.<!Blockly.RenderedConnection>} List of connections.
 */
 Blockly.ConnectionDB.prototype.getNeighbours = function(connection, maxRadius) {
  var db = this.connections_;
@@ -204,7 +199,6 @@ Blockly.ConnectionDB.prototype.getNeighbours = function(connection, maxRadius) {
  return neighbours;
 };

-
 /**
 * Is the candidate connection close to the reference connection.
 * Extremely fast; only looks at Y distance.
@@ -220,15 +214,15 @@ Blockly.ConnectionDB.prototype.isInYRange_ = function(index, baseY, maxRadius) {

 /**
 * Find the closest compatible connection to this connection.
- * @param {!Blockly.Connection} conn The connection searching for a compatible
+ * @param {!Blockly.RenderedConnection} conn The connection searching for a compatible
 *     mate.
 * @param {number} maxRadius The maximum radius to another connection.
 * @param {!Blockly.utils.Coordinate} dxy Offset between this connection's
 *     location in the database and the current location (as a result of
 *     dragging).
- * @return {!{connection: ?Blockly.Connection, radius: number}} Contains two
- *     properties:' connection' which is either another connection or null,
- *     and 'radius' which is the distance.
+ * @return {!{connection: Blockly.RenderedConnection, radius: number}}
+ *     Contains two properties: 'connection' which is either another
+ *     connection or null, and 'radius' which is the distance.
 */
 Blockly.ConnectionDB.prototype.searchForClosest = function(conn, maxRadius,
    dxy) {
@@ -244,10 +238,10 @@ Blockly.ConnectionDB.prototype.searchForClosest = function(conn, maxRadius,
  conn.x_ = baseX + dxy.x;
  conn.y_ = baseY + dxy.y;

-  // findPositionForConnection finds an index for insertion, which is always
+  // calculateIndexForYPos_ finds an index for insertion, which is always
  // after any block with the same y index.  We want to search both forward
  // and back, so search on both sides of the index.
-  var closestIndex = this.findPositionForConnection_(conn);
+  var closestIndex = this.calculateIndexForYPos_(conn.y_);

  var bestConnection = null;
  var bestRadius = maxRadius;
--- a/core/input.js
+++ b/core/input.js
@@ -191,9 +191,9 @@ Blockly.Input.prototype.setVisible = function(visible) {
  if (this.connection) {
    // Has a connection.
    if (visible) {
-      renderList = this.connection.unhideAll();
+      renderList = this.connection.startTrackingAll();
    } else {
-      this.connection.hideAll();
+      this.connection.stopTrackingAll();
    }
    var child = this.connection.targetBlock();
    if (child) {
--- a/core/keyboard_nav/navigation.js
+++ b/core/keyboard_nav/navigation.js
@@ -205,7 +205,7 @@ Blockly.navigation.insertFromFlyout = function() {
  // Connections are hidden when the block is first created.  Normally there's
  // enough time for them to become unhidden in the user's mouse movements,
  // but not here.
-  newBlock.setConnectionsHidden(false);
+  newBlock.startTrackingConnections();
  workspace.getCursor().setCurNode(
      Blockly.ASTNode.createBlockNode(newBlock));
  if (!Blockly.navigation.modify_()) {
--- a/core/rendered_connection.js
+++ b/core/rendered_connection.js
@@ -65,27 +65,23 @@ Blockly.RenderedConnection = function(source, type) {
  this.offsetInBlock_ = new Blockly.utils.Coordinate(0, 0);

  /**
-   * Has this connection been added to the connection database?
+   * Whether this connections is tracked in the database or not.
   * @type {boolean}
   * @private
   */
-  this.inDB_ = false;
-
-  /**
-   * Whether this connections is hidden (not tracked in a database) or not.
-   * @type {boolean}
-   * @private
-   */
-  this.hidden_ = !this.db_;
+  this.tracked_ = false;
 };
 Blockly.utils.object.inherits(Blockly.RenderedConnection, Blockly.Connection);

 /**
+ * Dispose of this connection. Remove it from the database (if it is
+ * tracked) and call the super-function to deal with connected blocks.
 * @override
+ * @package
 */
 Blockly.RenderedConnection.prototype.dispose = function() {
-  if (this.inDB_) {
-    this.db_.removeConnection_(this);
+  if (this.tracked_) {
+    this.db_.removeConnection(this, this.y_);
  }
  Blockly.RenderedConnection.superClass_.dispose.call(this);
 };
@@ -158,16 +154,12 @@ Blockly.RenderedConnection.prototype.bumpAwayFrom_ = function(staticConnection)
 * @param {number} y New absolute y coordinate, in workspace coordinates.
 */
 Blockly.RenderedConnection.prototype.moveTo = function(x, y) {
-  // Remove it from its old location in the database (if already present)
-  if (this.inDB_) {
-    this.db_.removeConnection_(this);
+  if (this.tracked_) {
+    this.db_.removeConnection(this, this.y_);
+    this.db_.addConnection(this, y);
  }
  this.x_ = x;
  this.y_ = y;
-  // Insert it into its new location in the database.
-  if (!this.hidden_) {
-    this.db_.addConnection(this);
-  }
 };

 /**
@@ -282,17 +274,73 @@ Blockly.RenderedConnection.prototype.highlight = function() {
 };

 /**
- * Unhide this connection, as well as all down-stream connections on any block
- * attached to this connection.  This happens when a block is expanded.
- * Also unhides down-stream comments.
+ * Remove the highlighting around this connection.
+ */
+Blockly.RenderedConnection.prototype.unhighlight = function() {
+  Blockly.utils.dom.removeNode(Blockly.Connection.highlightedPath_);
+  delete Blockly.Connection.highlightedPath_;
+};
+
+/**
+ * Set whether this connections is tracked in the database or not.
+ * @param {boolean} doTracking If true, start tracking. If false, stop tracking.
+ * @package
+ */
+Blockly.RenderedConnection.prototype.setTracking = function(doTracking) {
+  if (doTracking == this.tracked_) {
+    return;
+  }
+  if (this.sourceBlock_.isInFlyout) {
+    // Don't bother maintaining a database of connections in a flyout.
+    return;
+  }
+  if (doTracking) {
+    this.db_.addConnection(this, this.y_);
+  } else {
+    this.db_.removeConnection(this, this.y_);
+  }
+  this.tracked_ = doTracking;
+};
+
+/**
+ * Stop tracking this connection, as well as all down-stream connections on
+ * any block attached to this connection. This happens when a block is
+ * collapsed.
+ *
+ * Also closes down-stream icons/bubbles.
+ * @package
+ */
+Blockly.RenderedConnection.prototype.stopTrackingAll = function() {
+  this.setTracking(false);
+  if (this.targetConnection) {
+    var blocks = this.targetBlock().getDescendants(false);
+    for (var i = 0; i < blocks.length; i++) {
+      var block = blocks[i];
+      // Stop tracking connections of all children.
+      var connections = block.getConnections_(true);
+      for (var j = 0; j < connections.length; j++) {
+        connections[j].setTracking(false);
+      }
+      // Close all bubbles of all children.
+      var icons = block.getIcons();
+      for (var j = 0; j < icons.length; j++) {
+        icons[j].setVisible(false);
+      }
+    }
+  }
+};
+
+/**
+ * Start tracking this connection, as well as all down-stream connections on
+ * any block attached to this connection. This happens when a block is expanded.
 * @return {!Array.<!Blockly.Block>} List of blocks to render.
 */
-Blockly.RenderedConnection.prototype.unhideAll = function() {
-  this.setHidden(false);
-  // All blocks that need unhiding must be unhidden before any rendering takes
-  // place, since rendering requires knowing the dimensions of lower blocks.
-  // Also, since rendering a block renders all its parents, we only need to
-  // render the leaf nodes.
+Blockly.RenderedConnection.prototype.startTrackingAll = function() {
+  this.setTracking(true);
+  // All blocks that are not tracked must start tracking before any
+  // rendering takes place, since rendering requires knowing the dimensions
+  // of lower blocks. Also, since rendering a block renders all its parents,
+  // we only need to render the leaf nodes.
  var renderList = [];
  if (this.type != Blockly.INPUT_VALUE && this.type != Blockly.NEXT_STATEMENT) {
    // Only spider down.
@@ -312,7 +360,7 @@ Blockly.RenderedConnection.prototype.unhideAll = function() {
      connections = block.getConnections_(true);
    }
    for (var i = 0; i < connections.length; i++) {
-      renderList.push.apply(renderList, connections[i].unhideAll());
+      renderList.push.apply(renderList, connections[i].startTrackingAll());
    }
    if (!renderList.length) {
      // Leaf block.
@@ -322,52 +370,6 @@ Blockly.RenderedConnection.prototype.unhideAll = function() {
  return renderList;
 };

-/**
- * Remove the highlighting around this connection.
- */
-Blockly.RenderedConnection.prototype.unhighlight = function() {
-  Blockly.utils.dom.removeNode(Blockly.Connection.highlightedPath_);
-  delete Blockly.Connection.highlightedPath_;
-};
-
-/**
- * Set whether this connections is hidden (not tracked in a database) or not.
- * @param {boolean} hidden True if connection is hidden.
- */
-Blockly.RenderedConnection.prototype.setHidden = function(hidden) {
-  this.hidden_ = hidden;
-  if (hidden && this.inDB_) {
-    this.db_.removeConnection_(this);
-  } else if (!hidden && !this.inDB_) {
-    this.db_.addConnection(this);
-  }
-};
-
-/**
- * Hide this connection, as well as all down-stream connections on any block
- * attached to this connection.  This happens when a block is collapsed.
- * Also hides down-stream comments.
- */
-Blockly.RenderedConnection.prototype.hideAll = function() {
-  this.setHidden(true);
-  if (this.targetConnection) {
-    var blocks = this.targetBlock().getDescendants(false);
-    for (var i = 0; i < blocks.length; i++) {
-      var block = blocks[i];
-      // Hide all connections of all children.
-      var connections = block.getConnections_(true);
-      for (var j = 0; j < connections.length; j++) {
-        connections[j].setHidden(true);
-      }
-      // Close all bubbles of all children.
-      var icons = block.getIcons();
-      for (var j = 0; j < icons.length; j++) {
-        icons[j].setVisible(false);
-      }
-    }
-  }
-};
-
 /**
 * Check if the two connections can be dragged to connect to each other.
 * @param {!Blockly.Connection} candidate A nearby connection to check.
--- a/core/xml.js
+++ b/core/xml.js
@@ -545,8 +545,8 @@ Blockly.Xml.domToBlock = function(xmlBlock, workspace) {
    // Generate list of all blocks.
    var blocks = topBlock.getDescendants(false);
    if (workspace.rendered) {
-      // Hide connections to speed up assembly.
-      topBlock.setConnectionsHidden(true);
+      // Wait to track connections to speed up assembly.
+      topBlock.waitToTrackConnections();
      // Render each block.
      for (var i = blocks.length - 1; i >= 0; i--) {
        blocks[i].initSvg();
@@ -557,8 +557,8 @@ Blockly.Xml.domToBlock = function(xmlBlock, workspace) {
      // Populating the connection database may be deferred until after the
      // blocks have rendered.
      setTimeout(function() {
-        if (topBlock.workspace) {  // Check that the block hasn't been deleted.
-          topBlock.setConnectionsHidden(false);
+        if (!topBlock.disposed) {
+          topBlock.startTrackingConnections();
        }
      }, 1);
      topBlock.updateDisabled();
--- a/tests/mocha/connection_db_test.js
+++ b/tests/mocha/connection_db_test.js
@@ -44,10 +44,7 @@ suite('Connection Database', function() {
      }
    };
  });
-  // TODO: Re-enable once flyout checking is handled by the connection
-  //  (better yet - let it be handled by the flyout, but that's out of the
-  //  scope of this).
-  test.skip('Add Connection', function() {
+  test('Add Connection', function() {
    var y2 = {y_: 2};
    var y4 = {y_: 4};
    var y1 = {y_: 1};
@@ -75,7 +72,7 @@ suite('Connection Database', function() {
        this.database.connections_, [y1, y2, y3b, y3a, y4]);

  });
-  test.skip('Remove Connection', function() {
+  test('Remove Connection', function() {
    var y2 = {y_: 2};
    var y4 = {y_: 4};
    var y1 = {y_: 1};