From 5ffd43824ff68ecf655d8b4485684b7ea41a3fb4 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Wed, 26 May 2021 09:16:33 -0700 Subject: [PATCH] Change lastConnectionInRow to getPlaceForOrphanedOutput - Take 2 (#4851) * Change lastConnectionInRow to getPlaceForOrphanedOutput * PR comments --- core/connection.js | 54 ++--- core/renderers/common/renderer.js | 14 +- tests/mocha/connection_test.js | 326 +++++++++++++++++++++++++++++- 3 files changed, 355 insertions(+), 39 deletions(-) diff --git a/core/connection.js b/core/connection.js index 3ca9081e0..fcf214502 100644 --- a/core/connection.js +++ b/core/connection.js @@ -134,7 +134,7 @@ Blockly.Connection.prototype.connect_ = function(childConnection) { // Attempt to reattach the orphan at the end of the newly inserted // block. Since this block may be a row, walk down to the end // or to the first (and only) shadow block. - var connection = Blockly.Connection.lastConnectionInRow( + var connection = Blockly.Connection.getConnectionForOrphanedOutput( childBlock, orphanBlock); if (connection) { orphanBlock.outputConnection.connect(connection); @@ -372,30 +372,31 @@ Blockly.Connection.connectReciprocally_ = function(first, second) { }; /** - * Does the given block have one and only one connection point that will accept - * an orphaned block? + * Returns the single connection on the block that will accept the orphaned + * block, if one can be found. If the block has multiple compatible connections + * (even if they are filled) this returns null. If the block has no compatible + * connections, this returns null. * @param {!Blockly.Block} block The superior block. * @param {!Blockly.Block} orphanBlock The inferior block. * @return {Blockly.Connection} The suitable connection point on 'block', * or null. * @private */ -Blockly.Connection.singleConnection_ = function(block, orphanBlock) { - var connection = null; +Blockly.Connection.getSingleConnection_ = function(block, orphanBlock) { + var foundConnection = null; var output = orphanBlock.outputConnection; - for (var i = 0; i < block.inputList.length; i++) { - var thisConnection = block.inputList[i].connection; - var typeChecker = output.getConnectionChecker(); - if (thisConnection && - thisConnection.type == Blockly.connectionTypes.INPUT_VALUE && - typeChecker.canConnect(output, thisConnection, false)) { - if (connection) { + var typeChecker = output.getConnectionChecker(); + + for (var i = 0, input; (input = block.inputList[i]); i++) { + var connection = input.connection; + if (connection && typeChecker.canConnect(output, connection, false)) { + if (foundConnection) { return null; // More than one connection. } - connection = thisConnection; + foundConnection = connection; } } - return connection; + return foundConnection; }; /** @@ -410,18 +411,19 @@ Blockly.Connection.singleConnection_ = function(block, orphanBlock) { * of blocks, or null. * @package */ -Blockly.Connection.lastConnectionInRow = function(startBlock, orphanBlock) { - var newBlock = startBlock; - var connection; - while ((connection = Blockly.Connection.singleConnection_( - /** @type {!Blockly.Block} */ (newBlock), orphanBlock))) { - newBlock = connection.targetBlock(); - if (!newBlock || newBlock.isShadow()) { - return connection; - } - } - return null; -}; +Blockly.Connection.getConnectionForOrphanedOutput = + function(startBlock, orphanBlock) { + var newBlock = startBlock; + var connection; + while ((connection = Blockly.Connection.getSingleConnection_( + /** @type {!Blockly.Block} */ (newBlock), orphanBlock))) { + newBlock = connection.targetBlock(); + if (!newBlock || newBlock.isShadow()) { + return connection; + } + } + return null; + }; /** * Disconnect this connection. diff --git a/core/renderers/common/renderer.js b/core/renderers/common/renderer.js index af575403f..4d7c6ca17 100644 --- a/core/renderers/common/renderer.js +++ b/core/renderers/common/renderer.js @@ -248,19 +248,13 @@ Blockly.blockRendering.Renderer.prototype.orphanCanConnectAtEnd = function(topBlock, orphanBlock, localType) { var orphanConnection = null; var lastConnection = null; - if (localType == - Blockly.connectionTypes - .OUTPUT_VALUE) { // We are replacing an output. + if (localType == Blockly.connectionTypes.OUTPUT_VALUE) { orphanConnection = orphanBlock.outputConnection; - // TODO: I don't think this function necessarily has the correct logic, - // but for now it is being kept for behavioral backwards-compat. - lastConnection = Blockly.Connection - .lastConnectionInRow( + lastConnection = + Blockly.Connection.getConnectionForOrphanedOutput( /** @type {!Blockly.Block} **/ (topBlock), orphanBlock); - } else { // We are replacing a previous. + } else { orphanConnection = orphanBlock.previousConnection; - // TODO: This lives on the block while lastConnectionInRow lives on - // on the connection. Something is fishy. lastConnection = topBlock.lastConnectionInStack(); } diff --git a/tests/mocha/connection_test.js b/tests/mocha/connection_test.js index 73882b90b..3d96c18c5 100644 --- a/tests/mocha/connection_test.js +++ b/tests/mocha/connection_test.js @@ -156,9 +156,6 @@ suite('Connection', function() { teardown(function() { workspaceTeardown.call(this, this.workspace); - delete Blockly.Blocks['stack_block']; - delete Blockly.Blocks['row_block']; - delete Blockly.Blocks['statement_block']; }); suite('Add - No Block Connected', function() { @@ -784,4 +781,327 @@ suite('Connection', function() { }); }); }); + + suite('getConnectionForOrphanedOutput', function() { + setup(function() { + this.workspace = new Blockly.Workspace(); + + Blockly.defineBlocksWithJsonArray([ + { + 'type': 'input', + 'message0': '%1', + 'args0': [ + { + 'type': 'input_value', + 'name': 'INPUT', + 'check': 'check' + } + ], + }, + { + 'type': 'output', + 'message0': '', + 'output': 'check', + }, + ]); + }); + + teardown(function() { + workspaceTeardown.call(this, this.workspace); + }); + + suite('No available spots', function() { + setup(function() { + Blockly.defineBlocksWithJsonArray([ + { + 'type': 'output_and_statements', + 'message0': '%1 %2', + 'args0': [ + { + 'type': 'input_statement', + 'name': 'INPUT', + 'check': 'check' + }, + { + 'type': 'input_statement', + 'name': 'INPUT2', + 'check': 'check' + } + ], + 'output': 'check', + }, + { + 'type': 'output_and_inputs', + 'message0': '%1 %2', + 'args0': [ + { + 'type': 'input_value', + 'name': 'INPUT', + 'check': 'check2' + }, + { + 'type': 'input_value', + 'name': 'INPUT2', + 'check': 'check2' + } + ], + 'output': 'check', + }, + { + 'type': 'check_to_check2', + 'message0': '%1', + 'args0': [ + { + 'type': 'input_value', + 'name': 'INPUT', + 'check': 'check2' + }, + ], + 'output': 'check', + }, + { + 'type': 'check2_to_check', + 'message0': '%1', + 'args0': [ + { + 'type': 'input_value', + 'name': 'CHECK2TOCHECKINPUT', + 'check': 'check' + }, + ], + 'output': 'check2', + }, + ]); + }); + + test('No connection', function() { + const parent = this.workspace.newBlock('input'); + const oldChild = this.workspace.newBlock('output'); + const newChild = this.workspace.newBlock('output'); + + parent.getInput('INPUT').connection.connect(oldChild.outputConnection); + chai.assert.notExists( + Blockly.Connection.getConnectionForOrphanedOutput(oldChild, newChild)); + }); + + test('All statements', function() { + const parent = this.workspace.newBlock('input'); + const oldChild = this.workspace.newBlock('output_and_statements'); + const newChild = this.workspace.newBlock('output'); + + parent.getInput('INPUT').connection.connect(oldChild.outputConnection); + chai.assert.notExists( + Blockly.Connection.getConnectionForOrphanedOutput(oldChild, newChild)); + }); + + test('Bad checks', function() { + const parent = this.workspace.newBlock('input'); + const oldChild = this.workspace.newBlock('output_and_inputs'); + const newChild = this.workspace.newBlock('output'); + + parent.getInput('INPUT').connection.connect(oldChild.outputConnection); + chai.assert.notExists( + Blockly.Connection.getConnectionForOrphanedOutput(oldChild, newChild)); + }); + + test('Through different types', function() { + const parent = this.workspace.newBlock('input'); + const oldChild = this.workspace.newBlock('check_to_check2'); + const otherChild = this.workspace.newBlock('check2_to_check'); + const newChild = this.workspace.newBlock('output'); + + parent.getInput('INPUT').connection + .connect(oldChild.outputConnection); + oldChild.getInput('INPUT').connection + .connect(otherChild.outputConnection); + + chai.assert.notExists( + Blockly.Connection.getConnectionForOrphanedOutput(oldChild, newChild)); + }); + }); + + suite('Multiple available spots', function() { + setup(function() { + Blockly.defineBlocksWithJsonArray([ + { + 'type': 'multiple_inputs', + 'message0': '%1 %2', + 'args0': [ + { + 'type': 'input_value', + 'name': 'INPUT', + 'check': 'check' + }, + { + 'type': 'input_value', + 'name': 'INPUT2', + 'check': 'check' + }, + ], + 'output': 'check', + }, + { + 'type': 'single_input', + 'message0': '%1', + 'args0': [ + { + 'type': 'input_value', + 'name': 'INPUT', + 'check': 'check' + }, + ], + 'output': 'check', + }, + ]); + }); + + suite('No shadows', function() { + test('Top block', function() { + const parent = this.workspace.newBlock('input'); + const oldChild = this.workspace.newBlock('multiple_inputs'); + const newChild = this.workspace.newBlock('output'); + + parent.getInput('INPUT').connection.connect(oldChild.outputConnection); + chai.assert.notExists( + Blockly.Connection.getConnectionForOrphanedOutput(oldChild, newChild)); + }); + + test('Child blocks', function() { + const parent = this.workspace.newBlock('input'); + const oldChild = this.workspace.newBlock('multiple_inputs'); + const childX = this.workspace.newBlock('single_input'); + const childY = this.workspace.newBlock('single_input'); + const newChild = this.workspace.newBlock('output'); + + parent.getInput('INPUT').connection.connect(oldChild.outputConnection); + oldChild.getInput('INPUT').connection.connect(childX.outputConnection); + oldChild.getInput('INPUT2').connection.connect(childY.outputConnection); + + chai.assert.notExists( + Blockly.Connection.getConnectionForOrphanedOutput(oldChild, newChild)); + }); + + test('Spots filled', function() { + const parent = this.workspace.newBlock('input'); + const oldChild = this.workspace.newBlock('multiple_inputs'); + const otherChild = this.workspace.newBlock('output'); + const newChild = this.workspace.newBlock('output'); + + parent.getInput('INPUT').connection + .connect(oldChild.outputConnection); + oldChild.getInput('INPUT').connection + .connect(otherChild.outputConnection); + + chai.assert.notExists( + Blockly.Connection.getConnectionForOrphanedOutput(oldChild, newChild)); + }); + }); + + suite('Shadows', function() { + test('Top block', function() { + const parent = this.workspace.newBlock('input'); + const oldChild = this.workspace.newBlock('multiple_inputs'); + const newChild = this.workspace.newBlock('output'); + + parent.getInput('INPUT').connection.connect(oldChild.outputConnection); + oldChild.getInput('INPUT').connection.setShadowDom( + Blockly.Xml.textToDom('') + .firstChild); + oldChild.getInput('INPUT2').connection.setShadowDom( + Blockly.Xml.textToDom('') + .firstChild); + + chai.assert.notExists( + Blockly.Connection.getConnectionForOrphanedOutput(oldChild, newChild)); + }); + + test('Child blocks', function() { + const parent = this.workspace.newBlock('input'); + const oldChild = this.workspace.newBlock('multiple_inputs'); + const childX = this.workspace.newBlock('single_input'); + const childY = this.workspace.newBlock('single_input'); + const newChild = this.workspace.newBlock('output'); + + parent.getInput('INPUT').connection.connect(oldChild.outputConnection); + oldChild.getInput('INPUT').connection.connect(childX.outputConnection); + oldChild.getInput('INPUT2').connection.connect(childY.outputConnection); + childX.getInput('INPUT').connection.setShadowDom( + Blockly.Xml.textToDom('') + .firstChild); + childY.getInput('INPUT').connection.setShadowDom( + Blockly.Xml.textToDom('') + .firstChild); + + chai.assert.notExists( + Blockly.Connection.getConnectionForOrphanedOutput(oldChild, newChild)); + }); + + test('Spots filled', function() { + const parent = this.workspace.newBlock('input'); + const oldChild = this.workspace.newBlock('multiple_inputs'); + const otherChild = this.workspace.newBlock('output'); + const newChild = this.workspace.newBlock('output'); + + parent.getInput('INPUT').connection + .connect(oldChild.outputConnection); + oldChild.getInput('INPUT').connection + .connect(otherChild.outputConnection); + oldChild.getInput('INPUT2').connection.setShadowDom( + Blockly.Xml.textToDom('') + .firstChild); + + chai.assert.notExists( + Blockly.Connection.getConnectionForOrphanedOutput(oldChild, newChild)); + }); + }); + }); + + suite('Single available spot', function() { + setup(function() { + Blockly.defineBlocksWithJsonArray([ + { + 'type': 'single_input', + 'message0': '%1', + 'args0': [ + { + 'type': 'input_value', + 'name': 'INPUT', + 'check': 'check' + }, + ], + 'output': 'check', + }, + ]); + }); + + test('No shadows', function() { + const parent = this.workspace.newBlock('input'); + const oldChild = this.workspace.newBlock('single_input'); + const newChild = this.workspace.newBlock('output'); + + parent.getInput('INPUT').connection.connect(oldChild.outputConnection); + + const result = Blockly.Connection + .getConnectionForOrphanedOutput(oldChild, newChild); + chai.assert.exists(result); + chai.assert.equal(result.getParentInput().name, 'INPUT'); + }); + + test('Shadows', function() { + const parent = this.workspace.newBlock('input'); + const oldChild = this.workspace.newBlock('single_input'); + const newChild = this.workspace.newBlock('output'); + + parent.getInput('INPUT').connection.connect(oldChild.outputConnection); + oldChild.getInput('INPUT').connection.setShadowDom( + Blockly.Xml.textToDom('') + .firstChild); + + const result = Blockly.Connection + .getConnectionForOrphanedOutput(oldChild, newChild); + chai.assert.exists(result); + chai.assert.equal(result.getParentInput().name, 'INPUT'); + }); + }); + }); });