Multiverse / Multiverse-Inventories

BSD 3-Clause "New" or "Revised" License
91 stars 81 forks source link

Multiverse-Inventories clears players inventory when changing worlds "too fast" #382

Open johananderssonv opened 3 years ago

johananderssonv commented 3 years ago

Hi! I have an issue where player inventories gets cleared once they leave MobArena. I opened a ticket on their page and this is their response.

@garbagemule: The issue is not with MobArena, but with the plugin handling multiple inventories. You can solve the issue by configuring your multi-inventory plugin such that it either synchronously loads inventories early in the event cycle (PerWorldInventory works fine in that regard) or doesn't swap inventories at all. MobArena does exactly what it is supposed to: it stores and then clears your inventory when you join, and it clears and then restores your inventory when you leave. The problem is that the plugin handling inventories arrives at the party too late.

So the question is; is there anyway to configure the delay for when mvinv loads the players inventories when switching worlds?

johananderssonv commented 3 years ago

I've tried using version 3.0.0 and just tried it with version 4.0.0 snapshot

garbagemule commented 3 years ago

I just want to chime in here with a bit of technical details in hopes of clearing up any questions that may crop up :)

When a player joins an arena by clicking a join sign or typing the join command, MobArena executes a series of steps in a very specific order (see below) to accommodate various state changes made by other plugins, but it is a same-tick procedure, so any async operations initiated from the events triggered by these steps will likely run "too late" for MobArena to pick them up. The order of (relevant) steps is the following:

  1. Teleport to arena lobby (a location in the arena).
  2. Change gamemode to survival.
  3. Store and then clear inventory.
  4. Store and then set various player properties (flying, health, hunger, experience).

When the player leaves the arena, those steps are "undone" in the reverse order, i.e.:

  1. Restore various player properties.
  2. Clear inventory and restore saved inventory.
  3. Restore gamemode.
  4. Teleport the player back to where they joined.

We had a bit of an issue with PerWorldInventory loading inventories asynchronously, causing MobArena to store and clear the "wrong" inventory in step 3. PWI would load the world-specific inventory after MobArena had already stored and cleared whatever was visible at the time of teleportation. Synchronous inventory loading seemed to be the solution over there.

I don't think this is a matter of inventories being wiped when players leave MobArena, but rather when they join, but I could be wrong. Because MobArena does the inventory juggling before any gamemode or world changes happen, I don't see it being an on-leave issue, because I don't assume Multiverse-Inventories would kick in until the time of teleportation.

I also don't see a sound solution to this from the MobArena side of things. We could add delays here and there, but it's brittle, and I don't think that's the right way to go about it. We could make the join/leave process asynchronous and then depend on some sort of event, but I'm not a fan of introducing arbitrary dependencies for the sake of workarounds.

Hope that helps!

Cheers, mule

Skellett1 commented 3 years ago

I had a similar issue with inventories overwriting others, might be part of the same origin, as in your case the first inventory is being cleared before transferring back to the other world. Details for my case: We're trying to reduce player health to half a heart utilizing AttributeModifiers on items, in our case golden boots. They're unbreakable and have the enchants curse of vanishing/binding. The modifier is limited to when they're worn on the feet. All of these attributes are hidden.

Nothing goes wrong when the user has these in any slot where they have no effect however if they're worn as intended and the player moves to a world which does not share the inventory, first the inventory gets cleared and then the inventory from the world with the boots is transferred over, this includes the ender chest. It seems to be somewhat flashy, you're actually able to see for a split second, that your inventory is empty before you then have the items which you should not have.

dumptruckman commented 3 years ago

You know, @garbagemule, long long ago I messaged you about this exact problem and suggested the code change to make in MobArena that would prevent interference with Multiverse-Inventories. I definitely could not tell you what that is now but I recall being certain back then that there was nothing I could do in Multiverse-Inventories to fix this issue and that the only solution was the one I suggested to you.

I guess the only option now is to dig back through the code to understand what is wrong. Multiverse-Inventories isn't doing any kind of async loading. It simply hooks into the PlayerChangedWorldEvent and the PlayerGameModeChangeEvent to save old inventories and load new ones.

garbagemule commented 3 years ago

@dumptruckman I'd be happy to review a patch if you can conjure one up. I don't recall any such suggestion, but I probably just forgot - sorry. If it was made here on Github we could probably find it again? Chances are that it is a long long time ago, and in that case, the join/leave process has changed a bit, and it is now very strongly defined in a series of small steps that do something in one direction and undo them in the other, ensuring the order on the way out is exactly the inverse of the order on the way in.

Last time I worked on multi-inventory issues it was with PerWorldInventory, and back then the solution was all about sync vs. async loading, and as long as it was configured to load inventories on the main thread, everything was fine. Maybe it listens for different events or uses different listener priorities? At any rate, it is certainly possible to make a multi-inventory plugin work correctly with MobArena. It's probably also possible to make MobArena work with all multi-inventory plugins. It could be that the necessary changes in either plugin are incompatible with the direction or architecture of the plugin, and I think that's fair.

If you're up for it, maybe a way to approach this from the Multiverse-Inventories side is to cut MobArena out of the equation and just mimic what it does? Teleport a player to a different world, change their gamemode, then store their inventory, wipe it and add some items to it - all in the same tick. Die and then respawn (in the same world). Then reverse the process: restore the stored inventory, restore the gamemode, and teleport the player back to where they came from - all in the same tick. Additionally try without dying and respawning, but just "join and then leave".

If the server handles inventories "correctly", then any changes made by Multiverse-Inventories should take effect during the teleport and gamemode changes, so by the time the inventory storing happens, all changes have already been made. Conversely, on the way out, by the time Multiverse-Inventories sees the gamemode change and the teleport, the inventory should be exactly the same as it was before it was wiped. In a strongly sequential process, I don't understand how this kind of glitch happens, but have I just missed something?

garbagemule commented 3 years ago

I just tried to see if I could reproduce the issue on my end. I could not provoke an inventory loss.


Here's what I did:

I also tried with spectate-on-death: false, and the only difference was that I wasn't sent to the spectator area but directly back to the test world, where my stone block was correctly restored.

Lastly, I tried with an exit warp defined in the same world as the arena world. Again, no inventory wipes of any kind.


@johananderssonv Could you try to identify how your setup differs from mine?

@dumptruckman I'd hold off on digging into the code base(s) until verifying that there is a conflict here.

IDzyre commented 3 years ago

If you have a lobby world that you spawn in, and you have a survival world, and a nether world linked, if you disconnect or go to the lobby world, and go through a multiverse portal to the survival world (with last_location on) it will not sync your inventory with the survival world, it will sync it to the lobby world, even if the survival world and the nether world have inventory shared.

benwoo1110 commented 3 years ago

@IDzyre doesn't look like your issues is related to this, so please create a new issue and follow the issue template.