Plastix / PixelPilot

A top down dogfighting game with pixel graphics.
0 stars 0 forks source link

Listeners are indestructible #56

Closed JEphron closed 9 years ago

JEphron commented 9 years ago

There is currently no system in place to remove listeners from the events map. This means that there will always be a reference in the map, and setting the listener to null elsewhere will not cause it to be GC'd.

Plastix commented 9 years ago

Why would we want to remove listeners?

JEphron commented 9 years ago

PlaneMarker is a listener. What happens to it when the plane it's pointing at dies?

Plastix commented 9 years ago

Ooh I didn't think about that. On Feb 27, 2015 11:13 AM, "JEphron" notifications@github.com wrote:

PlaneMarker is a listener. What happens to it when the plane it's pointing at dies?

— Reply to this email directly or view it on GitHub https://github.com/Plastix/PixelPilot/issues/56#issuecomment-76420224.

JEphron commented 9 years ago

Definitely going to need to think about the structure here. First thing that comes to mind is having some sort of flag indicating that the object is destroyed. Second idea would be throwing a destroyEvent containing the object to be destroyed.

EyalSel commented 9 years ago

I spent ~10 minutes looking at the source, but here goes: On plane death (or on the destruction of any object with listeners), add its listeners to a buffer.

At the end of each game iteration you empty the buffer and remove all of those listeners from the map.

That's better than the headache of an extra flag, and it also avoids iterating through everything in search of raised flags at the end of every game iteration.

EDIT: In the case that the listeners are inaccessible from the object itself, a flag will be needed. If the object that a Listener listens to is flagged, that listener is added to the remove_these_listeners_from_map queue.

Plastix commented 9 years ago

It migh be worth it just making an unsubscribe method in Events and calling that when needed.

JEphron commented 9 years ago

Currently investigating WeakReference as a possible solution...

Plastix commented 9 years ago

Should be fixed by #73 but needs some testing. Alternatively, we can switch to mbassador with #74.

Plastix commented 9 years ago

Fixed by #74