Pr parameter fixes (#363)

* Make get parameter a dropdown

* Add warning when parameter is deleted

* Change to use i18n instead of hardcoded strings

* Remove check from loadExtraState

* Pulling checkParameterBlocks out

* Added a comment to make it clearer why the custom dropdown is needed

* Fixes #369

This adds the custom piece to FieldDropdown so it is shared between jump to step and get parameter blocks

* Move (no parameters) to i18n

* Fixed redundant check

Author: alan412

src/blocks/mrc_class_method_def.ts

@@ -37,7 +37,7 @@ import { FunctionData } from './utils/python_json_types';
 import { findConnectedBlocksOfType } from './utils/find_connected_blocks';
 import { makeLegalName } from './utils/validator';
 import { NONCOPYABLE_BLOCK } from './noncopyable_block';
-import { BLOCK_NAME as MRC_GET_PARAMETER_BLOCK_NAME } from './mrc_get_parameter';
+import { checkParameterBlocks, BLOCK_NAME as MRC_GET_PARAMETER_BLOCK_NAME } from './mrc_get_parameter';
 import * as paramContainer from './mrc_param_container'
 
 export const BLOCK_NAME = 'mrc_class_method_def';
@@ -226,6 +226,9 @@ const CLASS_METHOD_DEF = {
         mutateMethodCallers(this.workspace, this.mrcMethodId, methodForWithin);
       }
     }
+    
+    // Update all mrc_get_parameter blocks to recheck validity
+    checkParameterBlocks(this.getInputTargetBlock(INPUT_STACK));
   },
   decompose: function (this: ClassMethodDefBlock, workspace: Blockly.Workspace) {
     const parameterNames: string[] = [];

src/blocks/mrc_event_handler.ts

@@ -33,6 +33,7 @@ import { MRC_STYLE_EVENT_HANDLER } from '../themes/styles';
 import * as toolboxItems from '../toolbox/items';
 import * as storageModule from '../storage/module';
 import * as storageModuleContent from '../storage/module_content';
+import { checkParameterBlocks } from './mrc_get_parameter';
 
 export const BLOCK_NAME = 'mrc_event_handler';
 
@@ -265,7 +266,9 @@ const EVENT_HANDLER = {
                   type: arg.type,
                 });
               });
-              this.mrcUpdateParams();
+              this.mrcUpdateParams();              
+              // Update all mrc_get_parameter blocks to recheck validity
+              this.mrcCheckParameterBlocks();              
 
               // Since we found the mechanism event, we can break out of the loop.
               break;
@@ -328,6 +331,14 @@ const EVENT_HANDLER = {
     });
     return parameterNames;
   },
+
+  /**
+   * Checks all mrc_get_parameter blocks within this event handler to revalidate
+   * that their parameter names are still valid.
+   */
+  mrcCheckParameterBlocks: function(this: EventHandlerBlock): void {
+    checkParameterBlocks(this.getInputTargetBlock('DO'));
+  },
 };
 
 export function setup(): void {

src/blocks/mrc_get_parameter.ts

@@ -25,10 +25,11 @@ import {Order} from 'blockly/python';
 
 import { Editor } from '../editor/editor';
 import {ExtendedPythonGenerator} from '../editor/extended_python_generator';
-import {createFieldNonEditableText} from '../fields/FieldNonEditableText';
 import {MRC_STYLE_VARIABLES} from '../themes/styles';
 import {BLOCK_NAME as MRC_CLASS_METHOD_DEF, ClassMethodDefBlock} from './mrc_class_method_def';
 import {BLOCK_NAME as MRC_EVENT_HANDLER, EventHandlerBlock } from './mrc_event_handler';
+import { findConnectedBlocksOfType } from './utils/find_connected_blocks';
+import { CustomDropdownWithoutValidation } from '../fields/FieldDropdown';
 
 
 export const BLOCK_NAME = 'mrc_get_parameter';
@@ -70,11 +71,60 @@ const GET_PARAMETER_BLOCK = {
     this.mrcHasWarning = false;
 
     this.setStyle(MRC_STYLE_VARIABLES);
+    
+    const blockRef = this;
+    // Use a dummy initial option - it will be replaced when setValue is called
+    const dropdown: Blockly.Field = new CustomDropdownWithoutValidation(
+      function() {
+        // This function will be called to regenerate options when dropdown opens
+        return blockRef.getParameterOptions();
+      }
+    );
+    
+    dropdown.setValidator(this.validateParameterSelection.bind(this));
+    
     this.appendDummyInput()
         .appendField(Blockly.Msg.PARAMETER)
-        .appendField(createFieldNonEditableText(''), FIELD_PARAMETER_NAME);
+        .appendField(dropdown, FIELD_PARAMETER_NAME);
     this.setOutput(true, this.mrcParameterType);
   },
+  getParameterOptions: function(this: GetParameterBlock): [string, string][] {
+    const existingParameterNames: string[] = [];
+
+    const rootBlock: Blockly.Block | null = this.getRootBlock();
+    if (rootBlock && rootBlock.type === MRC_CLASS_METHOD_DEF) {
+      const classMethodDefBlock = rootBlock as ClassMethodDefBlock;
+      existingParameterNames.push(...classMethodDefBlock.mrcGetParameterNames());
+    } else if (rootBlock && rootBlock.type === MRC_EVENT_HANDLER) {
+      const eventHandlerBlock = rootBlock as EventHandlerBlock;
+      existingParameterNames.push(...eventHandlerBlock.mrcGetParameterNames());
+    }
+
+    // Get the field to check its value
+    const field = this.getField(FIELD_PARAMETER_NAME) as Blockly.FieldDropdown | null;
+    const currentValue = field?.getValue();
+    
+    // Always include the current field value if it exists and isn't already in the list
+    if (currentValue && !existingParameterNames.includes(currentValue)) {
+      existingParameterNames.unshift(currentValue);
+    }
+
+    if (existingParameterNames.length === 0) {
+      return [[Blockly.Msg.NO_PARAMETERS, '']];
+    }
+
+    return existingParameterNames.map(name => [name, name]);
+  },
+  validateParameterSelection: function(this: GetParameterBlock, newValue: string): string {
+    // Clear any previous warnings
+    this.setWarningText(null, WARNING_ID_NOT_IN_METHOD);
+    this.mrcHasWarning = false;
+    
+    // Options will be regenerated automatically on next dropdown open
+    // via the function passed to the CustomParameterDropdown constructor
+    
+    return newValue;
+  },
   setNameAndType: function(this: GetParameterBlock, name: string, type: string): void {
     this.setFieldValue(name, FIELD_PARAMETER_NAME);
     this.mrcParameterType = type;
@@ -105,21 +155,58 @@ const GET_PARAMETER_BLOCK = {
       legalParameterNames.push(...eventHandlerBlock.mrcGetParameterNames());
     }
 
-    if (legalParameterNames.includes(this.getFieldValue(FIELD_PARAMETER_NAME))) {
+    const currentParameterName = this.getFieldValue(FIELD_PARAMETER_NAME);
+    
+    if (legalParameterNames.includes(currentParameterName)) {
       // If this blocks's parameter name is in legalParameterNames, it's good.
       this.setWarningText(null, WARNING_ID_NOT_IN_METHOD);
       this.mrcHasWarning = false;
     } else {
       // Otherwise, add a warning to this block.
       if (!this.mrcHasWarning) {
-        this.setWarningText(Blockly.Msg.PARAMETERS_CAN_ONLY_GO_IN_THEIR_METHODS_BLOCK, WARNING_ID_NOT_IN_METHOD);
+        // Provide a more specific message depending on the situation
+        let warningMessage: string;
+        if (rootBlock.type === MRC_CLASS_METHOD_DEF || rootBlock.type === MRC_EVENT_HANDLER) {
+          // We're in a method/handler but the parameter doesn't exist
+          if (currentParameterName && currentParameterName !== '') {
+            const messageTemplate = rootBlock.type === MRC_CLASS_METHOD_DEF 
+              ? Blockly.Msg.PARAMETER_DOES_NOT_EXIST_IN_METHOD
+              : Blockly.Msg.PARAMETER_DOES_NOT_EXIST_IN_EVENT_HANDLER;
+            warningMessage = messageTemplate.replace('%1', currentParameterName);
+          } else {
+            warningMessage = Blockly.Msg.NO_PARAMETER_SELECTED;
+          }
+        } else {
+          // We're not even in a method/handler
+          warningMessage = Blockly.Msg.PARAMETERS_CAN_ONLY_GO_IN_THEIR_METHODS_BLOCK;
+        }
+        
+        this.setWarningText(warningMessage, WARNING_ID_NOT_IN_METHOD);
         this.getIcon(Blockly.icons.IconType.WARNING)!.setBubbleVisible(true);
         this.mrcHasWarning = true;
       }
     }
   },
+  /**
+   * Called to recheck parameter validity. Used when method parameters change.
+   */
+  mrcCheckParameter: function(this: GetParameterBlock): void {
+    this.checkBlockPlacement();
+  },
 };
 
+/*
+ * Rechecks all Get Parameter blocks connected to the target block.
+ */
+export function checkParameterBlocks(targetBlock: Blockly.Block | null): void {
+  if (targetBlock) {
+    findConnectedBlocksOfType(targetBlock, BLOCK_NAME).forEach((block) => {
+      const getParameterBlock = block as GetParameterBlock;
+      getParameterBlock.mrcCheckParameter();
+    });
+  }
+}
+
 export const setup = function() {
   Blockly.Blocks[BLOCK_NAME] = GET_PARAMETER_BLOCK;
 };

src/blocks/mrc_jump_to_step.ts

@@ -25,6 +25,7 @@ import { Editor } from '../editor/editor';
 import { ExtendedPythonGenerator } from '../editor/extended_python_generator';
 import { MRC_STYLE_VARIABLES } from '../themes/styles';
 import { BLOCK_NAME as MRC_STEPS, StepsBlock } from './mrc_steps'
+import { CustomDropdownWithoutValidation } from '../fields/FieldDropdown';
 
 export const BLOCK_NAME = 'mrc_jump_to_step';
 
@@ -49,23 +50,10 @@ const JUMP_TO_STEP_BLOCK = {
     this.mrcHasWarning = false;
 
     this.setStyle(MRC_STYLE_VARIABLES);
-    
-    // Create a custom dropdown that accepts any value and displays it correctly
-    class CustomStepDropdown extends Blockly.FieldDropdown {
-      override doClassValidation_(newValue?: string): string | null {
-        // Always accept the value, even if it's not in the current options
-        return newValue ?? null;
-      }
-      
-      override getText_(): string {
-        // Always return the current value, even if not in options
-        return this.value_ || '';
-      }
-    }
-    
+       
     const blockRef = this;
     // Use a function to dynamically generate options when dropdown opens
-    const dropdown: Blockly.Field = new CustomStepDropdown(
+    const dropdown: Blockly.Field = new CustomDropdownWithoutValidation(
       function() {
         // This function will be called to regenerate options when dropdown opens
         return blockRef.getStepOptions();

src/blocks/tokens.ts

@@ -36,6 +36,12 @@ 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'),
+    PARAMETER_DOES_NOT_EXIST_IN_METHOD:
+      t('BLOCKLY.PARAMETER_DOES_NOT_EXIST_IN_METHOD'),
+    PARAMETER_DOES_NOT_EXIST_IN_EVENT_HANDLER:
+      t('BLOCKLY.PARAMETER_DOES_NOT_EXIST_IN_EVENT_HANDLER'),
+    NO_PARAMETER_SELECTED:
+      t('BLOCKLY.NO_PARAMETER_SELECTED'),
     CLASS_METHOD_DEF_ALREADY_ON_WORKSPACE:
       t('BLOCKLY.CLASS_METHOD_DEF_ALREADY_ON_WORKSPACE'),
     EVENT_HANDLER_ALREADY_ON_WORKSPACE:

src/fields/FieldDropdown.ts

@@ -31,4 +31,24 @@ export function createFieldDropdown(items: string[]): Blockly.Field {
     options.push([item, item]);
   });
   return new Blockly.FieldDropdown(options);
+}
+
+/*
+ * Create a custom dropdown that accepts any value and displays it correctly
+ * This is necessary because we need to be able to force a parameter or step into the dropdown
+ * when we drag from it before it goes into a method or event that defines that parameter or a 
+ * step container that contains that step.
+ * 
+ * WARNING: This class relies on Blockly internals that are not part of the public API.
+ */
+export class CustomDropdownWithoutValidation extends Blockly.FieldDropdown {
+  override doClassValidation_(newValue?: string): string | null {
+    // Always accept the value, even if it's not in the current options
+    return newValue ?? null;
+  }
+  
+  override getText_(): string {
+    // Always return the current value, even if not in options
+    return this.value_ || '';
+  }
 }

src/i18n/locales/en/translation.json

@@ -131,6 +131,10 @@
         "PARAMETER": "parameter",
         "PARAMETERS": "Parameters",
         "PARAMETERS_CAN_ONLY_GO_IN_THEIR_METHODS_BLOCK": "Parameters can only go in their method's block",
+        "PARAMETER_DOES_NOT_EXIST_IN_METHOD": "Parameter \"%1\" does not exist in this method.",
+        "PARAMETER_DOES_NOT_EXIST_IN_EVENT_HANDLER": "Parameter \"%1\" does not exist in this event handler.",
+        "NO_PARAMETER_SELECTED": "No parameter selected.",
+        "NO_PARAMETERS": "(no parameters)",
         "JUMP_CAN_ONLY_GO_IN_THEIR_STEPS_BLOCK": "Jump can only go in their step's block",
         "STEP_DOES_NOT_EXIST_IN_STEPS": "Step \"%1\" does not exist in this steps block.",
         "NO_STEP_SELECTED": "No step selected.",

src/i18n/locales/es/translation.json

@@ -132,6 +132,10 @@
         "PARAMETER": "parámetro",
         "PARAMETERS": "Parámetros",
         "PARAMETERS_CAN_ONLY_GO_IN_THEIR_METHODS_BLOCK": "Los parámetros solo pueden ir en el bloque de su método",
+        "PARAMETER_DOES_NOT_EXIST_IN_METHOD": "El parámetro \"%1\" no existe en este método.",
+        "PARAMETER_DOES_NOT_EXIST_IN_EVENT_HANDLER": "El parámetro \"%1\" no existe en este controlador de eventos.",
+        "NO_PARAMETER_SELECTED": "No se ha seleccionado ningún parámetro.",
+        "NO_PARAMETERS": "(sin parámetros)",
         "JUMP_CAN_ONLY_GO_IN_THEIR_STEPS_BLOCK": "El salto solo puede ir en el bloque de su paso",
         "STEP_DOES_NOT_EXIST_IN_STEPS": "El paso \"%1\" no existe en este bloque de pasos.",
         "NO_STEP_SELECTED": "No se ha seleccionado ningún paso.",

src/i18n/locales/he/translation.json

@@ -131,6 +131,10 @@
     "PARAMETER": "פרמטר",
     "PARAMETERS": "פרמטרים",
     "PARAMETERS_CAN_ONLY_GO_IN_THEIR_METHODS_BLOCK": "פרמטרים יכולים ללכת רק בבלוק השיטה שלהם",
+    "PARAMETER_DOES_NOT_EXIST_IN_METHOD": "הפרמטר \"%1\" לא קיים בשיטה זו.",
+    "PARAMETER_DOES_NOT_EXIST_IN_EVENT_HANDLER": "הפרמטר \"%1\" לא קיים במטפל אירועים זה.",
+    "NO_PARAMETER_SELECTED": "לא נבחר פרמטר.",
+    "NO_PARAMETERS": "(אין פרמטרים)",
     "JUMP_CAN_ONLY_GO_IN_THEIR_STEPS_BLOCK": "קפיצה יכולה ללכת רק בבלוק הצעד שלה",
     "STEP_DOES_NOT_EXIST_IN_STEPS": "הצעד \"%1\" לא קיים בבלוק הצעדים הזה.",
     "NO_STEP_SELECTED": "לא נבחר צעד.",