search

Gamua / Starling-Framework

The Cross Platform Game Engine
http://www.starling-framework.org
Other
2.82k stars 821 forks source link

Juggler: Fixed pooled tweens not properly returned in pool when purging the juggler #1101

Closed Adolio closed 1 year ago

Adolio commented 1 year ago

Hey Daniel,

Here is small fix regarding pooled tweens & juggler purging. Since the purge method was removing the objects without passing through an event, the pooled tweens weren't able to be released back to the pool.

Here I first try to remove the objects through events and then remove the remaining objects directly. This give a chance to the tween to get recycled.

I have also changed an event handler's name for better clarity.

Thanks in advance for your review & test.

Aurélien

Proposal for another PR

I would suggest to create another event Event.REMOVED_FROM_JUGGLER to indicate when an object is actually removed from the juggler (following the later refactoring, it's fairly simple to achieve). This would maybe simplify the situation. The pooled tweens could listen to this event instead. Here listening to Event.REMOVE_FROM_JUGGLER is a sort of override of a request for removal which is not super intuitive in my sense. This would also give a bit more flexibility for the users as well.

PrimaryFeather commented 1 year ago

Hey Aurélien, you're right, the purge method, in its original form, breaks pooling. That's a problem I hadn't noticed before! The same is true for the remove... methods.

However, I'm not particularly happy about the additional dispatch of REMOVE_FROM_JUGGLER. While it is — as you correctly noted — a request for the object to be removed from the juggler, it's currently only ever dispatched when the tween / delayed call is complete. Which means that people could rely on that behavior, and we might break code if we change that.

However, your additional suggestion would solve that problem nicely, and there wouldn't be a potential for breaking existing code.

Would you like to give that a try?

Thanks a lot for those suggestions, btw!! 👍

Adolio commented 1 year ago

I was hopping for this answer 😁!

Here it is: https://github.com/Gamua/Starling-Framework/pull/1103