TarVK / pixi-shadows

Adds dynamic shadows to PixiJS
https://tarvk.github.io/pixi-shadows/dist/
MIT License
53 stars 14 forks source link

clean up mixins #11

Open amir-arad opened 2 years ago

amir-arad commented 2 years ago

looking back at this code, I think the Container mixin is pretty bad. This is obviously my fault, I basically knew nothing about PIXI nor even webGL/openGL when I wrote any of this. In any case, I am quite sure that the things the Container mixin does, are essentially done by pixi-layers also, but probably a lot cleaner. So I think that if we care to improve things in the future, we can probably exploit existing layer mechanics more, instead of monkey-patching something together like I did.

Also looking back at the code, using a "mixin" like this is quite questionable in the first place as explained before. Even if not relying more on pixi-layers, I kind of feel like it would be better to provide a helper function that performs this children pushing stuff (for flexibility) and additionally defining a Container class ourselves, which just extends the original Container class provided by pixi. Usage of pixi-shadows would then become a bit less transparent to the user though, which is a bit of a downside.

(from #10 discussion)

if staying with the current approach, consider consolidating them, with the tick logic, into one hook logic as I did here

amir-arad commented 2 years ago

@TarVK what should replace the mixing approach?