PlayerEx

PlayerEx

5M Downloads

Dying with Relics equipped and keepInventory on

sctjkc01 opened this issue ยท 4 comments

commented

I've been participating in a multiplayer session where we keep the Keep Inventory gamerule toggled on (just to mitigate frustration) and gradually over time we've noticed we were getting particularly overpowered. Enemies dropping a stack of "rare" drops, hundreds of health, wicked fast health regen and speed, nigh-invulnerable natural armor, and most notably the ability to not care about swimming in lava without Fire Resistance.

We just now pinned it down to Player Ex's relics system; for whatever reason, dying (including /kill) while a Relic is equipped and Keep Inventory is enabled will give the player a permanent copy of the Relic's stats that cannot be removed by any means. I don't know Forge, but I'd imagine the "on equip" event is firing on character respawn, but because of Keep Inventory "on unequip" doesn't.

commented

With the complete overhaul of the mod to API format and the removal of curios/items all together, this is no longer a relevant issue with the latest release. An addon mod bridging curios with this API will be releasing soon (this issue is not present with that mod either).

commented

Hi, thank you for making me aware of this issue.

Upon some more testing, it seems that even just leaving and rejoining a world results in a permanent increase in stats. I have found the issue: Curios does some silly things with its onEquipped method; when the player is cloned (dies/teleport etc.) if there are any curios on the original player onEquipped will be called no matter what on the new player (this is silly and I don't know why they do it like this!).

On top of this, onEquipped is called every time the player joins a world. These are issues on their end and even if I contacted them this is an older version of Minecraft now, and will likely not be updated. The only reason I am willing to fix this issue is because I have not yet finished reworking PlayerEx for 1.16.3, so 1.15.2 is still the latest version, which means that I am obliged to fix this issue.

That said, there are issues on my end too. When the player is cloned, I set all stats from the original player to the new player. This is necessary for stats that are not curio driven (i.e. levelled stats) to be maintained across players; however, this essentially causes all curio driven stats to be doubled whenever the player is cloned.

This is difficult to fix as generally there exists no way to differentiate stats from curios and stats from the player (levelled); and since I need to work around Curios' issues, an extra layer of difficulty presents itself there.

I am actively working towards a fix, but I am also exceedingly busy IRL at the moment. You can likely expect this to be fixed by December 20th, but that is a soft guarantee.

Again, thanks for pointing out this issue, and thank you for showing interest in my mod.

commented

Say. I've got a couple of questions I'd like to ask:

  1. What are your thoughts on external contributors to PlayerEx? Do you have any qualms with someone else submitting a bugfix?
  2. Is there anything in the design that would prevent creating a set of "purchased/base" copies of the base five IElements? I will admit the first thing I thought of was to create that set and copy those on a player clone (instead of the "active" set), in addition to adding it to the set that influences the other attributes.
  3. What is the order of events when a player respawns/joins/teleports/etc? If the Curio onEquipped events fire after the Forge onPlayerEntityCloned, that may make the above solution easy to implement.
commented

In reply to:

  1. I have no issues regarding contributions to PlayerEx; if you wish to submit a bugfix, I would be glad to merge it.
  2. This is indeed a better way to handle attributes, and it is how I plan to handle them in newer versions. Here is how I implemented this, and why it does not fix the issue (mostly):
  • Each element now has two components to determine total value: levelled bonus and equipped bonus.
  • Levelled bonus is handled by us, and equipped bonus is handled by curios.
  • This solves all bugs bar one: when leaving a world and then rejoining, curios still calls ICurio#onEquipped, and since this is an adder function, there will always be an increase in equipped values; hence, not really a solution.

Right now I can think of two ways to solve that: have each element contain four separate equipped values (one for each curios slot) and have the onEquipped method set the relevant value (which would then be added via the original adder function); or, have the onEquipped adder function be a fixed multiplicative setter. Both are bad solutions for multiple reasons.
3. The order of the events firing does have an impact, however the order of events fired has no influence on eliminating the bugs as the code causing the issue is housed in a TickEvent in Curios' event handler.

Again, if you come up with a full solution before I do and submit a pull request, I will gladly merge it. There is always a tidy solution, I have just not thought of it yet.