Handle booleans used as temp variables in p5.strands by davepagurek · Pull Request #8548 · processing/p5.js
Changes:
Usage of temp variables with booleans
Previously, if you made a temporary variable with a boolean value, it would break, e.g.:
let condition = 1 < 2 if (condition) { // ... }
I looked into a simple fix first, making all booleans into floats, so that we could make everything go through the same flow. However, the errors we were seeing were actually because we already assumed the variables to be floats. To make the initial error go away, every boolean operator would need to be wrapped in float(...) to be assignable to a float variable, but NOT when used in an if statement, as we would then get errors due to a boolean value being expected there.
Instead, I ended up updating our temp variable creation code to properly handle boolean types.
Usage of helper functions in JS
Another pattern that would break was this:
function conditionMet() { if (something) { return true } return false } if (conditionMet()) { // ... }
I thought this was related at first, but it's slightly different: the problem was in the early returns. We handle early returns special inp5.strands, turning them into a __p5.earlyReturn(val) statement so that we can output an early return node and continue through execution to generate nodes for the other paths too. But because we don't turn js helper functions into GLSL helper functions, early returns in a helper end up returning early in the whole hook.
So to address that, I added a step to the transpiler to make sure helpers always evaluate to a single strands node with one return. So the above helper now gets turned into this:
function conditionMet() { let returnValue if (something) { returnValue = true } else { // <-- Note this is an else now to have the same output! returnValue = false } return returnValue }
This introduced one new problem I needed to solve: let returnValue with no initializer. I made an ASSIGN_ON_USE data type for those nodes, which don't have a real type of their own. Type inference will take the value from whatever else they're operated on.
Minor: fix formatting in the transpiler
I noticed that half of the file is indented one too many times randomly. I de-indented that.
This file has become really big as our transpiler has had to do more, at some point it may make sense to refactor that. I think at least our unit tests are pretty decent for strands so that can help us feel confident that we haven't broken something when/if we embark upon that journey.
PR Checklist
-
npm run lintpasses - Inline reference is included / updated
- Unit tests are included / updated