diff --git a/src/blocks/mrc_call_python_function.ts b/src/blocks/mrc_call_python_function.ts index fd2b86e8..08e3c7f4 100644 --- a/src/blocks/mrc_call_python_function.ts +++ b/src/blocks/mrc_call_python_function.ts @@ -63,6 +63,8 @@ export type FunctionArg = { type: string, }; +const WARNING_ID_FUNCTION_CHANGED = 'function changed'; + // Functions used for creating blocks for the toolbox. export function addModuleFunctionBlocks( @@ -220,13 +222,12 @@ function createInstanceMethodBlock( } export function getInstanceComponentBlocks( - componentType: string, - componentName: string): ToolboxItems.ContentsType[] { + component: CommonStorage.Component): ToolboxItems.ContentsType[] { const contents: ToolboxItems.ContentsType[] = []; - const classData = getClassData(componentType); + const classData = getClassData(component.className); if (!classData) { - throw new Error('Could not find classData for ' + componentType); + throw new Error('Could not find classData for ' + component.className); } const functions = classData.instanceMethods; @@ -241,7 +242,7 @@ export function getInstanceComponentBlocks( if (findSuperFunctionData(functionData, componentFunctions)) { continue; } - const block = createInstanceComponentBlock(componentName, functionData); + const block = createInstanceComponentBlock(component, functionData); contents.push(block); } @@ -249,18 +250,19 @@ export function getInstanceComponentBlocks( } function createInstanceComponentBlock( - componentName: string, functionData: FunctionData): ToolboxItems.Block { + component: CommonStorage.Component, functionData: FunctionData): ToolboxItems.Block { const extraState: CallPythonFunctionExtraState = { functionKind: FunctionKind.INSTANCE_COMPONENT, returnType: functionData.returnType, args: [], tooltip: functionData.tooltip, importModule: '', - componentClassName: functionData.declaringClassName, - componentName: componentName, + componentClassName: component.className, + componentName: component.name, + componentBlockId: component.blockId, }; const fields: {[key: string]: any} = {}; - fields[FIELD_COMPONENT_NAME] = componentName; + fields[FIELD_COMPONENT_NAME] = component.name; fields[FIELD_FUNCTION_NAME] = functionData.functionName; const inputs: {[key: string]: any} = {}; // For INSTANCE_COMPONENT functions, the 0 argument is 'self', but @@ -307,6 +309,7 @@ function createInstanceRobotBlock(method: CommonStorage.Method): ToolboxItems.Bl returnType: method.returnType, actualFunctionName: method.pythonName, args: [], + classMethodDefBlockId: method.blockId, }; const fields: {[key: string]: any} = {}; fields[FIELD_FUNCTION_NAME] = method.visibleName; @@ -337,7 +340,7 @@ function createInstanceRobotBlock(method: CommonStorage.Method): ToolboxItems.Bl //.............................................................................. -export type CallPythonFunctionBlock = Blockly.Block & CallPythonFunctionMixin; +export type CallPythonFunctionBlock = Blockly.Block & CallPythonFunctionMixin & Blockly.BlockSvg; interface CallPythonFunctionMixin extends CallPythonFunctionMixinType { mrcFunctionKind: FunctionKind, mrcReturnType: string, @@ -345,9 +348,11 @@ interface CallPythonFunctionMixin extends CallPythonFunctionMixinType { mrcTooltip: string, mrcImportModule: string, mrcActualFunctionName: string, - mrcExportedFunction: boolean, + mrcClassMethodDefBlockId: string, mrcComponentClassName: string, - mrcComponentName: string, // Do not access directly. Call getComponentName. + mrcComponents: CommonStorage.Component[], + mrcComponentName: string, + mrcComponentBlockId: string, renameMethod(this: CallPythonFunctionBlock, newName: string): void; mutateMethod(this: CallPythonFunctionBlock, defBlockExtraState: ClassMethodDefExtraState): void; } @@ -385,9 +390,9 @@ type CallPythonFunctionExtraState = { */ actualFunctionName?: string, /** - * True if this blocks refers to an exported function (for example, from the Robot). + * The id of the mrc_class_method_def type that defines the method. Specified only if the function kind is INSTANCE_ROBOT. */ - exportedFunction?: boolean, + classMethodDefBlockId?: string, /** * The component name. Specified only if the function kind is INSTANCE_COMPONENT. */ @@ -396,6 +401,10 @@ type CallPythonFunctionExtraState = { * The component class name. Specified only if the function kind is INSTANCE_COMPONENT. */ componentClassName?: string, + /** + * The id of the mrc_component type that defines the component. Specified only if the function kind is INSTANCE_COMPONENT. + */ + componentBlockId?: string, }; const CALL_PYTHON_FUNCTION = { @@ -444,7 +453,7 @@ const CALL_PYTHON_FUNCTION = { const className = this.mrcComponentClassName; const functionName = this.getFieldValue(FIELD_FUNCTION_NAME); tooltip = 'Calls the instance method ' + className + '.' + functionName + - ' on the component named ' + this.getComponentName() + '.'; + ' on the component named ' + this.getFieldValue(FIELD_COMPONENT_NAME) + '.'; break; } case FunctionKind.INSTANCE_ROBOT: { @@ -487,14 +496,20 @@ const CALL_PYTHON_FUNCTION = { if (this.mrcActualFunctionName) { extraState.actualFunctionName = this.mrcActualFunctionName; } + if (this.mrcClassMethodDefBlockId) { + extraState.classMethodDefBlockId = this.mrcClassMethodDefBlockId; + } if (this.mrcComponentClassName) { extraState.componentClassName = this.mrcComponentClassName; } if (this.getField(FIELD_COMPONENT_NAME)) { - extraState.componentName = this.getComponentName(); - } - if (this.mrcExportedFunction) { - extraState.exportedFunction = this.mrcExportedFunction; + extraState.componentName = this.getFieldValue(FIELD_COMPONENT_NAME); + for (const component of this.mrcComponents) { + if (component.name == extraState.componentName) { + extraState.componentBlockId = component.blockId; + break; + } + } } return extraState; }, @@ -519,12 +534,25 @@ const CALL_PYTHON_FUNCTION = { ? extraState.importModule : ''; this.mrcActualFunctionName = extraState.actualFunctionName ? extraState.actualFunctionName : ''; - this.mrcExportedFunction = extraState.exportedFunction - ? extraState.exportedFunction : false; + this.mrcClassMethodDefBlockId = extraState.classMethodDefBlockId + ? extraState.classMethodDefBlockId : ''; this.mrcComponentClassName = extraState.componentClassName ? extraState.componentClassName : ''; + // Get the list of components whose type matches this.mrcComponentClassName so we can + // create a dropdown that has the appropriate component name choices. + this.mrcComponents = []; + const editor = Editor.getEditorForBlocklyWorkspace(this.workspace); + if (editor) { + editor.getComponentsFromRobot().forEach(component => { + if (component.className === this.mrcComponentClassName) { + this.mrcComponents.push(component); + } + }); + } this.mrcComponentName = extraState.componentName ? extraState.componentName : ''; + this.mrcComponentBlockId = extraState.componentBlockId + ? extraState.componentBlockId : ''; this.updateBlock_(); }, /** @@ -596,25 +624,14 @@ const CALL_PYTHON_FUNCTION = { break; } case FunctionKind.INSTANCE_COMPONENT: { - const componentNames: string[] = []; - // Get the list of component names whose type matches this.mrcComponentClassName so we can - // create a dropdown that has the appropriate component names. - const editor = Editor.getEditorForBlocklyWorkspace(this.workspace); - if (editor) { - const components = editor.getComponentsFromRobot(); - components.forEach(component => { - if (component.className === this.mrcComponentClassName) { - componentNames.push(component.name); - } - }); - } - const componentName = this.getComponentName(); - if (!componentNames.includes(componentName)) { - componentNames.push(componentName); + const componentNameChoices = []; + this.mrcComponents.forEach(component => componentNameChoices.push(component.name)); + if (!componentNameChoices.includes(this.mrcComponentName)) { + componentNameChoices.push(this.mrcComponentName); } this.appendDummyInput('TITLE') .appendField('call') - .appendField(createFieldDropdown(componentNames), FIELD_COMPONENT_NAME) + .appendField(createFieldDropdown(componentNameChoices), FIELD_COMPONENT_NAME) .appendField('.') .appendField(createFieldNonEditableText(''), FIELD_FUNCTION_NAME); break; @@ -660,15 +677,6 @@ const CALL_PYTHON_FUNCTION = { this.removeInput('ARG' + i); } }, - getComponentName(this: CallPythonFunctionBlock): string { - // If the COMPONENT_NAME field has been created, get the field value, which the user may have changed. - // If the COMPONENT_NAME field has not been created, get the component name from this.mrcComponentName. - return (this.getField(FIELD_COMPONENT_NAME)) - ? this.getFieldValue(FIELD_COMPONENT_NAME) : this.mrcComponentName; - }, - getComponentClassName(this: CallPythonFunctionBlock): string { - return this.mrcComponentClassName; - }, renameMethod: function(this: CallPythonFunctionBlock, newName: string): void { this.setFieldValue(newName, FIELD_FUNCTION_NAME); }, @@ -686,6 +694,115 @@ const CALL_PYTHON_FUNCTION = { }); this.updateBlock_(); }, + onLoad: function(this: CallPythonFunctionBlock): void { + const warnings: string[] = []; + + // If this block is calling a component method, check that the component + // still exists and hasn't been changed. + // If the component doesn't exist, put a visible warning on this block. + // If the component has changed, update the block if possible or put a + // visible warning on it. + if (this.mrcFunctionKind === FunctionKind.INSTANCE_COMPONENT) { + let foundComponent = false; + for (const component of this.mrcComponents) { + if (component.blockId === this.mrcComponentBlockId) { + foundComponent = true; + + // If the component name has changed, we can handle that. + if (this.getFieldValue(FIELD_COMPONENT_NAME) !== component.name) { + // Replace the FIELD_COMPONENT_NAME field. + const titleInput = this.getInput('TITLE') + if (titleInput) { + let indexOfComponentName = -1; + for (let i = 0, field; (field = titleInput.fieldRow[i]); i++) { + if (field.name === FIELD_COMPONENT_NAME) { + indexOfComponentName = i; + break; + } + } + if (indexOfComponentName != -1) { + const componentNameChoices = []; + this.mrcComponents.forEach(component => componentNameChoices.push(component.name)); + titleInput.removeField(FIELD_COMPONENT_NAME); + titleInput.insertFieldAt(indexOfComponentName, + createFieldDropdown(componentNameChoices), FIELD_COMPONENT_NAME); + } + this.setFieldValue(component.name, FIELD_COMPONENT_NAME); + } + } + + // Since we found the component, we can break out of the loop. + break; + } + } + if (!foundComponent) { + warnings.push('This block calls a method on a component that no longer exists.'); + } + + // TODO(lizlooney): Could the component's method have change? + } + + // If this block is calling a robot method, check that the robot method + // still exists and hasn't been changed. + // If the robot method doesn't exist, put a visible warning on this block. + // If the robot method has changed, update the block if possible or put a + // visible warning on it. + if (this.mrcFunctionKind === FunctionKind.INSTANCE_ROBOT) { + let foundRobotMethod = false; + const editor = Editor.getEditorForBlocklyWorkspace(this.workspace); + if (editor) { + const robotMethods = editor.getMethodsFromRobot(); + for (const robotMethod of robotMethods) { + if (robotMethod.blockId === this.mrcClassMethodDefBlockId) { + foundRobotMethod = true; + + // If the function name has changed, we can fix this block. + if (this.mrcActualFunctionName !== robotMethod.pythonName) { + this.mrcActualFunctionName = robotMethod.pythonName; + } + if (this.getFieldValue(FIELD_FUNCTION_NAME) !== robotMethod.visibleName) { + this.setFieldValue(robotMethod.visibleName, FIELD_FUNCTION_NAME); + } + + // Other things are more difficult. + if (this.mrcReturnType !== robotMethod.returnType) { + warnings.push('This block calls a method whose return type has changed.'); + } + if (this.mrcArgs.length !== robotMethod.args.length - 1) { + warnings.push('This block calls a method whose arguments have changed.'); + } else { + for (let i = 1; i < robotMethod.args.length; i++) { // Skip the self argument. + if (this.mrcArgs[i-1].name != robotMethod.args[i].name) { + warnings.push('This block calls a method whose arguments have changed.'); + break; + } + if (this.mrcArgs[i-1].type != robotMethod.args[i].type) { + warnings.push('This block calls a method whose arguments have changed.'); + break; + } + } + } + + // Since we found the robot method, we can break out of the loop. + break; + } + } + if (!foundRobotMethod) { + warnings.push('This block calls a method that no longer exists.'); + } + } + } + + if (warnings.length) { + // Add a warnings to the block. + const warningText = warnings.join('\n\n'); + this.setWarningText(warningText, WARNING_ID_FUNCTION_CHANGED); + this.bringToFront(); + } else { + // Clear the existing warning on the block. + this.setWarningText(null, WARNING_ID_FUNCTION_CHANGED); + } + }, }; export const setup = function() { @@ -748,7 +865,7 @@ export const pythonFromBlock = function( break; } case FunctionKind.INSTANCE_COMPONENT: { - const componentName = callPythonFunctionBlock.getComponentName(); + const componentName = block.getFieldValue(FIELD_COMPONENT_NAME); const functionName = callPythonFunctionBlock.mrcActualFunctionName ? callPythonFunctionBlock.mrcActualFunctionName : block.getFieldValue(FIELD_FUNCTION_NAME); diff --git a/src/blocks/mrc_class_method_def.ts b/src/blocks/mrc_class_method_def.ts index a14c7f32..58050626 100644 --- a/src/blocks/mrc_class_method_def.ts +++ b/src/blocks/mrc_class_method_def.ts @@ -404,6 +404,7 @@ export const pythonFromBlock = function ( if (block.mrcCanBeCalledOutsideClass) { // Update the mrcMethod. block.mrcMethod = { + blockId: block.id, visibleName: block.getFieldValue('NAME'), pythonName: funcName, returnType: block.mrcReturnType, diff --git a/src/blocks/mrc_get_python_variable.ts b/src/blocks/mrc_get_python_variable.ts index a7196c32..bbdc824f 100644 --- a/src/blocks/mrc_get_python_variable.ts +++ b/src/blocks/mrc_get_python_variable.ts @@ -191,7 +191,6 @@ interface GetPythonVariableMixin extends GetPythonVariableMixinType { mrcSelfType: string, mrcImportModule: string, mrcActualVariableName: string, - mrcExportedVariable: boolean, mrcKey: string, } type GetPythonVariableMixinType = typeof GET_PYTHON_VARIABLE; @@ -227,10 +226,6 @@ type GetPythonVariableExtraState = { * the FIELD_VARIABLE_NAME field. */ actualVariableName?: string, - /** - * True if this blocks refers to an exported variable (for example, from the Robot). - */ - exportedVariable?: boolean, }; const GET_PYTHON_VARIABLE = { @@ -289,7 +284,6 @@ const GET_PYTHON_VARIABLE = { const extraState: GetPythonVariableExtraState = { varKind: this.mrcVarKind, moduleOrClassName: this.mrcModuleOrClassName, - exportedVariable: this.mrcExportedVariable, }; if (this.mrcVarType) { extraState.varType = this.mrcVarType; @@ -324,8 +318,6 @@ const GET_PYTHON_VARIABLE = { ? extraState.importModule : ''; this.mrcActualVariableName = extraState.actualVariableName ? extraState.actualVariableName : ''; - this.mrcExportedVariable = extraState.exportedVariable - ? extraState.exportedVariable : false; this.mrcKey = createKey(this.mrcVarKind, this.mrcModuleOrClassName, this.mrcVarType); this.updateBlock_(); }, diff --git a/src/blocks/mrc_mechanism_component_holder.ts b/src/blocks/mrc_mechanism_component_holder.ts index dfa7e817..49311057 100644 --- a/src/blocks/mrc_mechanism_component_holder.ts +++ b/src/blocks/mrc_mechanism_component_holder.ts @@ -144,6 +144,7 @@ const MECHANISM_COMPONENT_HOLDER = { if (componentName && componentType) { components.push({ + blockId: componentBlock.id, name: componentName, className: componentType, }); diff --git a/src/blocks/mrc_set_python_variable.ts b/src/blocks/mrc_set_python_variable.ts index 218decf2..a59aecde 100644 --- a/src/blocks/mrc_set_python_variable.ts +++ b/src/blocks/mrc_set_python_variable.ts @@ -141,7 +141,6 @@ interface SetPythonVariableMixin extends SetPythonVariableMixinType { mrcSelfType: string, mrcImportModule: string, mrcActualVariableName: string, - mrcExportedVariable: boolean, mrcKey: string, } type SetPythonVariableMixinType = typeof SET_PYTHON_VARIABLE; @@ -177,10 +176,6 @@ type SetPythonVariableExtraState = { * the FIELD_VARIABLE_NAME field. */ actualVariableName?: string, - /** - * True if this blocks refers to an exported variable (for example, from the Robot). - */ - exportedVariable?: boolean, }; const SET_PYTHON_VARIABLE = { @@ -239,7 +234,6 @@ const SET_PYTHON_VARIABLE = { const extraState: SetPythonVariableExtraState = { varKind: this.mrcVarKind, moduleOrClassName: this.mrcModuleOrClassName, - exportedVariable: this.mrcExportedVariable, }; if (this.mrcVarType) { extraState.varType = this.mrcVarType; @@ -274,8 +268,6 @@ const SET_PYTHON_VARIABLE = { ? extraState.importModule : ''; this.mrcActualVariableName = extraState.actualVariableName ? extraState.actualVariableName : ''; - this.mrcExportedVariable = extraState.exportedVariable - ? extraState.exportedVariable : false; this.mrcKey = createKey(this.mrcVarKind, this.mrcModuleOrClassName, this.mrcVarType); this.updateBlock_(); }, diff --git a/src/editor/editor.ts b/src/editor/editor.ts index 1d6375a7..6bbce4d8 100644 --- a/src/editor/editor.ts +++ b/src/editor/editor.ts @@ -24,8 +24,9 @@ import * as Blockly from 'blockly/core'; import { extendedPythonGenerator } from './extended_python_generator'; import { GeneratorContext } from './generator_context'; import * as commonStorage from '../storage/common_storage'; -import * as mechanismComponentHolder from '../blocks/mrc_mechanism_component_holder'; +import * as callPythonFunction from '../blocks/mrc_call_python_function'; import * as classMethodDef from '../blocks/mrc_class_method_def'; +import * as mechanismComponentHolder from '../blocks/mrc_mechanism_component_holder'; //import { testAllBlocksInToolbox } from '../toolbox/toolbox_tests'; import { MethodsCategory } from '../toolbox/methods_category'; import { EventsCategory } from '../toolbox/event_category'; @@ -72,15 +73,19 @@ export class Editor { return; } - // TODO(lizlooney): Look at blocks with type 'mrc_call_python_function' that - // are calling methods defined in the Robot and check that the Robot method - // still exists and hasn't been changed. If it has changed, update the block - // if possible or put a visible warning on it. - - // TODO(lizlooney): Look at blocks with type 'mrc_call_python_function' that - // are calling methods on components defined in the Robot and check that the - // component and the method still exists and hasn't been changed. If it has - // changed, update the block if possible or put a visible warning on it. + // Look at mrc_call_python_function blocks that might need updating if the + // function definition has changed. + if (event.type === Blockly.Events.BLOCK_CREATE) { + const blockCreateEvent = event as Blockly.Events.BlockCreate; + if (blockCreateEvent.ids) { + blockCreateEvent.ids.forEach(id => { + const block = this.blocklyWorkspace.getBlockById(id); + if (block && block.type == callPythonFunction.BLOCK_NAME) { + (block as callPythonFunction.CallPythonFunctionBlock).onLoad(); + } + }); + } + } } private onChangeAfterLoading(event: Blockly.Events.Abstract) { diff --git a/src/storage/common_storage.ts b/src/storage/common_storage.ts index 97adfba3..7d59650b 100644 --- a/src/storage/common_storage.ts +++ b/src/storage/common_storage.ts @@ -59,6 +59,7 @@ export type MethodArg = { }; export type Method = { + blockId: string, // ID of the mrc_class_method_def block that defines the method. visibleName: string, pythonName: string, returnType: string, // 'None' for no return value, '' for an untyped return value. @@ -66,6 +67,7 @@ export type Method = { }; export type Component = { + blockId: string, // ID of the mrc_component block that defines the component. name: string, className: string, } diff --git a/src/toolbox/hardware_category.ts b/src/toolbox/hardware_category.ts index c12dfd58..73a17770 100644 --- a/src/toolbox/hardware_category.ts +++ b/src/toolbox/hardware_category.ts @@ -227,7 +227,7 @@ function getRobotComponentsBlocks(): toolboxItems.Category { contents.push({ kind: 'category', name: component.name, - contents: getInstanceComponentBlocks(component.className, component.name), + contents: getInstanceComponentBlocks(component), }); }); } @@ -288,7 +288,7 @@ function getComponentsBlocks(hideParams : boolean): toolboxItems.Category { contents.push({ kind: 'category', name: component.name, - contents: getInstanceComponentBlocks(component.className, component.name), + contents: getInstanceComponentBlocks(component), }); }); }