Crying Ghasts (Forge)

Crying Ghasts (Forge)

273k Downloads

Use WeakHashMaps in CollectiveEvents

shartte opened this issue ยท 4 comments

commented

While fixing an AE2 issue related to collective, I came across the two hash maps in CollectiveEvents.
See:
https://github.com/ricksouth/serilum-mc-mods/blob/76f9ced01759b5d1296c5c8105f2aa212801e562/sources/Collective/src/main/java/com/natamus/collective/events/CollectiveEvents.java#L51

This is bad. Since the world unload event isn't used to clean up, collective will keep levels alive forever while the game is still running. This can add up if people hop between their single player worlds.

Potential Fix: Use WeakHashMap, allowing Java to clean up the keys.

commented

Appreciate you letting me know, that's not something I've come across yet. Do you recommend using WeakHashMap for any HashMap that references a ServerLevel? I believe I've got other mods which use the same principle.

commented

Yeah. You can also subscribe to ServerWorldEvents.UNLOAD and remove the map-entry in that event.

commented

Appreciate it. Is both making it into a WeakHashMap and subscribing to the unload event to remove the entry a good idea? Or would you suggest to pick one of the two?

commented

Collective version 4.57 has made the changes to WeakHashMaps. Thanks for the suggestion!