Magic

Magic

190k Downloads

Inventory Overwriting Glitch

Gyztor opened this issue · 56 comments

commented

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.

commented

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

commented

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.

commented

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)

commented

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.

commented

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

commented

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

commented

okay just recreated it without cmi

commented

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
commented

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.

commented

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

commented

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

commented

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)

commented

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

commented

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.

commented

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.

commented

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

commented
  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
commented

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

commented

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

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

commented

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.

commented

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

commented

is it possible its luckperms somehow?

commented

not seeing anything in verbose with luckperms

commented

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.

commented

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

commented

yeah let me do that and test some restarts and stuff

commented

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.
commented

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

commented

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

commented

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.

commented

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

commented

i should have prefenced i have magic using mysql

commented

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.

commented

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

commented

formatting errors sorry for the edits

commented

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

commented

nope it broke again

commented

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?

commented

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)

commented

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 )

commented

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.

commented

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)

commented

This is probably a conflict with CMI.

commented

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

commented

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.

commented

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

commented

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

commented

but i have replicated this on a fully blank server

commented

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?

commented

or maybe its restoring before it opens the spell inventory?

commented

probrably that last hypthosis from the seen behaviour of with this new config

commented

hmm, nope i turned it back off to just the local files and its having issues still

commented

found the issue its logout_delay: 1

commented

setting it to 0 fixes the issue
i feel very dumb

commented

Well I'm glad you figured it out anyway... I'm really not sure what that option is actually for, but I could definitely see how that would cause issues on server shutdown. Delaying by 1 tick means it wouldn't actually get a chance to close the wand inventory or save player data.

The changlog says - Add option to delay logout, for compatibility with plugins that kill players on logout

... so I guess someone asked for that, but it's likely we never considered the implications on shutdown. Oh well.

commented

honestly i thought it added a buffer to allow a save to be more stable in case of something like a player crash