A couple issues came up during the Artist bug bash that shared a root cause related to the block.getVars method.

At the Artist Binary level /s/course4/lessons/18/levels/9?blocklyVersion=google, an error is displayed saying that we can't run the code:

This happens as part of a check to make sure nested for loops don't use the same variable.

This branch is based on #60448 because I also noticed that the controls_for_counter block was affected.

Explanation

Google Blockly has a standard implementation of block.getVars() that returns a list of variable names:
https://github.com/google/blockly/blob/17e4f1c96665dd56ff33f5a45db809f7e2805e82/core/block.ts#L1118-L1134

In CDO Blockly however, the same function returned an object with keys (representing variable types) and values (list of variable names of the given type). This explains the previous version of each version of getVars() that was modified in this branch. Most of these are old. In the case of migrated labs like Sprite Lab and Dance, we don't see to ever actually need or call these functions.

The outlier here is the implementation for Blockly.Variables.getVars which was added before Sprite Lab was migrated here:
https://github.com/code-dot-org/code-dot-org/pull/55276/files#diff-ff941d5d2ca36511c7401a50c5160671b0bba06e9bbd8f0d34ef21128e8a4f0bR22-R31

This was part of an effort to pre-emptively import missing variable methods from CDO Blockly. Unfortunately, that version wouldn't have worked as expected because it had the wrong return type for Google Blockly. Several functions in that PR (#55276) weren't able to be manually tested; we were just hoping that importing the CDO definitions would be helpful down the road.

After uncovering this descrepency, I was able to rework Blockly.Variables.getVars to just return a simple list of variable names. This function is assigned to several Artist blocks as block.getVars: variables_get_counter, variables_get_length, variables_get_sides, and controls_for_counter, all of which should now be working.

I did the same for several other custom getVars implementations, namely those used by blocks defined in our pools. These blocks are just found in Sprite Lab and Dance. Again, there doesn't seem to be any evidence that we've actually needed the block method in these labs which is why it hasn't been a problem. However, it makes sense to make them work correctly, should we need them in the future.

As a similar point of clean up, I noticed we also provided a custom getVars as part of the locationVariableDropdown custom input type. This was an experimental idea for a type of Sprite Lab block that would have worked with location variables (think var top = {x: 200, y: 400};. The idea was never fully implemented. In fact, there is no other reference to locationVariableDropdown in our entire main repo. Therefore, instead of updating this instance of getVar I instead deleted the entire custom input type.

Links

Bug Bash: https://docs.google.com/document/d/14nGoZMVSptvDnRt1He0t6JQBRf-Em3ZcSIMUoEPYMq0/edit

Testing story

I manually tested that previously-broken levels were now working.

It's also possible to get feedback about using nested loops with the same variable:

I confirmed that there were no regressions with blocks using the custom variable input or sprite picker in both Sprite Lab (shown) and Dance:

Deployment strategy

Follow-up work

Privacy

Security

Caching

PR Checklist:

• Tests provide adequate coverage
• Privacy and Security impacts have been assessed
• Code is well-commented
• New features are translatable or updates will not break translations
• Relevant documentation has been added or updated
• User impact is well-understood and desirable
• Pull Request is labeled appropriately
• Follow-up work items (including potential tech debt) are tracked and linked