Teacher Tool: Add Block Exist Criteria from Intro CS Curriculum #5644
Conversation
… output one, though we still need to update the block list)
…ty get/set blocks but we need some additional functionality to validate they're referencing the X and Y properties specifically. Block field validator can do it, but doesn't work well in nested validation (since it's on the same block) or as a side-by-side check (since count is variable).
…to thsparks/teachertool/rubric_to_checklist
…10004) This adds a validator plan to check for the number 0 block, which is useful if we want to ensure 0 was passed in as a parameter (used by microsoft/pxt-microbit#5644 to check for setting lives to 0 specifically, which produces output when non-zero values do not)
docs/teachertool/catalog.json
Outdated
| "id": "2CA4A5DA-4690-4514-97F5-2FE145AB3A59", | ||
| "use": "uses_led_coordinates", | ||
| "template": "Uses LED coordinates at least ${count} times", | ||
| "description": "Uses blocks that reference LED coordinates at least the specified number of times.", |
There was a problem hiding this comment.
Do we need a description for this criteria? It seems that the description for this is very similar to the template.
There was a problem hiding this comment.
I want to default to including descriptions, even if they feel a bit redundant. In this case, I think it's helpful to specify that we're looking specifically for blocks.
There was a problem hiding this comment.
Gotcha, makes sense. I find that "reference" is a bit of a confusing word to use here, would changing it with "access" or "check" still make sense for this description?
There was a problem hiding this comment.
Hmm "access" and "check" don't quite convey the correct meaning, in my mind. Perhaps "Uses blocks that accept LED coordinate inputs at least the specified number of times"?
| ] | ||
| }, | ||
| { | ||
| ".desc": "Runs code in response to any event (TODO : Non-Empty Check)", |
There was a problem hiding this comment.
Just wanted to ask a clarifying question for the difference between this validator and the uses_input validator. Is this validator looking at all of the open-mouthed "on-x-..." blocks and the uses_input validator looking at those blocks AND all the blocks that access something from the micro:bit board?
There was a problem hiding this comment.
More or less. uses_input is any kind of input (buttons, sound, etc...) and includes things like the "compass heading" and "sound level" blocks which aren't events.
This one is looking at any "event" (i.e. most of the open-mouthed) blocks, even ones which are not necessarily "inputs" (specifically, "music on [melody note played]").
| { | ||
| "validator": "blocksExist", | ||
| "blockCounts": [ | ||
| { | ||
| "blockId": "device_get_digital_pin", | ||
| "blockId": "radio_set_group", |
There was a problem hiding this comment.
I'm forgetting, did we set up the infrastructure to put validator plans in with the checks? That might be something interesting to think about going forward if not, but I'm just thinking out loud and it might not be worth the effort. This validator and the inputs and events validators made me think about this where we could split out some of the shared checks into a different validator, then list that validator as one of the checks. So in this case, you could put the radio_set_group check in a separate validator and then use that validator in the sends_radio_message and receives_radio_message to cut down on duplicates.
There was a problem hiding this comment.
Yeah, something like that might be helpful, but we don't support it yet, and it'd be nontrivial to implement. There'd be some potentially-complicated design implications to deal with and I didn't want to get into that mess in this change.
This adds several catalog criteria (and validator plans) based on the Intro CS Curriculum (see #5642 for details). Specifically, I've focused this change on the criteria that can be implemented using only block-exists checks with no additional functionality added.
Criteria included:
I've also removed two criteria:
There are a few things that'd be good to follow-up on with this change, notably:
All in all, while this was just about manageable for micro:bit (and there's a minimal expectation of changes, since a lot of this is limited by hardware), this is not going to be a scalable or maintainable approach for larger/more complex targets like Arcade. We may need to consider some level of runtime validation for that in the long-term...though that comes with its own set of challenges, since certain things may only happen if certain conditions are met.
Depends on microsoft/pxt#10004
Partially addresses #5642