When building the Methods category, use a button block instead of an mrc_class_method_def block to represent base class methods that are already on the workspace. (#314)

Author: lizlooney

src/blocks/mrc_class_method_def.ts

@@ -19,11 +19,14 @@
  * @fileoverview Blocks for class method definition
  * @author [email protected] (Alan Smith)
  */
+
 import * as Blockly from 'blockly';
+import { Order } from 'blockly/python';
+
+import type { MessageInstance } from 'antd/es/message/interface';
 import { MRC_STYLE_FUNCTIONS } from '../themes/styles';
 import { createFieldNonEditableText } from '../fields/FieldNonEditableText'
 import { createParameterFieldFlydown } from '../fields/field_flydown';
-import { Order } from 'blockly/python';
 import { ExtendedPythonGenerator } from '../editor/extended_python_generator';
 import * as storageModule from '../storage/module';
 import * as storageModuleContent from '../storage/module_content';
@@ -37,11 +40,14 @@ import * as paramContainer from './mrc_param_container'
 
 export const BLOCK_NAME = 'mrc_class_method_def';
 
+const BUTTON_CALLBACK_KEY = 'CLASS_METHOD_DEF_ALREADY_ON_WORKSPACE';
+const BUTTON_STYLE_PREFIX = 'classMethodDefButtonStyle_';
+
 const NO_RETURN_VALUE = 'None';
 const UNTYPED_RETURN_VALUE = '';
 
 const INPUT_TITLE = 'TITLE';
-export const FIELD_METHOD_NAME = 'NAME';
+const FIELD_METHOD_NAME = 'NAME';
 const FIELD_PARAM_PREFIX = 'PARAM_';
 const INPUT_STACK = 'STACK';
 export const INPUT_RETURN = 'RETURN';
@@ -542,31 +548,29 @@ export function createCustomMethodBlockWithReturn(): toolboxItems.Block {
 }
 
 export function getBaseClassBlocks(
-    baseClassName: string): toolboxItems.Block[] {
-  const blocks: toolboxItems.Block[] = [];
+    workspace: Blockly.WorkspaceSvg, baseClassName: string, methodNamesNotOverrideable: string[],
+    methodNamesAlreadyOverridden: string[]): toolboxItems.ContentsType[] {
+  const contents: toolboxItems.ContentsType[] = [];
   const classData = getClassData(baseClassName);
   if (classData) {
-    classData.instanceMethods.forEach(functionData => {
-      blocks.push(createClassMethodDefBlock(
-          functionData,
-          /* canChangeSignature */ false,
-          /* canBeCalledWithinClass */ false,
-          /* canBeCalledOutsideClass */ false,
-      ));
-    });
+    classData.instanceMethods
+        .filter(functionData => !methodNamesNotOverrideable.includes(functionData.functionName))
+        .forEach(functionData => {
+            if (methodNamesAlreadyOverridden.includes(functionData.functionName)) {
+              contents.push(createButton(workspace, functionData));
+            } else {
+              contents.push(createClassMethodDefBlock(functionData));
+            }
+        });
   }
-  return blocks;
+  return contents;
 }
 
-function createClassMethodDefBlock(
-    functionData: FunctionData,
-    canChangeSignature: boolean,
-    canBeCalledWithinClass: boolean,
-    canBeCalledOutsideClass: boolean): toolboxItems.Block {
+function createClassMethodDefBlock(functionData: FunctionData): toolboxItems.Block {
   const extraState: ClassMethodDefExtraState = {
-    canChangeSignature,
-    canBeCalledWithinClass,
-    canBeCalledOutsideClass,
+    canChangeSignature: false,
+    canBeCalledWithinClass: false,
+    canBeCalledOutsideClass: false,
     returnType: functionData.returnType,
     params: [],
   };
@@ -581,6 +585,15 @@ function createClassMethodDefBlock(
   return new toolboxItems.Block(BLOCK_NAME, extraState, fields, null);
 }
 
+function createButton(
+    workspace: Blockly.WorkspaceSvg, functionData: FunctionData): toolboxItems.Button {
+  // Use non-breakable spaces so it looks more like an class method def block.
+  const spaces = '\u00A0\u00A0';
+  const text = spaces + functionData.functionName + spaces;
+  const style = BUTTON_STYLE_PREFIX + workspace.getTheme().name;
+  return new toolboxItems.Button(text, BUTTON_CALLBACK_KEY, style);
+}
+
 // Misc
 
 export function getMethodsForWithin(
@@ -618,6 +631,12 @@ export function getMethodNamesAlreadyOverriddenInWorkspace(
   });
 }
 
+export function registerToolboxButton(workspace: Blockly.WorkspaceSvg, messageApi: MessageInstance) {
+  workspace.registerButtonCallback(BUTTON_CALLBACK_KEY, function (_button) {
+    messageApi.info(Blockly.Msg.CLASS_METHOD_DEF_ALREADY_ON_WORKSPACE);
+  });
+}
+
 /**
  * Upgrades the ClassMethodDefBlocks in the given workspace from version 002 to 003.
  * This function should only be called when upgrading old projects.

src/blocks/tokens.ts

@@ -36,6 +36,8 @@ export function customTokens(t: (key: string) => string): typeof Blockly.Msg {
     PARAMETERS: t('BLOCKLY.PARAMETERS'),
     PARAMETERS_CAN_ONLY_GO_IN_THEIR_METHODS_BLOCK:
       t('BLOCKLY.PARAMETERS_CAN_ONLY_GO_IN_THEIR_METHODS_BLOCK'),
+    CLASS_METHOD_DEF_ALREADY_ON_WORKSPACE:
+      t('BLOCKLY.CLASS_METHOD_DEF_ALREADY_ON_WORKSPACE'),
     EVENT_HANDLER_ALREADY_ON_WORKSPACE:
       t('BLOCKLY.EVENT_HANDLER_ALREADY_ON_WORKSPACE'),
     EVENT_HANDLER_ROBOT_EVENT_NOT_FOUND:
@@ -148,4 +150,4 @@ export function replaceTokens(template: string, values: Record<string, string>):
   return template.replace(/\{\{(\w+)\}\}/g, (match, token) => {
     return values[token] !== undefined ? values[token] : match;
   });
-}
+}

src/blocks/utils/python.ts

@@ -32,11 +32,25 @@ import * as SetPythonVariable from "../mrc_set_python_variable";
 // Utilities related to blocks for python modules and classes, including those from RobotPy, external samples, etc.
 
 export const MODULE_NAME_BLOCKS_BASE_CLASSES = 'blocks_base_classes';
+
 export const CLASS_NAME_ROBOT_BASE = MODULE_NAME_BLOCKS_BASE_CLASSES + '.RobotBase';
+export const ROBOT_METHOD_NAMES_NOT_OVERRIDEABLE: string[] = [
+  'define_hardware',
+  'fire_event',
+  'register_event_handler',
+  'unregister_event_handler',
+];
+
 export const CLASS_NAME_MECHANISM = MODULE_NAME_BLOCKS_BASE_CLASSES + '.Mechanism';
+export const MECHANISM_METHOD_NAMES_NOT_OVERRIDEABLE: string[] = [
+  'fire_event',
+  'register_event_handler',
+  'unregister_event_handler',
+];
 
 // TODO(lizlooney): what about PeriodicOpMode and LinearOpMode?
 export const CLASS_NAME_OPMODE = MODULE_NAME_BLOCKS_BASE_CLASSES + '.OpMode';
+export const OPMODE_METHOD_NAMES_NOT_OVERRIDEABLE: string[] = [];
 // TODO(lizlooney): Make sure to update the value of PERIODIC_METHOD_NAME when we update wpilib.
 export const PERIODIC_METHOD_NAME = 'loop';
 

src/i18n/locales/en/translation.json

@@ -126,6 +126,7 @@
         "PARAMETERS": "Parameters",
         "PARAMETERS_CAN_ONLY_GO_IN_THEIR_METHODS_BLOCK": "Parameters can only go in their method's block",
         "JUMP_CAN_ONLY_GO_IN_THEIR_STEPS_BLOCK": "Jump can only go in their step's block",
+        "CLASS_METHOD_DEF_ALREADY_ON_WORKSPACE": "This method is already on the workspace.",
         "EVENT_HANDLER_ALREADY_ON_WORKSPACE": "This event handler is already on the workspace.",
         "EVENT_HANDLER_ROBOT_EVENT_NOT_FOUND": "This block is an event handler for an event that no longer exists.",
         "EVENT_HANDLER_MECHANISM_EVENT_NOT_FOUND": "This block is an event handler for an event that no longer exists.",

src/i18n/locales/es/translation.json

@@ -127,6 +127,7 @@
         "PARAMETERS": "Parámetros",
         "PARAMETERS_CAN_ONLY_GO_IN_THEIR_METHODS_BLOCK": "Los parámetros solo pueden ir en el bloque de su método",
         "JUMP_CAN_ONLY_GO_IN_THEIR_STEPS_BLOCK": "El salto solo puede ir en el bloque de su paso",
+        "CLASS_METHOD_DEF_ALREADY_ON_WORKSPACE": "Este método ya está en el espacio de trabajo.",
         "EVENT_HANDLER_ALREADY_ON_WORKSPACE": "Este controlador de eventos ya está en el espacio de trabajo.",
         "EVENT_HANDLER_ROBOT_EVENT_NOT_FOUND": "Este bloque es un controlador de eventos para un evento que ya no existe.",
         "EVENT_HANDLER_MECHANISM_EVENT_NOT_FOUND": "Este bloque es un controlador de eventos para un evento que ya no existe.",

src/i18n/locales/he/translation.json

@@ -126,6 +126,7 @@
     "PARAMETERS": "פרמטרים",
     "PARAMETERS_CAN_ONLY_GO_IN_THEIR_METHODS_BLOCK": "פרמטרים יכולים ללכת רק בבלוק השיטה שלהם",
     "JUMP_CAN_ONLY_GO_IN_THEIR_STEPS_BLOCK": "קפיצה יכולה ללכת רק בבלוק הצעד שלה",
+    "CLASS_METHOD_DEF_ALREADY_ON_WORKSPACE": "שיטה זו כבר קיימת בסביבת העבודה.",
     "EVENT_HANDLER_ALREADY_ON_WORKSPACE": "מטפל אירועים זה כבר נמצא במרחב העבודה.",
     "EVENT_HANDLER_ROBOT_EVENT_NOT_FOUND": "הבלוק הזה הוא מנהל אירועים לאירוע שכבר לא קיים.",
     "EVENT_HANDLER_MECHANISM_EVENT_NOT_FOUND": "הבלוק הזה הוא מנהל אירועים לאירוע שכבר לא קיים.",

src/reactComponents/TabContent.tsx

@@ -30,7 +30,8 @@ import { extendedPythonGenerator } from '../editor/extended_python_generator';
 import * as storageModule from '../storage/module';
 import * as storageProject from '../storage/project';
 import * as commonStorage from '../storage/common_storage';
-import { registerToolboxButton } from '../blocks/mrc_event_handler';
+import * as classMethodDef from '../blocks/mrc_class_method_def'
+import * as eventHandler from '../blocks/mrc_event_handler'
 import { Content } from 'antd/es/layout/layout';
 
 /** Default size for code panel. */
@@ -130,7 +131,8 @@ export const TabContent = React.forwardRef<TabContentRef, TabContentProps>(({
   /** Called when workspace is created. */
   const setupWorkspace = React.useCallback((_modulePath: string, newWorkspace: Blockly.WorkspaceSvg) => {
     newWorkspace.addChangeListener(handleBlocksChanged);
-    registerToolboxButton(newWorkspace, messageApi);
+    classMethodDef.registerToolboxButton(newWorkspace, messageApi);
+    eventHandler.registerToolboxButton(newWorkspace, messageApi);
 
     const newEditor = new editor.Editor(
       newWorkspace,

src/themes/mrc_themes.ts

@@ -66,6 +66,9 @@ const create_themes = function (): Blockly.Theme[] {
         cssClasses +=
             '.eventHandlerButtonStyle_' + theme.name + ' {\n' +
             '    fill: ' + fill + ';\n' +
+            '}\n' +
+            '.classMethodDefButtonStyle_' + theme.name + ' {\n' +
+            '    fill: ' + fill + ';\n' +
             '}\n';
     });
     const styleElement = document.createElement('style');

src/toolbox/event_category.ts

@@ -63,12 +63,8 @@ class EventsCategory {
 
     // Add a block that lets the user define a new event.
     contents.push(
-      {
-        kind: 'label',
-        text: Blockly.Msg['CUSTOM_EVENTS_LABEL'],
-      },
-      createCustomEventBlock(storageNames.makeUniqueName('my_event', eventNames))
-    );
+        new toolboxItems.Label(Blockly.Msg['CUSTOM_EVENTS_LABEL']),
+        createCustomEventBlock(storageNames.makeUniqueName('my_event', eventNames)));
 
     // Get blocks for firing events defined in the current workspace.
     addFireEventBlocks(eventsFromWorkspace, contents);

src/toolbox/methods_category.ts

@@ -24,9 +24,18 @@ import * as Blockly from 'blockly/core';
 import * as toolboxItems from './items';
 import * as storageModule from '../storage/module';
 import { MRC_CATEGORY_STYLE_METHODS } from '../themes/styles';
-import { CLASS_NAME_ROBOT_BASE, CLASS_NAME_OPMODE, CLASS_NAME_MECHANISM } from '../blocks/utils/python';
+import {
+    CLASS_NAME_MECHANISM,
+    CLASS_NAME_OPMODE,
+    CLASS_NAME_ROBOT_BASE,
+    MECHANISM_METHOD_NAMES_NOT_OVERRIDEABLE,
+    OPMODE_METHOD_NAMES_NOT_OVERRIDEABLE,
+    ROBOT_METHOD_NAMES_NOT_OVERRIDEABLE } from '../blocks/utils/python';
 import { addInstanceWithinBlocks } from '../blocks/mrc_call_python_function';
-import { createCustomMethodBlock, getBaseClassBlocks, FIELD_METHOD_NAME, createCustomMethodBlockWithReturn } from '../blocks/mrc_class_method_def';
+import {
+    createCustomMethodBlock,
+    createCustomMethodBlockWithReturn,
+    getBaseClassBlocks } from '../blocks/mrc_class_method_def';
 import { createStepsBlock } from '../blocks/mrc_steps';
 import { Editor } from '../editor/editor';
 
@@ -50,10 +59,6 @@ export function getCategory(editor: Editor): toolboxItems.Category {
 }
 
 class MethodsCategory {
-  private robotClassBlocks = getBaseClassBlocks(CLASS_NAME_ROBOT_BASE);
-  private mechanismClassBlocks = getBaseClassBlocks(CLASS_NAME_MECHANISM);
-  private opmodeClassBlocks = getBaseClassBlocks(CLASS_NAME_OPMODE);
-
   public methodsFlyout(workspace: Blockly.WorkspaceSvg) {
     const editor = Editor.getEditorForBlocklyWorkspace(workspace);
     if (!editor) {
@@ -71,55 +76,34 @@ class MethodsCategory {
 
     switch (editor.getModuleType()) {
       case storageModule.ModuleType.ROBOT:
-        // TODO(lizlooney): We need a way to mark a method in python as not overridable.
-        // For example, in RobotBase, define_hardware, register_event_handler,
-        // unregister_event_handler, and fire_event should not be overridden in a user's robot.
-        const robotMethodNamesNotOverrideable: string[] = [
-          'define_hardware',
-          'fire_event',
-          'register_event_handler',
-          'unregister_event_handler',
-        ];
         // Add the methods for a Robot.
-        this.addClassBlocksForCurrentModule(
-            Blockly.Msg['MORE_ROBOT_METHODS_LABEL'], this.robotClassBlocks, robotMethodNamesNotOverrideable,
-            methodNamesAlreadyOverridden, contents);
+        this.addBaseClassBlocksForCurrentModule(
+            workspace, Blockly.Msg['MORE_ROBOT_METHODS_LABEL'], CLASS_NAME_ROBOT_BASE,
+            ROBOT_METHOD_NAMES_NOT_OVERRIDEABLE, methodNamesAlreadyOverridden, contents);
         break;
       case storageModule.ModuleType.MECHANISM:
-        // TODO(lizlooney): We need a way to mark a method in python as not overridable.
-        // For example, in Mechanism, register_event_handler, unregister_event_handler, and
-        // fire_event should not be overridden in a user's mechamism.
-        const mechanismMethodNamesNotOverrideable: string[] = [
-          'fire_event',
-          'register_event_handler',
-          'unregister_event_handler',
-        ];
         // Add the methods for a Mechanism.
-        this.addClassBlocksForCurrentModule(
-            Blockly.Msg['MORE_MECHANISM_METHODS_LABEL'], this.mechanismClassBlocks, mechanismMethodNamesNotOverrideable,
-            methodNamesAlreadyOverridden, contents);
+        this.addBaseClassBlocksForCurrentModule(
+            workspace, Blockly.Msg['MORE_MECHANISM_METHODS_LABEL'], CLASS_NAME_MECHANISM,
+            MECHANISM_METHOD_NAMES_NOT_OVERRIDEABLE, methodNamesAlreadyOverridden, contents);
         break;
       case storageModule.ModuleType.OPMODE:
         const hasSteps = editor.isStepsInWorkspace();
         if (!hasSteps) {
           contents.push(createStepsBlock());
         }
         // Add the methods for an OpMode.
-        this.addClassBlocksForCurrentModule(
-            Blockly.Msg['MORE_OPMODE_METHODS_LABEL'], this.opmodeClassBlocks, [],
-            methodNamesAlreadyOverridden, contents);
+        this.addBaseClassBlocksForCurrentModule(
+            workspace, Blockly.Msg['MORE_OPMODE_METHODS_LABEL'], CLASS_NAME_OPMODE,
+            OPMODE_METHOD_NAMES_NOT_OVERRIDEABLE, methodNamesAlreadyOverridden, contents);
         break;
     }
 
     // Add a block that lets the user define a new method.
     contents.push(
-      {
-        kind: 'label',
-        text: Blockly.Msg['CUSTOM_METHODS_LABEL'],
-      },
-      createCustomMethodBlock(),
-      createCustomMethodBlockWithReturn()
-    );
+        new toolboxItems.Label(Blockly.Msg['CUSTOM_METHODS_LABEL']),
+        createCustomMethodBlock(),
+        createCustomMethodBlockWithReturn());
 
     // Get blocks for calling methods defined in the current workspace.
     const methodsFromWorkspace = editor.getMethodsForWithinFromWorkspace();
@@ -132,31 +116,16 @@ class MethodsCategory {
     return toolboxInfo;
   }
 
-  private addClassBlocksForCurrentModule(
-      label: string, classBlocks: toolboxItems.Block[],
-      methodNamesNotOverrideable: string[],
-      methodNamesAlreadyOverridden: string[], contents: toolboxItems.ContentsType[]) {
-    let labelAdded = false;
-    for (const blockInfo of classBlocks) {
-      if (blockInfo.fields) {
-        const methodName = blockInfo.fields[FIELD_METHOD_NAME];
-        if (methodNamesNotOverrideable.includes(methodName)) {
-          continue;
-        }
-        if (methodNamesAlreadyOverridden.includes(methodName)) {
-          continue;
-        }
-        if (!labelAdded) {
-          contents.push(
-            {
-              kind: 'label',
-              text: label,
-            },
-          );
-          labelAdded = true;
-        }
-        contents.push(blockInfo);
-      }
+  private addBaseClassBlocksForCurrentModule(
+      workspace: Blockly.WorkspaceSvg, label: string, baseClassName: string,
+      methodNamesNotOverrideable: string[], methodNamesAlreadyOverridden: string[],
+      contents: toolboxItems.ContentsType[]) {
+    const baseClassBlocks: toolboxItems.ContentsType[] = getBaseClassBlocks(
+        workspace, baseClassName, methodNamesNotOverrideable, methodNamesAlreadyOverridden);
+    if (baseClassBlocks.length) {
+      contents.push(
+          new toolboxItems.Label(label),
+          ...baseClassBlocks);
     }
   }
 }