Run validation code after every draw loop #22267
Conversation
There was a problem hiding this comment.
This LGTM for a first pass at validation.
Now would be a great time to establish some norms about ownership of these success functions, and who's on point to fix them when they break. There's a sort-of-scary future where we are afraid of making any API changes because we think we may break validation on some levels, but we don't know which ones (and have no way to test except manually one by one).
apps/src/gamelab/GameLab.js
Outdated
| __validationState = null; | ||
| __validationResult = null; | ||
| ${this.level.validationCode} | ||
| ret = { |
There was a problem hiding this comment.
Where does the magic ret come from?
There was a problem hiding this comment.
Ah, I was trying to workaround a bug in our intepreter (or rather the acorn parser?) that appears to choke on bare objects used as standalone expressions. Updated to wrap the code in a function and properly return the value I care about.
| function levelSuccess(testResult) { | ||
| __validationState = 'succeeded'; | ||
| __validationResult = testResult; | ||
| } |
There was a problem hiding this comment.
Can I auto-pass any level with the following code? 😄
There was a problem hiding this comment.
I should catch you with this line just before the validation code runs:
code-dot-org/apps/src/gamelab/GameLab.js
Line 1209 in 41a1ee4
Though if you try to use __validationState in your program it'll get reset every tick. The task of adding more reserved keywords becomes ever more pressing.
There was a problem hiding this comment.
Ah, makes sense. What does configuring additional reserved words look like? Will we need upstream support from https://github.com/NeilFraser/JS-Interpreter?
There was a problem hiding this comment.
I'm pretty sure we can handle it at the blockly code generation layer by adding "reserved words":
https://github.com/code-dot-org/blockly/blob/00b15c2a78946cdd7d17e808a594c99a6b54bb68/core/code_generation/generator.js#L299-L306
This will then generate slightly different variable names if you try to make a variable name that collides with a reserved word:
https://github.com/code-dot-org/blockly/blob/00b15c2a78946cdd7d17e808a594c99a6b54bb68/core/code_generation/names.js#L110-L131
There was a problem hiding this comment.
Nice, it looks like we're already doing this! Maybe add these as reserved words, and remove the double-underscore?
code-dot-org/apps/src/gamelab/GameLab.js
Lines 467 to 469 in bfa51eb
| = f.label 'Validation code' | ||
| %p | ||
| This is a snippet of javascript that is run after every draw loop. Call levelSuccess() in here to stop the level as a success. You can also pass a number into levelSuccess() to give the student a non-perfect test result (see <a href="https://github.com/code-dot-org/code-dot-org/blob/101fbfd2dcdd635d7359e991559ba782f9fd00be/apps/src/constants.js#L33-L106">TestResults</a> for the list of valid test results). | ||
| = f.text_area :validation_code |
There was a problem hiding this comment.
What is our API contract with level builders? If they depend on internal behavior, and we change internal behavior, who owns fixing?
Who is responsible for keeping the code working, and testing it after deploys?
There was a problem hiding this comment.
I'm not sure, but I have a couple thoughts:
- Ideally levelbuilders will only use standard Gamelab properties and methods (which are stable anyway), and things specifically provided by
ValidationSetup. We can add accessors toValidationSetupfor things exposed by the helper library for when levelbuilders need to inspect things like behaviors. - Unless we programmatically prevent access to helper library methods and properties, I suspect levelbuilders will wind up using them anyway, and my instinct is that it would be very difficult to stop them programmatically.
There was a problem hiding this comment.
Tagging @ryansloan to make sure we don't lose track of this question:
What is our API contract with level builders?
|
@joshlory @balderdash I will add API contract question to Curriculum/Product agenda for our next meeting. Also, yay validation! |
Adds a field for validation code on the gamelab editor page:
This snippet of code is run at the end of every draw loop. If the validation code calls
levelSucess(), the success dialog will pop up and you can continue to the next level. I also hide the 'finish' button on any levels with validation code.I got started with failure "validation", but we need to switch to CSF-style topinstructions before we can display failure feeback.