Don’t allow 2 event handlers for the same event in the same module

src/blocks/mrc_event.ts

@@ -238,11 +238,11 @@ export const pythonFromBlock = function (
 
 // Functions used for creating blocks for the toolbox.
 
-export function createCustomEventBlock(): toolboxItems.Block {
+export function createCustomEventBlock(name: string): toolboxItems.Block {
   const extraState: EventExtraState = {
     params: [],
   };
   const fields: {[key: string]: any} = {};
-  fields[FIELD_EVENT_NAME] = 'my_event';
+  fields[FIELD_EVENT_NAME] = name;
   return new toolboxItems.Block(BLOCK_NAME, extraState, fields, null);
 }

src/blocks/mrc_event_handler.ts

@@ -226,9 +226,6 @@ export function pythonFromBlock(
   const blocklyName = `${sender}_${eventName}`;
   const funcName = generator.getProcedureName(blocklyName);
 
-  // TODO(lizlooney): if the user adds multiple event handlers for the same event
-  // name, we need to make the event handler function names unique.
-
   let xfix1 = '';
   if (generator.STATEMENT_PREFIX) {
     xfix1 += generator.injectId(generator.STATEMENT_PREFIX, block);
@@ -323,8 +320,16 @@ function createRobotEventHandlerBlock(
 
 // Misc
 
-export function getHasEventHandler(workspace: Blockly.Workspace): boolean {
+export function getHasAnyEnabledEventHandlers(workspace: Blockly.Workspace): boolean {
   return workspace.getBlocksByType(BLOCK_NAME).filter(block => {
     return block.isEnabled();
   }).length > 0;
 }
+
+export function getEventHandlerNames(workspace: Blockly.Workspace, names: string[]): void {
+  // Here we collect the event names of the event handlers in the given
+  // workspace, regardless of whether the event handler is enabled.
+  workspace.getBlocksByType(BLOCK_NAME).forEach(block => {
+    names.push(block.getFieldValue(FIELD_EVENT_NAME));
+  });
+}

src/editor/editor.ts

@@ -31,6 +31,7 @@ import * as mechanismComponentHolder from '../blocks/mrc_mechanism_component_hol
 //import { testAllBlocksInToolbox } from '../toolbox/toolbox_tests';
 import { MethodsCategory } from '../toolbox/methods_category';
 import { EventsCategory } from '../toolbox/event_category';
+import { RobotEventsCategory } from '../toolbox/hardware_category';
 import { getToolboxJSON } from '../toolbox/toolbox';
 
 const EMPTY_TOOLBOX: Blockly.utils.toolbox.ToolboxDefinition = {
@@ -44,8 +45,6 @@ export class Editor {
   private blocklyWorkspace: Blockly.WorkspaceSvg;
   private generatorContext: GeneratorContext;
   private storage: commonStorage.Storage;
-  private methodsCategory: MethodsCategory;
-  private eventsCategory: EventsCategory;
   private currentModule: commonStorage.Module | null = null;
   private modulePath: string = '';
   private robotPath: string = '';
@@ -59,8 +58,10 @@ export class Editor {
     this.blocklyWorkspace = blocklyWorkspace;
     this.generatorContext = generatorContext;
     this.storage = storage;
-    this.methodsCategory = new MethodsCategory(blocklyWorkspace);
-    this.eventsCategory = new EventsCategory(blocklyWorkspace);
+    // Create the custom toolbox categories so they register their flyout callbacks.
+    new MethodsCategory(blocklyWorkspace);
+    new EventsCategory(blocklyWorkspace);
+    new RobotEventsCategory(blocklyWorkspace);
   }
 
   private onChangeWhileLoading(event: Blockly.Events.Abstract) {
@@ -107,8 +108,6 @@ export class Editor {
   public async loadModuleBlocks(currentModule: commonStorage.Module | null) {
     this.generatorContext.setModule(currentModule);
     this.currentModule = currentModule;
-    this.methodsCategory.setCurrentModule(currentModule);
-    this.eventsCategory.setCurrentModule(currentModule);
 
     if (currentModule) {
       this.modulePath = currentModule.modulePath;
@@ -201,6 +200,13 @@ export class Editor {
     return this.getModuleContentText() !== this.moduleContentText;
   }
 
+  public getCurrentModuleType(): string {
+    if (this.currentModule) {
+      return this.currentModule.moduleType;
+    }
+    return commonStorage.MODULE_TYPE_UNKNOWN;
+  }
+
   private getModuleContentText(): string {
     if (!this.currentModule) {
       throw new Error('getModuleContentText: this.currentModule is null.');
@@ -258,6 +264,12 @@ export class Editor {
     return events;
   }
 
+  public getEventHandlerNamesFromWorkspace(): string[] {
+    const names: string[] = [];
+    eventHandler.getEventHandlerNames(this.blocklyWorkspace, names);
+    return names;
+  }
+
   public async saveBlocks() {
     const moduleContentText = this.getModuleContentText();
     try {

src/editor/extended_python_generator.ts

@@ -134,7 +134,7 @@ export class ExtendedPythonGenerator extends PythonGenerator {
 
     this.ports = Object.create(null);
     this.hasHardware = mechanismContainerHolder.getHardwarePorts(this.workspace, this.ports);
-    this.hasEventHandler = eventHandler.getHasEventHandler(this.workspace);
+    this.hasEventHandler = eventHandler.getHasAnyEnabledEventHandlers(this.workspace);
 
     const code = super.workspaceToCode(workspace);
 

src/storage/common_storage.ts

@@ -629,25 +629,10 @@ export function getClassNameForModule(moduleType: string, moduleName: string) {
  * Make a unique project name for an uploaded project.
  */
 export function makeUploadProjectName(
-  uploadFileName: string, existingProjectNames: string[]): string {
+    uploadFileName: string, existingProjectNames: string[]): string {
   const preferredName = uploadFileName.substring(
     0, uploadFileName.length - UPLOAD_DOWNLOAD_FILE_EXTENSION.length);
-  let name = preferredName; // No suffix.
-  let suffix = 0;
-  while (true) {
-    let nameClash = false;
-    for (const existingProjectName of existingProjectNames) {
-      if (name == existingProjectName) {
-        nameClash = true;
-        break;
-      }
-    }
-    if (!nameClash) {
-      return name;
-    }
-    suffix++;
-    name = preferredName + suffix;
-  }
+  return makeUniqueName(preferredName, existingProjectNames);
 }
 
 /**
@@ -704,3 +689,25 @@ export function _processUploadedModule(
   const moduleContentText = moduleContent.getModuleContentText();
   return [moduleName, moduleType, moduleContentText];
 }
+
+/**
+ * Makes a unique name given a list of existing names
+ */
+export function makeUniqueName(preferredName: string, existingNames: string[]): string {
+  let name = preferredName; // No suffix.
+  let suffix = 0;
+  while (true) {
+    let nameClash = false;
+    for (const existingName of existingNames) {
+      if (name == existingName) {
+        nameClash = true;
+        break;
+      }
+    }
+    if (!nameClash) {
+      return name;
+    }
+    suffix++;
+    name = preferredName + suffix;
+  }
+}

src/toolbox/event_category.ts

@@ -38,32 +38,31 @@ export const getCategory = () => ({
 });
 
 export class EventsCategory {
-  private currentModule: commonStorage.Module | null = null;
-
   constructor(blocklyWorkspace: Blockly.WorkspaceSvg) {
     blocklyWorkspace.registerToolboxCategoryCallback(CUSTOM_CATEGORY_EVENTS, this.eventsFlyout.bind(this));
   }
 
-  public setCurrentModule(currentModule: commonStorage.Module | null) {
-    this.currentModule = currentModule;
-  }
-
   public eventsFlyout(workspace: Blockly.WorkspaceSvg) {
     const contents: toolboxItems.ContentsType[] = [];
 
-    // Add a block that lets the user define a new event.
-    contents.push(
-      {
-        kind: 'label',
-        text: 'Custom Events',
-      },
-      createCustomEventBlock()
-    );
-
-    // Get blocks for firing methods defined in the current workspace.
     const editor = Editor.getEditorForBlocklyWorkspace(workspace);
     if (editor) {
       const eventsFromWorkspace = editor.getEventsFromWorkspace();
+      const eventNames: string[] = [];
+      eventsFromWorkspace.forEach(event => {
+        eventNames.push(event.name);
+      });
+
+      // Add a block that lets the user define a new event.
+      contents.push(
+        {
+          kind: 'label',
+          text: 'Custom Events',
+        },
+        createCustomEventBlock(commonStorage.makeUniqueName('my_event', eventNames))
+      );
+
+      // Get blocks for firing methods defined in the current workspace.
       addFireEventBlocks(eventsFromWorkspace, contents);
     }
 
@@ -73,4 +72,4 @@ export class EventsCategory {
 
     return toolboxInfo;
   }
-}
+}

src/toolbox/hardware_category.ts

@@ -34,31 +34,31 @@ export function getHardwareCategory(currentModule: commonStorage.Module): toolbo
       kind: 'category',
       name: Blockly.Msg['MRC_CATEGORY_HARDWARE'],
       contents: [
-        getRobotMechanismsBlocks(currentModule),
-        getComponentsBlocks(false),
+        getRobotMechanismsCategory(currentModule),
+        getComponentsCategory(false),
       ]
     };
   }
   if (currentModule.moduleType === commonStorage.MODULE_TYPE_MECHANISM) {
-    return getComponentsBlocks(true);
+    return getComponentsCategory(true);
   }
   if (currentModule.moduleType === commonStorage.MODULE_TYPE_OPMODE) {
     return {
       kind: 'category',
       name: Blockly.Msg['MRC_CATEGORY_ROBOT'],
       contents: [
-        getRobotMechanismsBlocks(currentModule),
-        getRobotComponentsBlocks(),
-        getRobotMethodsBlocks(),
-        getRobotEventsBlocks(),
+        getRobotMechanismsCategory(currentModule),
+        getRobotComponentsCategory(),
+        getRobotMethodsCategory(),
+        getRobotEventsCategory(),
       ]
     };
   }
   throw new Error('currentModule.moduleType has unexpected value: ' + currentModule.moduleType)
 }
 
-function getRobotMechanismsBlocks(currentModule: commonStorage.Module): toolboxItems.Category {
-  // getRobotMechanismsBlocks is called when the user is editing the robot or an opmode.
+function getRobotMechanismsCategory(currentModule: commonStorage.Module): toolboxItems.Category {
+  // getRobotMechanismsCategory is called when the user is editing the robot or an opmode.
   // If the user is editing the robot, it allows the user to add a mechanism to
   // the robot or use an existing mechanism.
   // If the user is editing an opmode, it allows the user to use a mechanism that
@@ -211,8 +211,8 @@ function getRobotMechanismsBlocks(currentModule: commonStorage.Module): toolboxI
   };
 }
 
-function getRobotComponentsBlocks(): toolboxItems.Category {
-  // getRobotComponentsBlocks is called when the user is editing an opmode.
+function getRobotComponentsCategory(): toolboxItems.Category {
+  // getRobotComponentsCategory is called when the user is editing an opmode.
   // It allows the user to use a component that was previously added to the Robot.
 
   const contents: toolboxItems.ContentsType[] = [];
@@ -242,8 +242,8 @@ function getRobotComponentsBlocks(): toolboxItems.Category {
   };
 }
 
-function getRobotMethodsBlocks(): toolboxItems.Category {
-  // getRobotMethodsBlocks is called when the user is editing an opmode.
+function getRobotMethodsCategory(): toolboxItems.Category {
+  // getRobotMethodsCategory is called when the user is editing an opmode.
   // It allows the user to use methods there previously defined in the Robot.
 
   const contents: toolboxItems.ContentsType[] = [];
@@ -266,8 +266,8 @@ function getRobotMethodsBlocks(): toolboxItems.Category {
   };
 }
 
-function getComponentsBlocks(hideParams : boolean): toolboxItems.Category {
-  // getComponentsBlocks is called when the user is editing the robot or a
+function getComponentsCategory(hideParams : boolean): toolboxItems.Category {
+  // getComponentsCategory is called when the user is editing the robot or a
   // mechanism. It allows the user to add a component or use an existing component.
 
   const contents: toolboxItems.ContentsType[] = [];
@@ -303,26 +303,41 @@ function getComponentsBlocks(hideParams : boolean): toolboxItems.Category {
   };
 }
 
-function getRobotEventsBlocks(): toolboxItems.Category {
-  // getRobotEventsBlocks is called when the user is editing an opmode.
-  // It allows the user to create event handlers for events previously defined in the Robot.
+const CUSTOM_CATEGORY_ROBOT_EVENTS = 'ROBOT_EVENTS';
 
-  const contents: toolboxItems.ContentsType[] = [];
+// The robot events category is shown when the user is editing an opmode.
+// It allows the user to create event handlers for events previously defined in the Robot.
+const getRobotEventsCategory = () => ({
+  kind: 'category',
+  name: Blockly.Msg['MRC_CATEGORY_EVENTS'],
+  custom: CUSTOM_CATEGORY_ROBOT_EVENTS,
+});
+
+export class RobotEventsCategory {
+  constructor(blocklyWorkspace: Blockly.WorkspaceSvg) {
+    blocklyWorkspace.registerToolboxCategoryCallback(CUSTOM_CATEGORY_ROBOT_EVENTS, this.robotEventsFlyout.bind(this));
+  }
+
+  public robotEventsFlyout(workspace: Blockly.WorkspaceSvg) {
+    const contents: toolboxItems.ContentsType[] = [];
+
+    // Get the list of events from the robot and add the blocks for handling events.
 
-  // Get the list of events from the robot and add the blocks for calling the
-  // robot functions.
-  const workspace = Blockly.getMainWorkspace();
-  if (workspace) {
     const editor = Editor.getEditorForBlocklyWorkspace(workspace);
     if (editor) {
       const eventsFromRobot = editor.getEventsFromRobot();
-      addRobotEventHandlerBlocks(eventsFromRobot, contents);
+      // Remove events if there is already a corresponding handler in the workspace.
+      const eventHandlerNames = editor.getEventHandlerNamesFromWorkspace();
+      const eventsToShow = eventsFromRobot.filter(event => {
+        return !eventHandlerNames.includes(event.name);
+      });
+      addRobotEventHandlerBlocks(eventsToShow, contents);
     }
-  }
 
-  return {
-    kind: 'category',
-    name: Blockly.Msg['MRC_CATEGORY_EVENTS'],
-    contents,
-  };
+    const toolboxInfo = {
+      contents: contents,
+    };
+
+    return toolboxInfo;
+  }
 }