elBukkit / MagicPlugin

A Bukkit plugin for spells, wands and other magic
http://mine.elmakers.com
MIT License
242 stars 148 forks source link

Inventory Overwriting Glitch #1047

Closed Gyztor closed 2 years ago

Gyztor commented 2 years ago

Something odd that happens when the server restarts is that if people are in their wand inventory there is no handler to close the inventory before its saved. This overwrites the inventory with the spells you had in your wand inventory and deleting all items in the inventory, i have tried disabling and enabling "reopen_wand_on_join" and usually just keep it on false for personal preferences. Luckily CMI is set to backup inventories OFTEN but this is a pretty big issue. My hypothesis is that inventories aren't backed up immediately to file and are still in memory before the plugin stops/the server shuts down (probable but less likely) OR the inventories aren't being switched back to the original inventory before shut down (more likely). These hypothesis come from the behavior and timing of when this happens.

NathanWolf commented 2 years ago

This is probably a conflict with CMI.

Gyztor commented 2 years ago

What option in CMI do you think it could be? as i dont have cmi edit any inventories just save them on an hourly basis

NathanWolf commented 2 years ago

I don't really know, I'm just guessing sorry :(

I can't remember the specifics, but I know for sure people on Discord have had issues with CMI before. If it ever restores the inventory backups it makes, it is possible it will restore the spell inventory rather than the survival inventory.

I know it's not a great solution to try running without CMI if it is what you are using to solve the problem. The only other solution i could give you is to turn off the spell inventory, and use chest mode instead.

Gyztor commented 2 years ago

alright, i'll give that a try, i do have a recommendation though, i would make it so when a person logs out or the server shuts down that before the server shuts down and the world is saved have the spell inventories be closed on leave/shutdown (or at least have an option for it) as it would make it save the correct inventory (because even with cmi disabled this can still happen where the spell inventory saves on shutdown still because that inventory isnt switched back to the base one)

NathanWolf commented 2 years ago

That is exactly how it works. It also has a backup of your inventory in the player data at all times, in case the server crashes hard without being able to restore the inventory, it will be restored on their next login.

This is why I believe your issues is a plugin conflict, and CMI has been the culprit in the past.

Any plugins that mess with player inventories are not going to work well with Magic at all, unless you switch all wands to chest or cycle-only mode.

Gyztor commented 2 years ago

going to try and disable cmi all together and see if this persists as in testing when disabing with PluginManager it still happened at restarts (which is honestly the only real place this happens) I should also mention cmi disables when stopping a server before the magic plugin

Gyztor commented 2 years ago

i have only had it happen once with a leave and come back and that was when someone crashed

Gyztor commented 2 years ago

okay just recreated it without cmi

NathanWolf commented 2 years ago

Sorry, I'm out of ideas then :(

You can switch all wands to use chest mode by default with this command:

/mconfig config wand_slots.spellmode.default_slotted spellmode_chest
Gyztor commented 2 years ago

alright, i think for now i will just deal with it and just make sure to make a notice on my server to tell people to make sure to be out of their spell inventories when leaving the server, though i am really enjoying this plugin and wanted to apologize if i came off snippy with my earlier comments as i have been tired and a bit grumpy from not falling asleep till the ripe time of 4am.

NathanWolf commented 2 years ago

It's totally ok- this is a very frustrating problem for me in general, it's not your fault.

Gyztor commented 2 years ago

Yeah, i can completely understand as i have tried developing virtual chest plugins and other similar things its not easy at all, but i do have an idea, maybe it could be possible to store the spell inventory in the player data area and just switch the inventories to the 2nd stored area that saves in the playerData rather than the plugins own database (or memory) to ensure they both always exist and can be swapped easily

NathanWolf commented 2 years ago

That is, again, basically how it all works ... the player inventory is always backed up while the spell inventory is active, when it's not active the spell inventory just exists in-memory.

I make every possible attempt to ensure there's no way a player's inventory can be lost. I am unaware of any possible way for it to happen at this point other than another plugin interfering.

The only possibility to make this safer is to fake the entire spell inventory with packets, but I'm honestly not sure that's possible without ProtocolLib or custom server software since it would likely require intercepting packets from the player to avoid confusing the server (e.g. they're dragging around items they don't really have)

Gyztor commented 2 years ago

hm, well i did it on a clean server with basically no plugins other than api and this still persisted which is why its odd. the only thing i can think on else it would be is runecraft but runecraft doesnt really mess with the inventory

NathanWolf commented 2 years ago

What is runecraft if not a plugin?

Definitely don't use plugin managers or /reload by the way- if you are hot-reloading the server things will break.

If you can make it happen on a clean server can you provide steps?

If you have a reproducible case that is almost always fixable. It's just these random "I don't know what caused it" problems that are basically impossible to track down.

Gyztor commented 2 years ago

Yeah, i don't really use the plugin manager except just monitoring and command digging for personal documentation. and i dont use /reload that command scares me lol i dont give anything permissions to that but yeah let me write out the reproduction steps that happen.

Gyztor commented 2 years ago
  1. Install the magic plugin and full restart server
  2. give yourself admin wand for testing /magic give admin
  3. open the spell inventory with right click or dropping (depending on settings)
  4. Keep open the spell inventory and restart the server /restart or /stop or restart with server manager
  5. Main inventory overwritten with spell inventory
Gyztor commented 2 years ago

i would also make sure the setting for turning reopen_wand_on_join= false so you dont need to close the spell inventory after and its the same as what i had

NathanWolf commented 2 years ago

Thanks .. I thought there would be more to it than that :|

Can you please paste what /version and /version magic tell you?

Gyztor commented 2 years ago

/version is being broken but the version my pluginmanager says is 10.7-13f19d7 on server version 1.18.2 Paper (299)

NathanWolf commented 2 years ago

To be clear I have a ton of users, players and myself that often log out (or are online when the server shuts down) with spell inventories open and never have this issue ... and just to be 100% sure I tried it just now myself on a local server.

So something else must be going on, but I have no idea what - unless you are on some strange server flavor and/or your plugin manager is getting in the way.

I don't consider having a plugin manager (and again what is runecraft?) as a completely clean test, for what that's worth 😂

If you can really make this happen on Spigot or Paper with zero plugins or other mods installed then... I'm at a complete loss.

Gyztor commented 2 years ago

Runecraft for reference: https://www.spigotmc.org/resources/runecraft.39771/ it am using the official paper fork.

Gyztor commented 2 years ago

is it possible its luckperms somehow?

Gyztor commented 2 years ago

not seeing anything in verbose with luckperms

NathanWolf commented 2 years ago

Don't think it could be LuckPerms .. no idea about Runecraft and looks like it's closed-source so hard to say without testing it.

Gyztor commented 2 years ago

yeah, i can keep trying to trail an error until i find something

Gyztor commented 2 years ago

image got it down to these plugins and its still happening (ignore void gen its meant for multiverse generation)

NathanWolf commented 2 years ago

Those are all pretty standard so I'm really at a loss sorry :(

Gyztor commented 2 years ago

yeah, my last guess is it possible it being connected to a mysql database is the issue?

NathanWolf commented 2 years ago

One thing you could try: /mconfig config log_verbosity 20

This will turn up magic's logging, so it will log anytime it is saving player data.

NathanWolf commented 2 years ago

Whenever you see the messages about a player having logged out and their data getting saved, this is when magic restores the inventory.

So either

  1. You don't see those messages, and so for some reason the plugin is not doing what it's supposed to
  2. Something else comes in and tries to save/restore the player inventories.. if you are using mysql then presumably you'd have a plugin managing player inventories, and if that were the case that would almost certainly be the problem.
Gyztor commented 2 years ago

yeah let me do that and test some restarts and stuff

Gyztor commented 2 years ago

i should have prefenced i have magic using mysql

NathanWolf commented 2 years ago

Oh, i see- well that is a good lead, that is at least something different from my testing.

Let me know what you see with those debug messages- or if you are seeing errors when players log out or the server shuts down, that could definitely be a good clue.

Gyztor commented 2 years ago

on shutdown:

[18:32:48 INFO]: [Magic] Finished saving
[18:32:48 INFO]: [Magic] Player quit: 951816b1-7d5e-484d-865f-495b25544d56
[18:32:48 INFO]: [Magic] Finalizing quit of player 951816b1-7d5e-484d-865f-495b25544d56 using external data? false, loaded? true, loading? false, shutting down? true
[18:32:48 INFO]: [Magic] Unregistered player 951816b1-7d5e-484d-865f-495b25544d56
[18:32:48 INFO]: [Magic] Saving player data for GyztorMizirath (951816b1-7d5e-484d-865f-495b25544d56) synchronously at 1650580368996
[18:32:49 INFO]: [Magic] Finished saving data for 951816b1-7d5e-484d-865f-495b25544d56 and released lock 

on startup:

[18:33:19 INFO]: [Magic] Checking lock for player 951816b1-7d5e-484d-865f-495b25544d56 at 1650580399127
[18:33:19 INFO]: [Magic] Cached preloaded mage data cache for id 951816b1-7d5e-484d-865f-495b25544d56
[18:33:19 INFO]: [Magic]  Finished Loading mage data for 951816b1-7d5e-484d-865f-495b25544d56 at 1650580399137
[18:33:19 INFO]: UUID of player GyztorMizirath is 951816b1-7d5e-484d-865f-495b25544d56
[18:33:19 INFO]: [Magic] Registered player 951816b1-7d5e-484d-865f-495b25544d56
[18:33:19 INFO]: GyztorMizirath joined the game
[18:33:19 INFO]: GyztorMizirath[/10.0.0.2:59198] logged in with entity id 42 at ([world]289.76041236372663, 108.16950678268455, -4.3536606624663685)
[18:33:19 INFO]: [Magic] Loading mage data for GyztorMizirath (951816b1-7d5e-484d-865f-495b25544d56) at 1650580399709
[18:33:19 INFO]: [Magic] Obtaining lock for player 951816b1-7d5e-484d-865f-495b25544d56 at 1650580399709
[18:33:19 INFO]: [Magic] Obtained lock for player 951816b1-7d5e-484d-865f-495b25544d56 at 1650580399713 in 4ms
[18:33:19 INFO]: [Magic] Loaded preloaded mage data from cache for id 951816b1-7d5e-484d-865f-495b25544d56 and obtained lock

not really seeing any errors other than its not showing anything along the lines of closing the inventory

Gyztor commented 2 years ago

formatting errors sorry for the edits

Gyztor commented 2 years ago

oh? did it magically start working let me do some consecutive restarts real quick

Gyztor commented 2 years ago

nope it broke again

NathanWolf commented 2 years ago

Kind of sad there are no errors there ... but yeah this:

Finalizing quit of player

Is printed just after the plugin has "deactivated" the player, which includes closing the spell inventory and restoring their survival inventory. So i think if it gets there it should be ok, as long as some other plugin/thing hasn't tried to save the player's inventory before then.

If you are using mysql for Magic data storage then I assume you are also storing player inventories in mysql, right?

Gyztor commented 2 years ago

no i usually have it store within the usual player data location on the disk ( as on my testing server the only thing able to modify inventories or save them anywhere is Magic at this current moment)

Gyztor commented 2 years ago

i dont even have cmi copying inventories every hour as a backup procedure to easily restore inventories if something goes wrong ( as it is disabled )

NathanWolf commented 2 years ago

Mysql is kinda overkill unless you really need it, normally only if you have a multi-server setup and want player data to sync across servers.

I'd be a little bit of a message to switch it back at this point, definitely doable but a little complex just for testing.

I really doubt that's the issue anyway, though, since the magic data is not really part of the problem here.

Gyztor commented 2 years ago

yeah, and from the issues my players have had it hasnt mattered if it was mysql or not ( as i tried to switch it to mysql to see if it helped)

Gyztor commented 2 years ago

any updates or anything else found out with this by chance?

NathanWolf commented 2 years ago

No, sorry- I really don’t have anything I can look at here. It’s only happening for you .. I guess the mysql storage is an uncommon config, but I use pgsql on my dev server which should be more or less equivalent.

At any rate, if I can’t reproduce a problem it’s almost impossible to fix.

What is ViaBackwards? I’m familiar with ViaVersion, but not that one. That’s the only thing from your plugin list I don’t really recognize.

Gyztor commented 2 years ago

via backwards allows previous version to join but no one has really been playing on earlier versions earlier than 1.17 tbh

Gyztor commented 2 years ago

but i have replicated this on a fully blank server

Gyztor commented 2 years ago

i can try with a fully blank config too real quick though

Gyztor commented 2 years ago

oh? found something odd. it seems its mysql. (oddly its working again on the truely clean server and plugin) so the only thing it could be is its not connecting to mysql fast enough or not restoring from it fast enough?

Gyztor commented 2 years ago

or maybe its restoring before it opens the spell inventory?