search

simiancraft / create-phaser-app

Phaser 3, Webpack 4, Babel 7+boilerplate and a scaffold
https://simiancraft.github.io/create-phaser-app/
Other
373 stars 29 forks source link

Fix to event listener memory leak #63

Closed carystanley closed 5 years ago

carystanley commented 5 years ago

Was taking a look at your code because I love how you organize the behaviors in an FSM. But I realized that the way you are handling animations, though it probably works, will eventually cause issues (a memory leak of sorts)

https://github.com/simiancraft/create-phaser-app/blob/master/src/sprites/player/animation-sequencer.js#L38-L49

Whenever you run an animation sequence, you are adding a new listener to the animation. This doesn't cause visible bugs because you are in a promise that which enforces that it in only resolved once. But your listener list keeps growing and growing. This will fix the memory leak, and align with what you intended.

the-simian commented 5 years ago

@carystanley hey THANK YOU for the PR. I had gotten the animation sequencer going and hadn't exhaustively tested it yet. I see that using .on is rebinding repeatedly, so you swapped with once. good catch!

I'm wrapping up some large pieces of work for my dayjob, and am ready to resume work on this as soon as I can, so expect updates in the near future. I'll go ahead and pull this in, and I'll publish it to npm after I test it!