feat: procedure blocks update models (#6672)

* feat: procedure blocks have models

* feat: add updating the name of the model

* feat: add procedure defs updating the model enabled state

* feat: add procedure blocks updating parameters in the model

* fix: add disposing of the procedure model

* chore: updates test to check for identity of parameters

* chore: move statement handling into setStatement

* fix: make parameter IDs consistent

* chore: un-only tests

* chore: fixup tests

* chore: revert validator to use Procedures.rename

* chore: cleanup typo
This commit is contained in:
Beka Westberg
2023-01-05 22:31:20 +00:00
committed by GitHub
parent 37ad8eed83
commit 5d20b62339
6 changed files with 167 additions and 222 deletions

View File

@@ -28,12 +28,15 @@ const {Block} = goog.requireType('Blockly.Block');
// TODO (6248): Properly import the BlockDefinition type.
/* eslint-disable-next-line no-unused-vars */
const BlockDefinition = Object;
const {ObservableProcedureModel} = goog.require('Blockly.procedures.ObservableProcedureModel');
const {ObservableParameterModel} = goog.require('Blockly.procedures.ObservableParameterModel');
const {config} = goog.require('Blockly.config');
/* eslint-disable-next-line no-unused-vars */
const {FieldLabel} = goog.require('Blockly.FieldLabel');
const {Msg} = goog.require('Blockly.Msg');
const {Mutator} = goog.require('Blockly.Mutator');
const {Names} = goog.require('Blockly.Names');
const serialization = goog.require('Blockly.serialization');
/* eslint-disable-next-line no-unused-vars */
const {VariableModel} = goog.requireType('Blockly.VariableModel');
/* eslint-disable-next-line no-unused-vars */
@@ -85,7 +88,8 @@ const blocks = createBlockDefinitionsFromJsonArray([
'procedure_def_var_mixin',
'procedure_def_update_shape_mixin',
'procedure_def_context_menu_mixin',
'procedure_def_get_legal_name_helper',
'procedure_def_onchange_mixin',
'procedure_def_validator_helper',
'procedure_defnoreturn_get_caller_block_mixin',
'procedure_defnoreturn_set_comment_helper',
],
@@ -160,7 +164,8 @@ const blocks = createBlockDefinitionsFromJsonArray([
'procedure_def_var_mixin',
'procedure_def_update_shape_mixin',
'procedure_def_context_menu_mixin',
'procedure_def_get_legal_name_helper',
'procedure_def_onchange_mixin',
'procedure_def_validator_helper',
'procedure_defreturn_get_caller_block_mixin',
'procedure_defreturn_set_comment_helper',
],
@@ -274,6 +279,12 @@ exports.blocks = blocks;
/** @this {Block} */
const procedureDefGetDefMixin = function() {
const mixin = {
model: null,
getProcedureModel() {
return this.model;
},
/**
* Return all variables referenced by this block.
* @return {!Array<string>} List of variable names.
@@ -282,6 +293,7 @@ const procedureDefGetDefMixin = function() {
getVars: function() {
return this.arguments_;
},
/**
* Return all variables referenced by this block.
* @return {!Array<!VariableModel>} List of variable models.
@@ -290,8 +302,20 @@ const procedureDefGetDefMixin = function() {
getVarModels: function() {
return this.argumentVarModels_;
},
/**
* Disposes of the data model for this procedure block when the block is
* disposed.
*/
destroy: function() {
this.workspace.getProcedureMap().delete(this.getProcedureModel().getId());
},
};
mixin.model = new ObservableProcedureModel(
this.workspace, this.getFieldValue('NAME'));
this.workspace.getProcedureMap().add(mixin.model);
this.mixin(mixin, true);
};
// Using register instead of registerMixin to avoid triggering warnings about
@@ -382,7 +406,18 @@ const procedureDefUpdateShapeMixin = {
if (this.getInput('RETURN')) {
this.moveInputBefore('STACK', 'RETURN');
}
// Restore the stack, if one was saved.
Mutator.reconnect(this.statementConnection_, this, 'STACK');
this.statementConnection_ = null;
} else {
// Save the stack, then disconnect it.
const stackConnection = this.getInput('STACK').connection;
this.statementConnection_ = stackConnection.targetConnection;
if (this.statementConnection_) {
const stackBlock = stackConnection.targetBlock();
stackBlock.unplug();
stackBlock.bumpNeighbours();
}
this.removeInput('STACK', true);
}
this.hasStatements_ = hasStatements;
@@ -435,13 +470,13 @@ Extensions.registerMixin(
'procedure_def_update_shape_mixin', procedureDefUpdateShapeMixin);
/** @this {Block} */
const procedureDefGetLegalNameHelper = function() {
const procedureDefValidatorHelper = function() {
const nameField = this.getField('NAME');
nameField.setValue(Procedures.findLegalName('', this));
nameField.setValidator(Procedures.rename);
};
Extensions.register(
'procedure_def_get_legal_name_helper', procedureDefGetLegalNameHelper);
'procedure_def_validator_helper', procedureDefValidatorHelper);
const procedureDefMutator = {
arguments_: [],
@@ -570,41 +605,29 @@ const procedureDefMutator = {
* @this {Block}
*/
decompose: function(workspace) {
/*
* Creates the following XML:
* <block type="procedures_mutatorcontainer">
* <statement name="STACK">
* <block type="procedures_mutatorarg">
* <field name="NAME">arg1_name</field>
* <next>etc...</next>
* </block>
* </statement>
* </block>
*/
const containerBlockDef = {
'type': 'procedures_mutatorcontainer',
'inputs': {
'STACK': {},
},
};
const containerBlockNode = xmlUtils.createElement('block');
containerBlockNode.setAttribute('type', 'procedures_mutatorcontainer');
const statementNode = xmlUtils.createElement('statement');
statementNode.setAttribute('name', 'STACK');
containerBlockNode.appendChild(statementNode);
let node = statementNode;
for (let i = 0; i < this.arguments_.length; i++) {
const argBlockNode = xmlUtils.createElement('block');
argBlockNode.setAttribute('type', 'procedures_mutatorarg');
const fieldNode = xmlUtils.createElement('field');
fieldNode.setAttribute('name', 'NAME');
const argumentName = xmlUtils.createTextNode(this.arguments_[i]);
fieldNode.appendChild(argumentName);
argBlockNode.appendChild(fieldNode);
const nextNode = xmlUtils.createElement('next');
argBlockNode.appendChild(nextNode);
node.appendChild(argBlockNode);
node = nextNode;
let connDef = containerBlockDef['inputs']['STACK'];
for (const param of this.getProcedureModel().getParameters()) {
connDef['block'] = {
'type': 'procedures_mutatorarg',
'id': param.getId(),
'fields': {
'NAME': param.getName(),
},
'next': {},
};
connDef = connDef['block']['next'];
}
const containerBlock = Xml.domToBlock(containerBlockNode, workspace);
const containerBlock =
serialization.blocks.append(
containerBlockDef, workspace, {recordUndo: false});
if (this.type === 'procedures_defreturn') {
containerBlock.setFieldValue(this.hasStatements_, 'STATEMENTS');
@@ -612,8 +635,9 @@ const procedureDefMutator = {
containerBlock.removeInput('STATEMENT_INPUT');
}
// Initialize procedure's callers with blank IDs.
// Update callers (called for backwards compatibility).
Procedures.mutateCallers(this);
return containerBlock;
},
@@ -627,11 +651,14 @@ const procedureDefMutator = {
this.arguments_ = [];
this.paramIds_ = [];
this.argumentVarModels_ = [];
// TODO: Remove old data handling logic?
let paramBlock = containerBlock.getInputTargetBlock('STACK');
while (paramBlock && !paramBlock.isInsertionMarker()) {
const varName = paramBlock.getFieldValue('NAME');
this.arguments_.push(varName);
const variable = this.workspace.getVariable(varName, '');
const variable = Variables.getOrCreateVariablePackage(
this.workspace, null, varName, '');
this.argumentVarModels_.push(variable);
this.paramIds_.push(paramBlock.id);
@@ -640,29 +667,25 @@ const procedureDefMutator = {
}
this.updateParams_();
Procedures.mutateCallers(this);
for (let i = this.model.getParameters().length; i >= 0; i--) {
this.model.deleteParameter(i);
}
// Show/hide the statement input.
let hasStatements = containerBlock.getFieldValue('STATEMENTS');
let i = 0;
paramBlock = containerBlock.getInputTargetBlock('STACK');
while (paramBlock && !paramBlock.isInsertionMarker()) {
this.model.insertParameter(
new ObservableParameterModel(
this.workspace, paramBlock.getFieldValue('NAME'), paramBlock.id),
i);
paramBlock =
paramBlock.nextConnection && paramBlock.nextConnection.targetBlock();
i++;
}
const hasStatements = containerBlock.getFieldValue('STATEMENTS');
if (hasStatements !== null) {
hasStatements = hasStatements === 'TRUE';
if (this.hasStatements_ !== hasStatements) {
if (hasStatements) {
this.setStatements_(true);
// Restore the stack, if one was saved.
Mutator.reconnect(this.statementConnection_, this, 'STACK');
this.statementConnection_ = null;
} else {
// Save the stack, then disconnect it.
const stackConnection = this.getInput('STACK').connection;
this.statementConnection_ = stackConnection.targetConnection;
if (this.statementConnection_) {
const stackBlock = stackConnection.targetBlock();
stackBlock.unplug();
stackBlock.bumpNeighbours();
}
this.setStatements_(false);
}
}
this.setStatements_(hasStatements === 'TRUE');
}
},
};
@@ -718,6 +741,16 @@ const procedureDefContextMenuMixin = {
Extensions.registerMixin(
'procedure_def_context_menu_mixin', procedureDefContextMenuMixin);
const procedureDefOnChangeMixin = {
onchange: function(e) {
if (e.type === Events.BLOCK_CHANGE && e.element == 'disabled') {
this.model.setEnabled(!e.newValue);
}
},
};
Extensions.registerMixin(
'procedure_def_onchange_mixin', procedureDefOnChangeMixin);
/** @this {Block} */
const procedureDefNoReturnSetCommentHelper = function() {
if ((this.workspace.options.comments ||

View File

@@ -5,11 +5,13 @@
*/
import type {Block} from '../block.js';
import {IProcedureModel} from './i_procedure_model.js';
/** The interface for a block which models a procedure. */
export interface IProcedureBlock {
doProcedureUpdate(): void;
getProcedureModel(): IProcedureModel;
}
/** A type guard which checks if the given block is a procedure block. */

View File

@@ -23,6 +23,7 @@ import type {Abstract} from './events/events_abstract.js';
import type {BubbleOpen} from './events/events_bubble_open.js';
import * as eventUtils from './events/utils.js';
import {Field, UnattachedFieldError} from './field.js';
import {isProcedureBlock} from './interfaces/i_procedure_block.js';
import {Msg} from './msg.js';
import {Names} from './names.js';
import {ObservableProcedureMap} from './procedures/observable_procedure_map.js';
@@ -192,6 +193,7 @@ export function rename(this: Field, name: string): string {
name = name.trim();
const legalName = findLegalName(name, block);
if (isProcedureBlock(block)) block.getProcedureModel().setName(legalName);
const oldName = this.getValue();
if (oldName !== name && oldName !== legalName) {
// Rename any callers.

View File

@@ -11,6 +11,8 @@ import type {IProcedureModel} from '../interfaces/i_procedure_model';
import {triggerProceduresUpdate} from './update_procedures.js';
import type {VariableModel} from '../variable_model.js';
import type {Workspace} from '../workspace.js';
import * as goog from '../../closure/goog/goog.js';
goog.declareModuleId('Blockly.procedures.ObservableParameterModel');
export class ObservableParameterModel implements IParameterModel {

View File

@@ -11,6 +11,8 @@ import type {IProcedureModel} from '../interfaces/i_procedure_model.js';
import {isObservable} from '../interfaces/i_observable.js';
import {triggerProceduresUpdate} from './update_procedures.js';
import type {Workspace} from '../workspace.js';
import * as goog from '../../closure/goog/goog.js';
goog.declareModuleId('Blockly.procedures.ObservableProcedureModel');
export class ObservableProcedureModel implements IProcedureModel {

View File

@@ -27,8 +27,8 @@ suite('Procedures', function() {
sharedTestTeardown.call(this);
});
suite.skip('updating data models', function() {
test(
suite('updating data models', function() {
test.skip(
'renaming a procedure def block updates the procedure model',
function() {
const defBlock = createProcDefBlock(this.workspace);
@@ -58,9 +58,11 @@ suite('Procedures', function() {
function() {
// Create a stack of container, parameter.
const defBlock = createProcDefBlock(this.workspace);
defBlock.mutator.setVisible(true);
const mutatorWorkspace = defBlock.mutator.getWorkspace();
const containerBlock =
this.workspace.newBlock('procedures_mutatorcontainer');
const paramBlock = this.workspace.newBlock('procedures_mutatorarg');
mutatorWorkspace.newBlock('procedures_mutatorcontainer');
const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg');
paramBlock.setFieldValue('param name', 'NAME');
containerBlock.getInput('STACK').connection.connect(paramBlock.previousConnection);
@@ -75,9 +77,11 @@ suite('Procedures', function() {
test('adding a parameter adds a variable to the variable map', function() {
// Create a stack of container, parameter.
const defBlock = createProcDefBlock(this.workspace);
defBlock.mutator.setVisible(true);
const mutatorWorkspace = defBlock.mutator.getWorkspace();
const containerBlock =
this.workspace.newBlock('procedures_mutatorcontainer');
const paramBlock = this.workspace.newBlock('procedures_mutatorarg');
mutatorWorkspace.newBlock('procedures_mutatorcontainer');
const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg');
paramBlock.setFieldValue('param name', 'NAME');
containerBlock.getInput('STACK').connection
.connect(paramBlock.previousConnection);
@@ -85,7 +89,7 @@ suite('Procedures', function() {
defBlock.compose(containerBlock);
chai.assert.isTrue(
this.workspace.getVariableMap().getVariables('')
this.workspace.getVariableMap().getVariablesOfType('')
.some((variable) => variable.name === 'param name'),
'Expected the variable map to have a matching variable');
});
@@ -96,16 +100,20 @@ suite('Procedures', function() {
function() {
// Create a stack of container, param1, param2.
const defBlock = createProcDefBlock(this.workspace);
defBlock.mutator.setVisible(true);
const mutatorWorkspace = defBlock.mutator.getWorkspace();
const containerBlock =
this.workspace.newBlock('procedures_mutatorcontainer');
const paramBlock1 = this.workspace.newBlock('procedures_mutatorarg');
mutatorWorkspace.newBlock('procedures_mutatorcontainer');
const paramBlock1 = mutatorWorkspace.newBlock('procedures_mutatorarg');
paramBlock1.setFieldValue('param name1', 'NAME');
const paramBlock2 = this.workspace.newBlock('procedures_mutatorarg');
const paramBlock2 = mutatorWorkspace.newBlock('procedures_mutatorarg');
paramBlock2.setFieldValue('param name2', 'NAME');
containerBlock.getInput('STACK').connection
.connect(paramBlock1.previousConnection);
paramBlock1.nextConnection.connect(paramBlock2.previousConnection);
defBlock.compose(containerBlock);
const id1 = defBlock.getProcedureModel().getParameter(0).getId();
const id2 = defBlock.getProcedureModel().getParameter(1).getId();
// Reconfigure the stack to be container, param2, param1.
paramBlock2.previousConnection.disconnect();
@@ -119,20 +127,57 @@ suite('Procedures', function() {
defBlock.getProcedureModel().getParameter(0).getName(),
'param name2',
'Expected the first parameter of the procedure to be param 2');
chai.assert.equal(
defBlock.getProcedureModel().getParameter(0).getId(),
id2,
'Expected the first parameter of the procedure to be param 2');
chai.assert.equal(
defBlock.getProcedureModel().getParameter(1).getName(),
'param name2',
'param name1',
'Expected the second parameter of the procedure to be param 1');
chai.assert.equal(
defBlock.getProcedureModel().getParameter(1).getId(),
id1,
'Expected the second parameter of the procedure to be param 1');
});
test('decomposing and recomposing maintains parameter IDs', function() {
// Create a stack of container, param.
const defBlock = createProcDefBlock(this.workspace);
defBlock.mutator.setVisible(true);
const mutatorWorkspace = defBlock.mutator.getWorkspace();
const containerBlock =
mutatorWorkspace.newBlock('procedures_mutatorcontainer');
const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg');
paramBlock.setFieldValue('param name', 'NAME');
containerBlock.getInput('STACK').connection
.connect(paramBlock.previousConnection);
defBlock.compose(containerBlock);
const paramBlockId = defBlock.getProcedureModel().getParameter(0).getId();
Blockly.Events.disable();
mutatorWorkspace.clear();
Blockly.Events.enable();
const container = defBlock.decompose(mutatorWorkspace);
defBlock.compose(container);
chai.assert.equal(
defBlock.getProcedureModel().getParameter(0).getId(),
paramBlockId,
'Expected the parameter ID to be maintained');
});
test(
'deleting a parameter from a procedure def updates the procedure model',
function() {
// Create a stack of container, parameter.
const defBlock = createProcDefBlock(this.workspace);
defBlock.mutator.setVisible(true);
const mutatorWorkspace = defBlock.mutator.getWorkspace();
const containerBlock =
this.workspace.newBlock('procedures_mutatorcontainer');
const paramBlock = this.workspace.newBlock('procedures_mutatorarg');
mutatorWorkspace.newBlock('procedures_mutatorcontainer');
const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg');
paramBlock.setFieldValue('param name', 'NAME');
containerBlock.getInput('STACK').connection
.connect(paramBlock.previousConnection);
defBlock.compose(containerBlock);
@@ -148,9 +193,11 @@ suite('Procedures', function() {
test('renaming a procedure parameter updates the parameter model', function() {
// Create a stack of container, parameter.
const defBlock = createProcDefBlock(this.workspace);
defBlock.mutator.setVisible(true);
const mutatorWorkspace = defBlock.mutator.getWorkspace();
const containerBlock =
this.workspace.newBlock('procedures_mutatorcontainer');
const paramBlock = this.workspace.newBlock('procedures_mutatorarg');
mutatorWorkspace.newBlock('procedures_mutatorcontainer');
const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg');
paramBlock.setFieldValue('param name', 'NAME');
containerBlock.getInput('STACK').connection
.connect(paramBlock.previousConnection);
@@ -159,9 +206,10 @@ suite('Procedures', function() {
paramBlock.setFieldValue('new param name', 'NAME');
defBlock.compose(containerBlock);
chai.assert.isEmpty(
defBlock.getProcedureModel().getParameters(),
'Expected the procedure model to have no parameters');
chai.assert.equal(
defBlock.getProcedureModel().getParameter(0).getName(),
'new param name',
'Expected the procedure model to have a matching parameter');
});
test('deleting a procedure deletes the procedure model', function() {
@@ -1399,123 +1447,6 @@ suite('Procedures', function() {
});
});
suite('Enable/Disable', function() {
setup(function() {
const toolbox = document.getElementById('toolbox-categories');
this.workspaceSvg = Blockly.inject('blocklyDiv', {toolbox: toolbox});
});
teardown(function() {
workspaceTeardown.call(this, this.workspaceSvg);
});
const domText = (testSuite.defType === 'procedures_defreturn') ?
('<xml xmlns="https://developers.google.com/blockly/xml">' +
'<block type="procedures_defreturn" id="bar-def">' +
'<field name="NAME">bar</field>' +
'<value name="RETURN">' +
'<block type="procedures_callreturn" id="bar-c1">' +
'<mutation name="bar"></mutation>' +
'</block>' +
'</value>' +
'</block>' +
'<block type="procedures_callreturn" id="bar-c2">' +
'<mutation name="bar"></mutation>' +
'</block>' +
'</xml>') :
('<xml xmlns="https://developers.google.com/blockly/xml">' +
'<block type="procedures_defnoreturn" id="bar-def">' +
'<field name="NAME">bar</field>' +
'</block>' +
'<block type="procedures_callnoreturn" id="bar-c1">' +
'<mutation name="bar"></mutation>' +
'</block>' +
'<block type="procedures_callnoreturn" id="bar-c2">' +
'<mutation name="bar"></mutation>' +
'</block>' +
'</xml>');
setup(function() {
const dom = Blockly.Xml.textToDom(domText);
Blockly.Xml.appendDomToWorkspace(dom, this.workspaceSvg);
this.barDef = this.workspaceSvg.getBlockById('bar-def');
this.barCalls = [
this.workspaceSvg.getBlockById('bar-c1'),
this.workspaceSvg.getBlockById('bar-c2'),
];
});
test('Set disabled updates callers', function() {
this.workspaceSvg.clearUndo();
Blockly.Events.setGroup('g1');
this.barDef.setEnabled(false);
Blockly.Events.setGroup(false);
for (let i = 0; i < 2; i++) {
chai.assert.isFalse(this.barCalls[i].isEnabled(),
'Callers are disabled when their definition is disabled (call ' +
i + ')');
}
const firedEvents = this.workspaceSvg.undoStack_;
chai.assert.equal(firedEvents.length, 3,
'An event was fired for the definition and each caller');
for (let i = 0; i < 3; i++) {
chai.assert.equal(firedEvents[i].group, 'g1',
'Disable events are in the same group (event ' + i + ')');
}
this.workspaceSvg.clearUndo();
Blockly.Events.setGroup('g2');
this.barDef.setEnabled(true);
Blockly.Events.setGroup(false);
for (let i = 0; i < 2; i++) {
chai.assert.isTrue(this.barCalls[i].isEnabled(),
'Callers are enabled when their definition is enabled (call ' +
i + ')');
}
chai.assert.equal(firedEvents.length, 3,
'An event was fired for the definition and each caller');
for (let i = 0; i < 3; i++) {
chai.assert.equal(firedEvents[i].group, 'g2',
'Enable events are in the same group (event ' + i + ')');
}
});
test('Set disabled updates callers while remembering old caller state', function() {
this.barCalls[0].setEnabled(false);
this.workspaceSvg.clearUndo();
Blockly.Events.setGroup('g1');
this.barDef.setEnabled(false);
Blockly.Events.setGroup(false);
for (let i = 0; i < 2; i++) {
chai.assert.isFalse(this.barCalls[i].isEnabled(),
'Callers are disabled when their definition is disabled (call ' +
i + ')');
}
const firedEvents = this.workspaceSvg.undoStack_;
chai.assert.equal(firedEvents.length, 2,
'An event was fired for the definition and the enabled caller');
for (let i = 0; i < 2; i++) {
chai.assert.equal(firedEvents[i].group, 'g1',
'Disable events are in the same group (event ' + i + ')');
}
this.workspaceSvg.clearUndo();
Blockly.Events.setGroup('g2');
this.barDef.setEnabled(true);
Blockly.Events.setGroup(false);
chai.assert.isFalse(this.barCalls[0].isEnabled(),
'Caller remains in disabled state when the definition is enabled');
chai.assert.isTrue(this.barCalls[1].isEnabled(),
'Caller returns to previous enabled state when the definition is enabled');
chai.assert.equal(firedEvents.length, 2,
'An event was fired for the definition and the enabled caller');
for (let i = 0; i < 2; i++) {
chai.assert.equal(firedEvents[i].group, 'g2',
'Enable events are in the same group (event ' + i + ')');
}
});
});
suite('Mutation', function() {
setup(function() {
this.defBlock = this.workspace.newBlock(testSuite.defType);
@@ -1701,33 +1632,6 @@ suite('Procedures', function() {
});
}
});
suite('Untyped Arguments', function() {
function assertArguments(argumentsArray) {
this.defBlock.arguments_ = argumentsArray;
const mutatorWorkspace = new Blockly.Workspace(
new Blockly.Options({
parentWorkspace: this.workspace,
}));
this.defBlock.decompose(mutatorWorkspace);
const argBlocks = mutatorWorkspace.getBlocksByType('procedures_mutatorarg');
chai.assert.equal(argBlocks.length, argumentsArray.length);
for (let i = 0; i < argumentsArray.length; i++) {
const argString = argumentsArray[i];
const argBlockValue = argBlocks[i].getFieldValue('NAME');
chai.assert.equal(argBlockValue, argString);
}
}
test('Simple Single Arg', function() {
assertArguments.call(this, ['arg']);
});
test('Multiple Args', function() {
assertArguments.call(this, ['arg1', 'arg2']);
});
test('<>', function() {
assertArguments.call(this, ['<>']);
});
});
});
});
/**