floppaxd / DeepMobEvolution

DML rewrite adding new features and improvements.
4 stars 6 forks source link

Check side for LivingDeathEvent #36

Open jchung01 opened 5 months ago

jchung01 commented 5 months ago

killedEntityUUIDBlacklist in EntityDeathEventHandler is not a thread-safe list, and entityDeath(LivingDeathEvent) was firing twice, from the client and server. This would cause undefined behavior with crashes that looked like this and this. It looks like the code should only be run server side, so added a check.

jchung01 commented 5 months ago

Actually, this may only be an issue in Java 9+, I'll reopen this after further investigation.

jchung01 commented 5 months ago

I verified through debugging on runClient with regular Forge that LivingDeathEvent is fired twice, so I think this is okay to pull in. However, not entirely sure why killedEntityUUIDBlacklist is necessary if it's just tracking duplicate mob instances to blacklist, which I don't know in which circumstance can happen. Maybe the list was made because of this issue of sharing it between client and server in an integrated server (single-player) case? In which case, usage of the blacklist may be removed altogether.

jchung01 commented 1 month ago

@ThePansmith Can this be merged and a release published to CurseForge? It's a simple fix.