From e95f503decb6692108e3e8373f1b14f9aa516e16 Mon Sep 17 00:00:00 2001 From: Matt Hillsdon Date: Thu, 13 Mar 2025 16:54:27 +0000 Subject: [PATCH 1/2] chore: simplify tryToConnectNodes We only insert blocks so remove other cases. --- src/actions/clipboard.ts | 7 +-- src/actions/enter.ts | 9 +--- src/navigation.ts | 114 ++++++++++----------------------------- 3 files changed, 30 insertions(+), 100 deletions(-) diff --git a/src/actions/clipboard.ts b/src/actions/clipboard.ts index 6088c2af..7280444a 100644 --- a/src/actions/clipboard.ts +++ b/src/actions/clipboard.ts @@ -376,12 +376,7 @@ export class Clipboard { const block = clipboard.paste(this.copyData, pasteWorkspace) as BlockSvg; if (block) { if (targetNode) { - this.navigation.tryToConnectNodes( - pasteWorkspace, - targetNode, - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - ASTNode.createBlockNode(block)!, - ); + this.navigation.tryToConnectBlock(targetNode, block); } Events.setGroup(false); return true; diff --git a/src/actions/enter.ts b/src/actions/enter.ts index 9071fa54..81bdda31 100644 --- a/src/actions/enter.ts +++ b/src/actions/enter.ts @@ -130,14 +130,7 @@ export class EnterAction { const newBlock = this.createNewBlock(workspace); if (!newBlock) return; if (stationaryNode) { - if ( - !this.navigation.tryToConnectNodes( - workspace, - stationaryNode, - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - ASTNode.createBlockNode(newBlock)!, - ) - ) { + if (!this.navigation.tryToConnectBlock(stationaryNode, newBlock)) { console.warn( 'Something went wrong while inserting a block from the flyout.', ); diff --git a/src/navigation.ts b/src/navigation.ts index 29cf84cb..f6da432f 100644 --- a/src/navigation.ts +++ b/src/navigation.ts @@ -672,61 +672,52 @@ export class Navigation { } /** +<<<<<<< HEAD +======= + * Hides the flyout cursor and optionally hides the flyout. + * + * @param workspace The workspace. + * @param shouldHide True if the flyout should be hidden. + */ + resetFlyout(workspace: Blockly.WorkspaceSvg, shouldHide: boolean) { + if (this.getFlyoutCursor(workspace)) { + this.getFlyoutCursor(workspace)!.hide(); + if (shouldHide) { + workspace.getFlyout()!.hide(); + } + } + } + + /** +>>>>>>> f14f250 (chore: simplify tryToConnectNodes) * Tries to intelligently connect the blocks or connections * represented by the given nodes, based on node types and locations. * * @param workspace The main workspace. * @param stationaryNode The first node to connect. - * @param movingNode The second node to connect. + * @param movingBlock The block we're moving. * @returns True if the key was handled; false if something went * wrong. */ - tryToConnectNodes( - workspace: Blockly.WorkspaceSvg, + tryToConnectBlock( stationaryNode: Blockly.ASTNode, - movingNode: Blockly.ASTNode, + movingBlock: Blockly.BlockSvg, ): boolean { - if (!this.logConnectionWarning(stationaryNode, movingNode)) { - return false; - } - const stationaryType = stationaryNode.getType(); - const movingType = movingNode.getType(); - const stationaryLoc = stationaryNode.getLocation(); - const movingLoc = movingNode.getLocation(); if (stationaryNode.isConnection()) { - if (movingNode.isConnection()) { - const stationaryAsConnection = - stationaryLoc as Blockly.RenderedConnection; - const movingAsConnection = movingLoc as Blockly.RenderedConnection; - return this.connect(movingAsConnection, stationaryAsConnection); - } // Connect the moving block to the stationary connection using // the most plausible connection on the moving block. - if ( - movingType === Blockly.ASTNode.types.BLOCK || - movingType === Blockly.ASTNode.types.STACK - ) { - const stationaryAsConnection = - stationaryLoc as Blockly.RenderedConnection; - const movingAsBlock = movingLoc as Blockly.BlockSvg; - return this.insertBlock(movingAsBlock, stationaryAsConnection); - } + const stationaryAsConnection = + stationaryLoc as Blockly.RenderedConnection; + return this.insertBlock(movingBlock, stationaryAsConnection); } else if (stationaryType === Blockly.ASTNode.types.WORKSPACE) { - const block = movingNode - ? (movingNode.getSourceBlock() as Blockly.BlockSvg) - : null; - return this.moveBlockToWorkspace(block, stationaryNode); - } else if ( - stationaryType === Blockly.ASTNode.types.BLOCK && - movingType === Blockly.ASTNode.types.BLOCK - ) { + return this.moveBlockToWorkspace(movingBlock, stationaryNode); + } else if (stationaryType === Blockly.ASTNode.types.BLOCK) { // Insert the moving block above the stationary block, if the // appropriate connections exist. const stationaryBlock = stationaryLoc as Blockly.BlockSvg; - const movingBlock = movingLoc as Blockly.BlockSvg; if (stationaryBlock.previousConnection) { return this.insertBlock( movingBlock, @@ -736,54 +727,10 @@ export class Navigation { return this.insertBlock(movingBlock, stationaryBlock.outputConnection); } } - this.warn('Unexpected state in tryToConnectNodes.'); + this.warn(`Unexpected case in tryToConnectBlock ${stationaryType}.`); return false; } - /** - * Warns the user if the given cursor or marker node can not be connected. - * - * @param markerNode The node to try to connect to. - * @param cursorNode The node to connect to the markerNode. - * @returns True if the marker and cursor are valid types, false - * otherwise. - */ - logConnectionWarning( - markerNode: Blockly.ASTNode, - cursorNode: Blockly.ASTNode, - ): boolean { - if (!markerNode) { - this.warn('Cannot insert with no marked node.'); - return false; - } - - if (!cursorNode) { - this.warn('Cannot insert with no cursor node.'); - return false; - } - const markerType = markerNode.getType(); - const cursorType = cursorNode.getType(); - - // Check the marker for invalid types. - if (markerType === Blockly.ASTNode.types.FIELD) { - this.warn('Should not have been able to mark a field.'); - return false; - } else if (markerType === Blockly.ASTNode.types.STACK) { - this.warn('Should not have been able to mark a stack.'); - return false; - } - - // Check the cursor for invalid types. - if (cursorType === Blockly.ASTNode.types.FIELD) { - this.warn('Cannot attach a field to anything else.'); - return false; - } else if (cursorType === Blockly.ASTNode.types.WORKSPACE) { - this.warn('Cannot attach a workspace to anything else.'); - return false; - } - return true; - } - /** * Disconnects the block from its parent and moves it to the position of the * workspace node. @@ -1144,12 +1091,7 @@ export class Navigation { ) as Blockly.BlockSvg; if (block) { if (targetNode) { - this.tryToConnectNodes( - workspace, - targetNode, - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - Blockly.ASTNode.createBlockNode(block)!, - ); + this.tryToConnectBlock(targetNode, block); } return true; } From dc434a098511d1e58d401e0c4ba04c6fa3b3fb16 Mon Sep 17 00:00:00 2001 From: Matt Hillsdon Date: Thu, 13 Mar 2025 16:56:54 +0000 Subject: [PATCH 2/2] feat: heurstic insertion logic - use first statement or value input - in statement inputs move to the last next connection in the chain - use next connections over previous connections --- src/actions/insert.ts | 8 ++----- src/navigation.ts | 51 +++++++++++++++++++++++-------------------- 2 files changed, 29 insertions(+), 30 deletions(-) diff --git a/src/actions/insert.ts b/src/actions/insert.ts index 84c60117..37eecc2f 100644 --- a/src/actions/insert.ts +++ b/src/actions/insert.ts @@ -78,12 +78,8 @@ export class InsertAction { */ private registerContextMenuAction() { const insertAboveItem: ContextMenuRegistry.RegistryItem = { - displayText: (scope) => { - if (scope.block?.previousConnection) { - return 'Insert Block Above (I)'; - } else { - return 'Insert Block (I)'; - } + displayText: () => { + return 'Insert Block (I)'; }, preconditionFn: (scope: ScopeWithConnection) => { const block = scope.block ?? scope.connection?.getSourceBlock(); diff --git a/src/navigation.ts b/src/navigation.ts index f6da432f..54c2e7e1 100644 --- a/src/navigation.ts +++ b/src/navigation.ts @@ -672,28 +672,9 @@ export class Navigation { } /** -<<<<<<< HEAD -======= - * Hides the flyout cursor and optionally hides the flyout. - * - * @param workspace The workspace. - * @param shouldHide True if the flyout should be hidden. - */ - resetFlyout(workspace: Blockly.WorkspaceSvg, shouldHide: boolean) { - if (this.getFlyoutCursor(workspace)) { - this.getFlyoutCursor(workspace)!.hide(); - if (shouldHide) { - workspace.getFlyout()!.hide(); - } - } - } - - /** ->>>>>>> f14f250 (chore: simplify tryToConnectNodes) * Tries to intelligently connect the blocks or connections * represented by the given nodes, based on node types and locations. * - * @param workspace The main workspace. * @param stationaryNode The first node to connect. * @param movingBlock The block we're moving. * @returns True if the key was handled; false if something went @@ -715,15 +696,37 @@ export class Navigation { } else if (stationaryType === Blockly.ASTNode.types.WORKSPACE) { return this.moveBlockToWorkspace(movingBlock, stationaryNode); } else if (stationaryType === Blockly.ASTNode.types.BLOCK) { - // Insert the moving block above the stationary block, if the - // appropriate connections exist. const stationaryBlock = stationaryLoc as Blockly.BlockSvg; - if (stationaryBlock.previousConnection) { + + // 1. Connect blocks to first compatible input + const inputType = movingBlock.outputConnection + ? Blockly.inputs.inputTypes.VALUE + : Blockly.inputs.inputTypes.STATEMENT; + const compatibleInputs = stationaryBlock.inputList.filter( + (input) => input.type === inputType, + ); + const input = compatibleInputs.length > 0 ? compatibleInputs[0] : null; + let connection = input?.connection; + if (connection) { + if (inputType === Blockly.inputs.inputTypes.STATEMENT) { + while (connection.targetBlock()?.nextConnection) { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + connection = connection.targetBlock()!.nextConnection!; + } + } return this.insertBlock( movingBlock, - stationaryBlock.previousConnection, + connection as Blockly.RenderedConnection, ); - } else if (stationaryBlock.outputConnection) { + } + + // 2. Connect statement blocks to next connection. + if (stationaryBlock.nextConnection && !movingBlock.outputConnection) { + return this.insertBlock(movingBlock, stationaryBlock.nextConnection); + } + + // 3. Output connection. This will wrap around or displace. + if (stationaryBlock.outputConnection) { return this.insertBlock(movingBlock, stationaryBlock.outputConnection); } }