From bb8348befd4be91e97e4dd51059a868020d559e3 Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Thu, 9 Jul 2020 16:56:48 -0700 Subject: [PATCH] Move to a single canConnect function, and update tests --- blocks/logic.js | 11 ++-- core/connection.js | 17 +++--- core/connection_checks.js | 94 +++++++++++++++++++------------ core/connection_db.js | 15 +++-- core/keyboard_nav/navigation.js | 10 ++-- tests/mocha/connection_db_test.js | 21 ++----- tests/mocha/index.html | 1 - 7 files changed, 89 insertions(+), 80 deletions(-) diff --git a/blocks/logic.js b/blocks/logic.js index c0900fdb5..6cea9d1d4 100644 --- a/blocks/logic.js +++ b/blocks/logic.js @@ -543,9 +543,9 @@ Blockly.Constants.Logic.LOGIC_COMPARE_ONCHANGE_MIXIN = { var blockB = this.getInputTargetBlock('B'); // Disconnect blocks that existed prior to this change if they don't match. if (blockA && blockB && - !this.workspace.connectionTypeChecker.checkType( - blockA.outputConnection, blockB.outputConnection)) { - //!blockA.outputConnection.checkType(blockB.outputConnection)) { + //!this.workspace.connectionTypeChecker.checkType( + //blockA.outputConnection, blockB.outputConnection)) { + !blockA.outputConnection.checkType(blockB.outputConnection)) { // Mismatch between two inputs. Revert the block connections, // bumping away the newly connected block(s). Blockly.Events.setGroup(e.group); @@ -613,8 +613,9 @@ Blockly.Constants.Logic.LOGIC_TERNARY_ONCHANGE_MIXIN = { for (var i = 0; i < 2; i++) { var block = (i == 1) ? blockA : blockB; if (block && - block.workspace.connectionTypeChecker.checkType(block.outputConnection, parentConnection)) { - //!block.outputConnection.checkType(parentConnection)) { + // !block.workspace.connectionTypeChecker.checkType( + // block.outputConnection, parentConnection)) { + !block.outputConnection.checkType(parentConnection)) { // Ensure that any disconnections are grouped with the causing event. Blockly.Events.setGroup(e.group); if (parentConnection === this.prevParentConnection_) { diff --git a/core/connection.js b/core/connection.js index 9e769a1a9..a3597de03 100644 --- a/core/connection.js +++ b/core/connection.js @@ -47,6 +47,7 @@ Blockly.Connection.REASON_TARGET_NULL = 3; Blockly.Connection.REASON_CHECKS_FAILED = 4; Blockly.Connection.REASON_DIFFERENT_WORKSPACES = 5; Blockly.Connection.REASON_SHADOW_PARENT = 6; +Blockly.Connection.REASON_DRAG_CHECKS_FAILED = 7; /** * Connection this connection connects to. Null if not connected. @@ -250,6 +251,7 @@ Blockly.Connection.prototype.isConnected = function() { * an error code otherwise. */ Blockly.Connection.prototype.canConnectWithReason = function(target) { + // TODO: deprecation warning with date, plus tests. return this.getConnectionTypeChecker().canConnectWithReason(this, target); }; @@ -261,12 +263,10 @@ Blockly.Connection.prototype.canConnectWithReason = function(target) { * @package */ Blockly.Connection.prototype.checkConnection = function(target) { - +// TODO: Add deprecation warning notices *and* add tests to make sure these +// still work (for any blocks that use them). var checker = this.getConnectionTypeChecker(); - var reason = checker.canConnectWithReason(this, target); - if (reason != Blockly.Connection.CAN_CONNECT) { - throw Error(checker.getErrorMessage(reason, this, target)); - } + checker.canConnect(this, target, false, true); }; Blockly.Connection.prototype.getConnectionTypeChecker = function() { @@ -315,7 +315,7 @@ Blockly.Connection.prototype.getConnectionTypeChecker = function() { * @return {boolean} True if the connection is allowed, false otherwise. */ Blockly.Connection.prototype.isConnectionAllowed = function(candidate) { - return this.getConnectionTypeChecker().isConnectionAllowed(this, candidate); + return this.getConnectionTypeChecker().canConnect(this, candidate, true, false); }; /** @@ -339,10 +339,7 @@ Blockly.Connection.prototype.connect = function(otherConnection) { } var checker = this.getConnectionTypeChecker(); - var reason = checker.canConnectWithReason(this, otherConnection); - if (reason != Blockly.Connection.CAN_CONNECT) { - throw Error(checker.getErrorMessage(reason, this, otherConnection)); - } + checker.canConnect(this, otherConnection, false, true); var eventGroup = Blockly.Events.getGroup(); if (!eventGroup) { diff --git a/core/connection_checks.js b/core/connection_checks.js index ade3ef949..7e51f62cb 100644 --- a/core/connection_checks.js +++ b/core/connection_checks.js @@ -9,7 +9,6 @@ goog.requireType('Blockly.Connection'); * @constructor */ Blockly.ConnectionTypeChecker = function() { - }; /** @@ -39,6 +38,8 @@ Blockly.ConnectionTypeChecker.prototype.getErrorMessage = function(errorCode, return msg; case Blockly.Connection.REASON_SHADOW_PARENT: return 'Connecting non-shadow to shadow block.'; + case Blockly.Connection.REASON_DRAG_CHECKS_FAILED: + return 'Drag checks failed.' default: return 'Unknown connection failure: this should never happen!'; } @@ -53,6 +54,17 @@ Blockly.ConnectionTypeChecker.prototype.getErrorMessage = function(errorCode, * an error code otherwise. */ Blockly.ConnectionTypeChecker.prototype.canConnectWithReason = function(one, two) { + var validity = this.doValidityChecks(one, two); + if (validity != Blockly.Connection.CAN_CONNECT) { + return validity; + } + if (!this.checkType(one, two)) { + return Blockly.Connection.REASON_CHECKS_FAILED; + } + return Blockly.Connection.CAN_CONNECT; +}; + +Blockly.ConnectionTypeChecker.prototype.doValidityChecks = function(one, two) { if (!one || !two) { return Blockly.Connection.REASON_TARGET_NULL; } @@ -65,20 +77,58 @@ Blockly.ConnectionTypeChecker.prototype.canConnectWithReason = function(one, two } // TODO (fenichel): The null checks seem like they're only for making tests // work better. - if (blockA && blockA == blockB) { + if (blockA == blockB) { return Blockly.Connection.REASON_SELF_CONNECTION; } else if (two.type != Blockly.OPPOSITE_TYPE[one.type]) { return Blockly.Connection.REASON_WRONG_TYPE; - } else if (blockA && blockB && blockA.workspace !== blockB.workspace) { + } else if (blockA.workspace !== blockB.workspace) { return Blockly.Connection.REASON_DIFFERENT_WORKSPACES; } else if (blockA.isShadow() && !blockB.isShadow()) { return Blockly.Connection.REASON_SHADOW_PARENT; - } else if (!this.checkType(one, two)) { - return Blockly.Connection.REASON_CHECKS_FAILED; } return Blockly.Connection.CAN_CONNECT; }; +/** + * Checks whether the current connection can connect with the target + * connection. + * @param {Blockly.Connection} one Connection to check compatibility with. + * @param {Blockly.Connection} two Connection to check compatibility with. + * @return {boolean} Whether the connection is legal. + */ +Blockly.ConnectionTypeChecker.prototype.canConnect = function(one, two, + isDragging, shouldThrow) { + var validity = this.doValidityChecks(one, two); + if (validity != Blockly.Connection.CAN_CONNECT) { + if (shouldThrow) { + throw Error(this.getErrorMessage(validity, one, two)); + } + return false; + } + + var passesTypeChecks = this.checkType(one, two); + if (!passesTypeChecks) { + + if (shouldThrow) { + throw Error(this.getErrorMessage( + Blockly.Connection.REASON_CHECKS_FAILED, one, two)); + } + return false; + } + + if (isDragging) { + var passesDragChecks = this.passesDragChecks(one, two); + if (!passesDragChecks) { + if (shouldThrow) { + throw Error(this.getErrorMessage( + Blockly.Connection.REASON_DRAG_CHECKS_FAILED, one, two)); + } + return false; + } + } + return true; +}; + /** * Is this connection compatible with another connection with respect to the * value type system. E.g. square_root("Hello") is not compatible. @@ -104,23 +154,11 @@ Blockly.ConnectionTypeChecker.prototype.checkType = function(one, two) { return false; }; - -/** - * Check if the two connections can be dragged to connect to each other. - * @param {!Blockly.Connection} one The moving connection to check. - * @param {!Blockly.Connection} two A stationary connection to check. - * @return {boolean} True if the connection is allowed, false otherwise. - */ -Blockly.ConnectionTypeChecker.prototype.isConnectionAllowed = function(one, two) { +Blockly.ConnectionTypeChecker.prototype.passesDragChecks = function(one, two) { // Don't consider insertion markers. if (two.sourceBlock_.isInsertionMarker()) { return false; } - // Type checking. - var canConnect = one.canConnectWithReason(two); - if (canConnect != Blockly.Connection.CAN_CONNECT) { - return false; - } switch (two.type) { case Blockly.PREVIOUS_STATEMENT: @@ -160,7 +198,7 @@ Blockly.ConnectionTypeChecker.prototype.isConnectionAllowed = function(one, two) break; } default: - throw Error('Unknown connection type in isConnectionAllowed'); + throw Error('Unknown connection type in passesDragChecks'); } // Don't let blocks try to connect to themselves or ones they nest. @@ -171,24 +209,6 @@ Blockly.ConnectionTypeChecker.prototype.isConnectionAllowed = function(one, two) return true; }; -/** - * Check if the two connections can be dragged to connect to each other. - * @param {!Blockly.RenderedConnection} dragging The connection to check on a - * block that is being dragged. - * @param {!Blockly.RenderedConnection} candidate The stationary connection to - * check. - * @param {number=} maxRadius The maximum radius allowed for connections, in - * workspace units. - * @return {boolean} True if the connection is allowed, false otherwise. - */ -Blockly.ConnectionTypeChecker.prototype.canConnectDuringDrag = function( - dragging, candidate, maxRadius) { - if (dragging.distanceFrom(candidate) > maxRadius) { - return false; - } - - return this.isConnectionAllowed(dragging, candidate); -}; /** * Check if the two connections can be dragged to connect to each other. * This is used by the connection database when searching for the closest diff --git a/core/connection_db.js b/core/connection_db.js index 5a8283f12..ff31704a2 100644 --- a/core/connection_db.js +++ b/core/connection_db.js @@ -238,15 +238,17 @@ Blockly.ConnectionDB.prototype.searchForClosest = function(conn, maxRadius, var bestConnection = null; var bestRadius = maxRadius; var temp; + var curDistance; // Walk forward and back on the y axis looking for the closest x,y point. var pointerMin = closestIndex - 1; while (pointerMin >= 0 && this.isInYRange_(pointerMin, conn.y, maxRadius)) { temp = this.connections_[pointerMin]; - if (this.typeChecker_.canConnectDuringDrag(conn, temp, bestRadius)) { - //conn.isConnectionAllowed(temp, bestRadius)) { + curDistance = temp.distanceFrom(conn); + if (curDistance <= bestRadius && + this.typeChecker_.canConnect(conn, temp, true, false)) { bestConnection = temp; - bestRadius = temp.distanceFrom(conn); + bestRadius = curDistance; } pointerMin--; } @@ -255,10 +257,11 @@ Blockly.ConnectionDB.prototype.searchForClosest = function(conn, maxRadius, while (pointerMax < this.connections_.length && this.isInYRange_(pointerMax, conn.y, maxRadius)) { temp = this.connections_[pointerMax]; - if (this.typeChecker_.canConnectDuringDrag(conn, temp, bestRadius)) { - //conn.isConnectionAllowed(temp, bestRadius)) { + curDistance = temp.distanceFrom(conn); + if (curDistance <= bestRadius && + this.typeChecker_.canConnect(conn, temp, true, false)) { bestConnection = temp; - bestRadius = temp.distanceFrom(conn); + bestRadius = curDistance; } pointerMax++; } diff --git a/core/keyboard_nav/navigation.js b/core/keyboard_nav/navigation.js index 9a59ee5c3..5a06690dc 100644 --- a/core/keyboard_nav/navigation.js +++ b/core/keyboard_nav/navigation.js @@ -411,9 +411,9 @@ Blockly.navigation.moveAndConnect_ = function(movingConnection, destConnection) } var movingBlock = movingConnection.getSourceBlock(); - if (destConnection.canConnectWithReason(movingConnection) == - Blockly.Connection.CAN_CONNECT) { + var checker = movingConnection.getConnectionTypeChecker(); + if (checker.canConnect(movingConnection, destConnection, false, false)) { Blockly.navigation.disconnectChild_(movingConnection, destConnection); if (!destConnection.isSuperior()) { @@ -502,10 +502,12 @@ Blockly.navigation.connect_ = function(movingConnection, destConnection) { return true; } else { try { - destConnection.checkConnection(movingConnection); + var checker = movingConnection.getConnectionTypeChecker(); + checker.canConnect(movingConnection, destConnection, false, true); } catch (e) { - // If nothing worked report the error from the original connections. + // If nothing worked there should be an error. + // Report the error from the original connections. Blockly.navigation.warn_('Connection failed with error: ' + e); } return false; diff --git a/tests/mocha/connection_db_test.js b/tests/mocha/connection_db_test.js index 34165831e..52f926391 100644 --- a/tests/mocha/connection_db_test.js +++ b/tests/mocha/connection_db_test.js @@ -194,26 +194,13 @@ suite('Connection Database', function() { suite('Search For Closest', function() { setup(function() { this.allowedStub = null; - this.database.typeChecker_.canConnectDuringDrag = function( - dragging, candidate, maxRadius) { - if (dragging.distanceFrom(candidate) > maxRadius) { - return false; - } - // Ignore non-distance parameters. - return true; - }; - + this.allowedStub = sinon.stub(this.database.typeChecker_, 'canConnect') + .callsFake(function(dragging, candidate) { + return true; + }); this.createCheckConnection = function(x, y) { var checkConnection = this.createConnection(x, y, Blockly.NEXT_STATEMENT, new Blockly.ConnectionDB()); - // this.allowedStub = sinon.stub(checkConnection, 'isConnectionAllowed') - // .callsFake(function(candidate, maxRadius) { - // if (this.distanceFrom(candidate) > maxRadius) { - // return false; - // } - // // Ignore non-distance parameters. - // return true; - // }); return checkConnection; }; }); diff --git a/tests/mocha/index.html b/tests/mocha/index.html index bc236ad93..9c81f3889 100644 --- a/tests/mocha/index.html +++ b/tests/mocha/index.html @@ -41,7 +41,6 @@ -