search

iron431 / irons-spells-n-spellbooks

Other
77 stars 60 forks source link

[Bug] Memory Leak of Client Player #556

Open pietro-lopes opened 2 months ago

pietro-lopes commented 2 months ago

Observed behaviour

Continuation of #549

Now it is holding on ClientMagicData on 3.4.5

image

Expected behaviour

No leak

Steps to reproduce

Same as old issue.

Server Type

Single Player

Crashlog

No response

Iron's Spells N Spellbooks version

3.4.5

Forge version

1.21.1 - 21.1.22

Other mods

No response

iron431 commented 2 months ago

The SpellSelectionManager and the SpellBarOverlay do not store a reference to ClientMagicData, nor does any class

pietro-lopes commented 2 months ago

ClientMagicData stores spellSelectionManager which stores player

iron431 commented 2 months ago

That SpellSelectionManager is thrown away when the player exits the world https://github.com/iron431/irons-spells-n-spellbooks/blob/1.21/src/main/java/io/redspace/ironsspellbooks/player/ClientPlayerEvents.java#L70C15-L70C16

pietro-lopes commented 2 months ago

I see, but that does not include dimension change or death. Both triggers player creation again. We don't have something similar to PlayerEvent.StopTracking on client side, so I don't know which event should be used for both cases (player dim change and death)

I would just not store player and replace player with Minecraft.getInstance().player, this is not an expensive call, unless there is something I'm missing that you shouldn't use.

pietro-lopes commented 2 months ago

OH, I forgot about ClientPlayerNetworkEvent.Clone, you can listen to that to replace/clear player.