code-dot-org / dance-party

Renderer for the Dance game type. Based on p5.js and p5.play.js.
https://code-dot-org.github.io/dance-party/
13 stars 13 forks source link

Break up background and foreground effects #673

Closed bencodeorg closed 11 months ago

bencodeorg commented 11 months ago

A proposed follow-up to https://github.com/code-dot-org/dance-party/pull/672 -- there is very little shared between background and foreground effects, so we could potentially totally separate them.

Some pros and cons I considered while putting this together -- I'm leaning towards this feeling like a good idea, but could be dissuaded :):

Pros:

Cons:

bencodeorg commented 11 months ago

Making a couple more changes here, will re-request review when they're done!

bencodeorg commented 11 months ago

Left a couple questions. Also curious if you were going to use the more modern syntax as @sanchitmalhotra126 had mentioned from #672 (review).

@fisher-alice re: modernizing syntax, I couldn't quite figure out why import statements aren't working, and it felt like work that could be picked up separately (maybe in conjunction with the "move effect definitions from object syntax to class syntax"?)

fisher-alice commented 11 months ago

Left a couple questions. Also curious if you were going to use the more modern syntax as @sanchitmalhotra126 had mentioned from #672 (review).

@fisher-alice re: modernizing syntax, I couldn't quite figure out why import statements aren't working, and it felt like work that could be picked up separately (maybe in conjunction with the "move effect definitions from object syntax to class syntax"?)

Sounds reasonable to me!