New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[PoemBot][HOC] Make foreground effects one-time bursts #42259
Conversation
@@ -2,7 +2,7 @@ | |||
"category": "", | |||
"config": { | |||
"func": "setForegroundEffect", | |||
"blockText": "set foreground effect {EFFECT}", | |||
"blockText": "do {EFFECT} effect", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[this can happen post-playtest] should we rename the func/file for this block?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that's a good callout. Changing the names of blocks is tough because it requires updating all the levels that use the block, and also breaks existing student projects that use the block. As a result, we've ended up with a bunch of block names that aren't quite right, but aren't worth fixing.
In this case, I held off on doing it as part of this change because I'm not sure whether this block is 100% settled or still iterating. Also, I didn't want to have to update all the playtest levels right away :) I think we should wait until after the playtest and get the block 100% finalized, then change the name before launch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perfect, i think that's the right direction here
}); | ||
confetti = confetti.filter(confetto => confetto.y < 420); | ||
this.p5.noStroke(); | ||
confetti.forEach(confetto => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol'ing that i never thought about the singular of "confetti"
break; | ||
} | ||
case 'bubbles': { | ||
let bubbles = []; | ||
this.foregroundEffect = () => { | ||
for (let i = 0; i < 25; i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit for this effect and others that now generate a set number of items in the effect: we should pull this value into a constant so we can easily identify/change how many bubbles are generated as part of the effect
Request from Amy: could we do all the foreground effects with a
do [dropdown]
and in the dropdown all the foreground effects are there? We basically do 1 second of the effect. That 1 second varies based on the when the animation "ends" for something like ripple or starburst.Basically, makes it so that instead of foreground effects continuously cycling through the effect, it just happens once.
There are two pieces here:
a658861
and1faffe5
.0f3684b
. There might be some additional parameter tuning needed here as follow-up work once Amy and Mike start playing with the effects.