search

CivMC / Civ

Monorepo for development of and running a Civ Server.
MIT License
4 stars 10 forks source link

Revert "Discard entities before adding NPCs" #535

Closed okx-code closed 1 month ago

okx-code commented 1 month ago

This reverts commit 5d2151f1cd458b66db140586924c28cb20cb698d, from PR https://github.com/CivMC/CombatTagPlus/pull/21.

This fixes players losing their bed position as well as players losing their crafting recipes and xp (https://github.com/CivMC/Civ/issues/93), because the fact that the UUID is the same, combined with https://github.com/CivMC/Civ/blob/f525b7afeb0fa63e4cef3e48c3ec1d7dc790fc5d/plugins/exilepearl-paper/src/main/java/com/devotedmc/ExilePearl/listener/PlayerListener.java#L303, causes an interaction where the player's saved data is overriden with the NPC's data, which has most things copied but not the bed location. This change seems pretty broken, and was only applied recently with the 1.20 update, so I decided to roll it back until we implement a fix for everything.

Alternatively, we could make sure we are copying all of the data to the NPC, but since this has caused ~two~ three (https://github.com/CivMC/Civ/issues/93) independent issues already, it may cause more, and I think the safest option is to roll it back.

github-actions[bot] commented 1 month ago

badge

Build Successful! You can find a link to the downloadable artifact below.

Name Link
Commit f82f88797414e6067293546678db12fc5bd6f7d7
Logs https://github.com/CivMC/Civ/actions/runs/9072234704
Download https://github.com/CivMC/Civ/suites/23746188821/artifacts/
Protonull commented 1 month ago

Given how fraught with bugs combat loggers have been, it might be worth looking into custom Paper patches. These issues all stem from CombatTagPlus being a plugin, and so its approach can only be to duplicate the player entity upon logout. But this has led to many, many bugs, including dupes, because combat-loggers are consequential: they're meant to be the player.

Whereas, if combat loggers were implemented through patches (or mixins), you wouldn't need to create a duplicate entity. You could just prevent the server from deleting the player-entity when they logout. There'd likely still be issues to iron out, but having the same entity to work with would be a lot easier than playing Whack-A-Mole with these combat-logger bugs.