From 0c9f79f48fb39339a81e4e05bfa91d28f9380799 Mon Sep 17 00:00:00 2001 From: TannerGabriel Date: Sun, 10 Aug 2025 17:22:57 +0200 Subject: [PATCH 1/5] Add stack option to mutator --- .../src/blocks/procedures.js | 59 ++++++++++++++++--- block-lexical-variables/src/msg.js | 2 + 2 files changed, 54 insertions(+), 7 deletions(-) diff --git a/block-lexical-variables/src/blocks/procedures.js b/block-lexical-variables/src/blocks/procedures.js index 7c87e63..f9dba7f 100644 --- a/block-lexical-variables/src/blocks/procedures.js +++ b/block-lexical-variables/src/blocks/procedures.js @@ -574,13 +574,16 @@ Blockly.Blocks['procedures_defreturn'] = { Blockly.Msg['LANG_PROCEDURES_DEFRETURN_PROCEDURE'], this); this.createHeader(legalName); this.horizontalParameters = true; // horizontal by default + this.appendStatementInput('STACK') + .appendField(Blockly.Msg['LANG_PROCEDURES_DOTHENRETURN_DO']); this.appendInputFromRegistry('indented_input', 'RETURN') - .setAlign(Blockly.inputs.Align.RIGHT) - .appendField(Blockly.Msg['LANG_PROCEDURES_DEFRETURN_RETURN']); + .setAlign(Blockly.inputs.Align.RIGHT) + .appendField(Blockly.Msg['LANG_PROCEDURES_DEFRETURN_RETURN']); this.setMutator(new Blockly.icons.MutatorIcon(['procedures_mutatorarg'], this)); this.setTooltip(Blockly.Msg['LANG_PROCEDURES_DEFRETURN_TOOLTIP']); this.arguments_ = []; this.warnings = [{name: 'checkEmptySockets', sockets: ['RETURN']}]; + this.stackEnabled_ = true; }, createHeader: function(procName) { return this.appendDummyInput('HEADER') @@ -595,10 +598,48 @@ Blockly.Blocks['procedures_defreturn'] = { parameterFlydown: Blockly.Blocks.procedures_defnoreturn.parameterFlydown, setParameterOrientation: Blockly.Blocks.procedures_defnoreturn.setParameterOrientation, - mutationToDom: Blockly.Blocks.procedures_defnoreturn.mutationToDom, - domToMutation: Blockly.Blocks.procedures_defnoreturn.domToMutation, - decompose: Blockly.Blocks.procedures_defnoreturn.decompose, - compose: Blockly.Blocks.procedures_defnoreturn.compose, + mutationToDom: function () { + const m = Blockly.Blocks.procedures_defnoreturn.mutationToDom.call(this); + m.setAttribute('stack_enabled', this.stackEnabled_ ? 'true' : 'false'); + return m; + }, + domToMutation: function (xml) { + Blockly.Blocks.procedures_defnoreturn.domToMutation.call(this, xml); + + this.stackEnabled_ = xml.getAttribute('stack_enabled') === 'true'; + const hasStack = !!this.getInput('STACK'); + if (this.stackEnabled_ && !hasStack) { + this.appendStatementInput('STACK') + .appendField(Blockly.Msg['LANG_PROCEDURES_DOTHENRETURN_DO']); + if (this.getInput('RETURN')) this.moveInputBefore('STACK', 'RETURN'); + } else if (!this.stackEnabled_ && hasStack) { + this.removeInput('STACK'); + } + }, + decompose: function (workspace) { + const containerBlock = + Blockly.Blocks.procedures_defnoreturn.decompose.call(this, workspace); + + const cb = containerBlock.getField('STACK_ENABLED'); + if (cb) cb.setValue(this.stackEnabled_ ? 'TRUE' : 'FALSE'); + + return containerBlock; + }, + compose: function (containerBlock) { + const cb = containerBlock.getField('STACK_ENABLED'); + this.stackEnabled_ = cb ? cb.getValue() === 'TRUE' : false; + + const hasStack = !!this.getInput('STACK'); + if (this.stackEnabled_ && !hasStack) { + this.appendStatementInput('STACK') + .appendField(Blockly.Msg['LANG_PROCEDURES_DOTHENRETURN_DO']); + if (this.getInput('RETURN')) this.moveInputBefore('STACK', 'RETURN'); + } else if (!this.stackEnabled_ && hasStack) { + this.removeInput('STACK'); + } + + Blockly.Blocks.procedures_defnoreturn.compose.call(this, containerBlock); + }, dispose: Blockly.Blocks.procedures_defnoreturn.dispose, getProcedureDef: Blockly.Blocks.procedures_defnoreturn.getProcedureDef, getDeclaredVars: Blockly.Blocks.procedures_defnoreturn.getDeclaredVars, @@ -621,8 +662,12 @@ Blockly.Blocks['procedures_mutatorcontainer'] = { // this.setColour(Blockly.PROCEDURE_CATEGORY_HUE); this.setStyle('procedure_blocks'); this.appendDummyInput() - .appendField(Blockly.Msg['LANG_PROCEDURES_MUTATORCONTAINER_TITLE']); + .appendField(Blockly.Msg['LANG_PROCEDURES_MUTATORCONTAINER_TITLE']); this.appendStatementInput('STACK'); + this.appendDummyInput('ENABLE_STACK') + .appendField(Blockly.Msg['LANG_PROCEDURES_DEFRETURN_ENABLE_STACK']) + .appendField(new Blockly.FieldCheckbox('false'), Blockly.Msg['LANG_PROCEDURES_DEFRETURN_STACK_ENABLE_FIELD']) + .appendField(Blockly.Msg['LANG_PROCEDURES_MUTATORCONTAINER_STACK']); this.setTooltip(Blockly.Msg['LANG_PROCEDURES_MUTATORCONTAINER_TOOLTIP']); this.contextMenu = false; this.mustNotRenameCapturables = true; diff --git a/block-lexical-variables/src/msg.js b/block-lexical-variables/src/msg.js index cdaa364..3ee5c71 100644 --- a/block-lexical-variables/src/msg.js +++ b/block-lexical-variables/src/msg.js @@ -72,6 +72,8 @@ Blockly.Msg['LANG_PROCEDURES_DEFRETURN_RETURN'] = 'result'; Blockly.Msg['LANG_PROCEDURES_DEFRETURN_COLLAPSED_PREFIX'] = 'to '; Blockly.Msg['LANG_PROCEDURES_DEFRETURN_TOOLTIP'] = 'A procedure returning a result value.'; +Blockly.Msg['LANG_PROCEDURES_DEFRETURN_ENABLE_STACK'] = 'allow statements' +Blockly.Msg['LANG_PROCEDURES_DEFRETURN_STACK_ENABLE_FIELD'] = 'STACK_ENABLED' Blockly.Msg['LANG_PROCEDURES_DEF_DUPLICATE_WARNING'] = 'Warning:\nThis procedure has\nduplicate inputs.'; Blockly.Msg['LANG_PROCEDURES_CALLNORETURN_CALL'] = 'call '; From ce8756e9f427e1aabe5a6814c4184f45dac0b3b2 Mon Sep 17 00:00:00 2001 From: TannerGabriel Date: Sun, 10 Aug 2025 17:39:22 +0200 Subject: [PATCH 2/5] Only add enable Stack field to procedures_defreturn and reconstruct when updating parameters --- .../src/blocks/procedures.js | 40 +++++++++++++++---- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/block-lexical-variables/src/blocks/procedures.js b/block-lexical-variables/src/blocks/procedures.js index f9dba7f..e0a3ff8 100644 --- a/block-lexical-variables/src/blocks/procedures.js +++ b/block-lexical-variables/src/blocks/procedures.js @@ -172,7 +172,8 @@ Blockly.Blocks['procedures_defnoreturn'] = { // [lyn, 10/24/13] need to reconstruct first input // Body of procedure - const bodyInput = this.inputList[this.inputList.length - 1]; + const stackInput = this.getInput('STACK'); + const returnInput = this.getInput('RETURN'); // stop rendering until block is recreated const savedRendered = this.rendered; @@ -193,7 +194,7 @@ Blockly.Blocks['procedures_defnoreturn'] = { // necessary) // Only args and body are left - const oldArgCount = this.inputList.length - 1; + const oldArgCount = this.inputList.length - (stackInput ? 1 : 0) - (returnInput ? 1 : 0); if (oldArgCount > 0) { const paramInput0 = this.getInput('VAR0'); if (paramInput0) { // Yes, they were vertical @@ -242,7 +243,20 @@ Blockly.Blocks['procedures_defnoreturn'] = { } // put the last two arguments back - this.inputList = this.inputList.concat(bodyInput); + const wantStack = (this.bodyInputName === 'STACK') || !!this.stackEnabled_; + if (wantStack) { + if (stackInput) { + this.inputList = this.inputList.concat(stackInput); + } else if (this.bodyInputName !== 'STACK') { + // defreturn with STACK enabled but missing -> create it + this.appendStatementInput('STACK') + .appendField(Blockly.Msg['LANG_PROCEDURES_DEFNORETURN_DO']); + } + } + + if (returnInput) { + this.inputList = this.inputList.concat(returnInput); + } this.rendered = savedRendered; // [lyn, 10/28/13] I thought this rerendering was unnecessary. But I was @@ -664,10 +678,6 @@ Blockly.Blocks['procedures_mutatorcontainer'] = { this.appendDummyInput() .appendField(Blockly.Msg['LANG_PROCEDURES_MUTATORCONTAINER_TITLE']); this.appendStatementInput('STACK'); - this.appendDummyInput('ENABLE_STACK') - .appendField(Blockly.Msg['LANG_PROCEDURES_DEFRETURN_ENABLE_STACK']) - .appendField(new Blockly.FieldCheckbox('false'), Blockly.Msg['LANG_PROCEDURES_DEFRETURN_STACK_ENABLE_FIELD']) - .appendField(Blockly.Msg['LANG_PROCEDURES_MUTATORCONTAINER_STACK']); this.setTooltip(Blockly.Msg['LANG_PROCEDURES_MUTATORCONTAINER_TOOLTIP']); this.contextMenu = false; this.mustNotRenameCapturables = true; @@ -675,6 +685,22 @@ Blockly.Blocks['procedures_mutatorcontainer'] = { // [lyn. 11/24/12] Set procBlock associated with this container. setProcBlock: function(procBlock) { this.procBlock_ = procBlock; + const isDefReturn = !!procBlock && procBlock.type === 'procedures_defreturn'; + + if (this.getInput('ENABLE_STACK')) this.removeInput('ENABLE_STACK'); + + if (isDefReturn) { + const row = this.appendDummyInput('ENABLE_STACK') + .appendField(Blockly.Msg['LANG_PROCEDURES_DEFRETURN_ENABLE_STACK']) + .appendField(new Blockly.FieldCheckbox('false'), Blockly.Msg['LANG_PROCEDURES_DEFRETURN_STACK_ENABLE_FIELD']) + .appendField(Blockly.Msg['LANG_PROCEDURES_MUTATORCONTAINER_STACK']); + row.init(); + + const cb = this.getField('STACK_ENABLED'); + if (cb) cb.setValue(procBlock.stackEnabled_ ? 'TRUE' : 'FALSE'); + } + + if (this.rendered) this.render(); }, // [lyn. 11/24/12] Set procBlock associated with this container. // Invariant: should not be null, since only created as mutator for a From 2956436a7920772232d99e9a619cfa5bd9b48df5 Mon Sep 17 00:00:00 2001 From: TannerGabriel Date: Mon, 11 Aug 2025 09:19:55 +0200 Subject: [PATCH 3/5] Test if old procedure_defreturn works in new version --- .../test/procedures.mocha.js | 114 ++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 block-lexical-variables/test/procedures.mocha.js diff --git a/block-lexical-variables/test/procedures.mocha.js b/block-lexical-variables/test/procedures.mocha.js new file mode 100644 index 0000000..fa4e8be --- /dev/null +++ b/block-lexical-variables/test/procedures.mocha.js @@ -0,0 +1,114 @@ +import * as Blockly from 'blockly/core'; +import * as libraryBlocks from 'blockly/blocks'; + +import '../src/msg'; +import '../src/utilities'; +import '../src/workspace'; +import '../src/procedure_utils'; +import '../src/fields/flydown'; +import '../src/fields/field_flydown'; +import '../src/fields/field_global_flydown'; +import '../src/fields/field_nocheck_dropdown'; +import '../src/fields/field_parameter_flydown'; +import '../src/fields/field_procedurename'; +import '../src/blocks/lexical-variables'; +import '../src/blocks/controls'; +import '../src/blocks/variable-get-set.js'; +import '../src/procedure_database'; +import '../src/blocks/procedures'; +import '../src/generators/controls'; +import '../src/generators/procedures'; +import '../src/generators/lexical-variables'; + + +import chai from 'chai'; + + +/** + * Utility function to check if a block contains another block by type or id. + * @param {Blockly.Block} block - The block to search within. + * @param {string} identifier - The type or id of the block to search for. + * @param {boolean} isId - Whether to search by id (true) or type (false). + * @returns {boolean} - True if the block contains the specified block, false otherwise. + */ +function containsBlocks(block, identifier, isId = true) { + if (!block) return false; + + if (isId && block.id === identifier) return true; + if (!isId && block.type === identifier) return true; + + const children = block.getChildren(true); + for (const child of children) { + if (containsBlocks(child, identifier, isId)) { + return true; + } + } + + return false; +} + +suite ('Procedures', function() { + setup(function () { + this.workspace = new Blockly.Workspace(); + Blockly.common.setMainWorkspace(this.workspace); + }); + teardown(function () { + delete this.workspace; + }) + + suite('procedures_defreturn', function() { + test('Load procedure from old version (without stack_enable)', function() { + const xml = Blockly.utils.xml.textToDom( + '' + + ' ' + + ' ' + + ' ' + + ' ' + + ' ' + + ' do_something' + + ' x' + + ' y' + + ' ' + + ' ' + + ' ' + + ' ' + + ' name' + + ' ' + + ' ' + + ' name' + + ' ' + + ' ' + + ' y' + + ' ' + + ' ' + + ' ' + + ' ' + + ' ' + + ' ' + + ' ' + + ' ' + + ' x' + + ' ' + + ' ' + + ' ' + + ' ' + + ' ' + + '' + ); + + Blockly.Xml.domToWorkspace(xml, this.workspace); + const block = this.workspace.getBlockById('p'); + chai.assert.isDefined(block, 'Procedure block should be defined'); + chai.assert.equal(block.type, 'procedures_defreturn', 'Block type should be procedures_defreturn'); + + const mutation = block.mutationToDom(); + chai.assert.isNotNull(mutation, 'Mutation should not be null'); + chai.assert.equal(mutation.getAttribute('stack_enabled'), 'false', 'stack_enabled should be false'); + + const blockIds = ['a', 'b', 'c', 'd', 'f']; + blockIds.forEach(id => { + chai.assert.isTrue(containsBlocks(block, id), `Block with id ${id} should be contained`); + }); + }) + }) +}) \ No newline at end of file From 24bbc3ca783d3267e516d37673b47444a15fb2e1 Mon Sep 17 00:00:00 2001 From: TannerGabriel Date: Mon, 11 Aug 2025 09:37:04 +0200 Subject: [PATCH 4/5] Add test for toggling procedure_defreturn stack input --- .../test/procedures.mocha.js | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/block-lexical-variables/test/procedures.mocha.js b/block-lexical-variables/test/procedures.mocha.js index fa4e8be..556c671 100644 --- a/block-lexical-variables/test/procedures.mocha.js +++ b/block-lexical-variables/test/procedures.mocha.js @@ -47,6 +47,20 @@ function containsBlocks(block, identifier, isId = true) { return false; } +/** + * Utility function to check if a procedure block has a stack field. + * @param procedureBlock - The procedure block to check. + * @param stackEnabled - Whether the stack field should be present (true) or not (false). + */ +function checkStackFieldExists(procedureBlock, stackEnabled = false) { + const stackField = procedureBlock.getInput('STACK'); + if (stackEnabled) { + chai.assert.isDefined(stackField, 'Procedure block should have a stack field'); + } else { + chai.assert.isNull(stackField, 'Procedure block should not have a stack field'); + } +} + suite ('Procedures', function() { setup(function () { this.workspace = new Blockly.Workspace(); @@ -110,5 +124,37 @@ suite ('Procedures', function() { chai.assert.isTrue(containsBlocks(block, id), `Block with id ${id} should be contained`); }); }) + + test('Mutation button click should enable stack', function() { + const xml = Blockly.utils.xml.textToDom( + '' + + ' ' + + ' ' + + ' ' + + ' ' + + ' ' + + ' do_something' + + ' ' + + '' + ); + + Blockly.Xml.domToWorkspace(xml, this.workspace); + const block = this.workspace.getBlockById('p'); + chai.assert.isDefined(block, 'Procedure block should be defined'); + + const mutation = block.mutationToDom(); + chai.assert.equal(mutation.getAttribute('stack_enabled'), 'false', 'stack_enabled should initially be false'); + checkStackFieldExists(block, false); + + block.domToMutation(Blockly.utils.xml.textToDom( + '' + + ' ' + + ' ' + + '' + )); + const updatedMutation = block.mutationToDom(); + chai.assert.equal(updatedMutation.getAttribute('stack_enabled'), 'true', 'stack_enabled should be true after toggle'); + checkStackFieldExists(block, true); + }); }) }) \ No newline at end of file From d213ceaac878813d68a5fe6e2b909a9dc6b0d08c Mon Sep 17 00:00:00 2001 From: TannerGabriel Date: Tue, 12 Aug 2025 15:49:16 +0200 Subject: [PATCH 5/5] Add saveExtraState and loadExtraState to procedures --- .../src/blocks/procedures.js | 28 +++++ .../test/procedures.mocha.js | 111 ++++++++++++++++++ 2 files changed, 139 insertions(+) diff --git a/block-lexical-variables/src/blocks/procedures.js b/block-lexical-variables/src/blocks/procedures.js index e0a3ff8..a070b2c 100644 --- a/block-lexical-variables/src/blocks/procedures.js +++ b/block-lexical-variables/src/blocks/procedures.js @@ -404,6 +404,17 @@ Blockly.Blocks['procedures_defnoreturn'] = { xmlElement.getAttribute('vertical_parameters') !== 'true'; this.updateParams_(params); }, + saveExtraState() { + const state = {}; + if (!this.horizontalParameters) state.vertical_parameters = true; + if (this.arguments_?.length) state.args = [...this.arguments_]; + return state; + }, + loadExtraState(state) { + const params = state?.args || []; + this.horizontalParameters = state?.vertical_parameters !== true; + this.updateParams_(params); + }, decompose: function(workspace) { const containerBlock = workspace.newBlock('procedures_mutatorcontainer'); containerBlock.initSvg(); @@ -630,6 +641,23 @@ Blockly.Blocks['procedures_defreturn'] = { this.removeInput('STACK'); } }, + saveExtraState: function () { + const element = Blockly.Blocks.procedures_defnoreturn.saveExtraState.call(this); + element.stackEnabled = !!this.stackEnabled_; + return element; + }, + loadExtraState: function (state) { + Blockly.Blocks.procedures_defnoreturn.loadExtraState.call(this, state); + this.stackEnabled_ = state.stackEnabled; + const hasStack = !!this.getInput('STACK'); + if (this.stackEnabled_ && !hasStack) { + this.appendStatementInput('STACK') + .appendField(Blockly.Msg['LANG_PROCEDURES_DOTHENRETURN_DO']); + if (this.getInput('RETURN')) this.moveInputBefore('STACK', 'RETURN'); + } else if (!this.stackEnabled_ && hasStack) { + this.removeInput('STACK'); + } + }, decompose: function (workspace) { const containerBlock = Blockly.Blocks.procedures_defnoreturn.decompose.call(this, workspace); diff --git a/block-lexical-variables/test/procedures.mocha.js b/block-lexical-variables/test/procedures.mocha.js index 556c671..b8e8410 100644 --- a/block-lexical-variables/test/procedures.mocha.js +++ b/block-lexical-variables/test/procedures.mocha.js @@ -125,6 +125,85 @@ suite ('Procedures', function() { }); }) + test('Load procedure from old version using JSON (without stack_enable)', function() { + const json = { + blocks: { + languageVersion: 0, + blocks: [ + { + type: 'procedures_defreturn', + id: 'p', + x: 310, + y: 213, + fields: { + NAME: 'do_something', + VAR0: 'x', + VAR1: 'y', + }, + extraState: { args: ['x', 'y'] }, + inputs: { + RETURN: { + block: { + type: 'controls_do_then_return', + id: 'a', + inputs: { + STM: { + block: { + type: 'simple_local_declaration_statement', + id: 'b', + fields: { VAR: 'name' }, + inputs: { + DO: { + block: { + type: 'lexical_variable_set', + id: 'c', + fields: { VAR: 'name' }, + inputs: { + VALUE: { + block: { + type: 'lexical_variable_get', + id: 'd', + fields: { VAR: 'y' }, + }, + }, + }, + }, + }, + }, + }, + }, + VALUE: { + block: { + type: 'lexical_variable_get', + id: 'f', + fields: { VAR: 'x' }, + }, + }, + }, + }, + }, + }, + }, + ], + }, + }; + + Blockly.serialization.workspaces.load(json, this.workspace); + const block = this.workspace.getBlockById('p'); + chai.assert.isDefined(block, 'Procedure block should be defined'); + chai.assert.equal(block.type, 'procedures_defreturn', 'Block type should be procedures_defreturn'); + + const extra = block.saveExtraState ? block.saveExtraState() : null; + chai.assert.isNotNull(extra, 'Extra state should not be null'); + chai.assert.strictEqual(extra.stackEnabled, false, 'stack_enabled should be false'); + + const blockIds = ['a', 'b', 'c', 'd', 'f']; + blockIds.forEach(id => { + chai.assert.isTrue(containsBlocks(block, id), `Block with id ${id} should be contained`); + }); + }); + + test('Mutation button click should enable stack', function() { const xml = Blockly.utils.xml.textToDom( '' + @@ -156,5 +235,37 @@ suite ('Procedures', function() { chai.assert.equal(updatedMutation.getAttribute('stack_enabled'), 'true', 'stack_enabled should be true after toggle'); checkStackFieldExists(block, true); }); + + test('Mutation button click should enable stack using JSON', function() { + const json = { + blocks: { + languageVersion: 0, + blocks: [ + { + type: 'procedures_defreturn', + id: 'p', + x: 310, + y: 213, + fields: { NAME: 'do_something' }, + extraState: { stack_enabled: false, args: ['x', 'y'] }, + }, + ], + }, + }; + + Blockly.serialization.workspaces.load(json, this.workspace); + const block = this.workspace.getBlockById('p'); + chai.assert.isDefined(block, 'Procedure block should be defined'); + + const extra = block.saveExtraState(); + chai.assert.strictEqual(extra.stackEnabled, false, 'stack_enabled should initially be false'); + checkStackFieldExists(block, false); + + block.loadExtraState({ ...extra, stackEnabled: true }); + + const updated = block.saveExtraState(); + chai.assert.strictEqual(updated.stackEnabled, true, 'stack_enabled should be true after toggle'); + checkStackFieldExists(block, true); + }); }) }) \ No newline at end of file