OpenInv

4M Downloads

Items getting duplicated

ProvidenceSh opened this issue · 19 comments

commented

MC Version 1.21.4.

Items are being duplicated when I open player's inventories. I give someone one jukebox and it duplicates the item into their upper inventory. It just fills more in that specific column when I do /oi

commented
commented

Says Config version 8.. whatever gets downloaded from Bukkit as "Latest File"
Java version 1.21.4-9298f59

...apparently this also scrambles people's inventories.
Repro steps:
/oi (user)
/oi (user)
existing items in their inv get duplicated

commented

Server version from /version, please. I need to know the actual implementation you're using, not just the Minecraft version. You can also check /version openinv while you're there.

commented

I should also note that scrambling sounds like outside plugin interference. Are you running an automatic sorting plugin, by any chance? If so, try removing it temporarily.

commented
commented

Appreciate it, thanks. I'll try to reproduce again, but I suspect a plugin incompatibility - while I've redone the build process a decent bit, the code for the internals hasn't changed much barring a few bugfixes.

commented

I cannot reproduce.
https://github.com/user-attachments/assets/2da8c505-fb99-4dc4-886f-c17536d06463

You'll have to narrow down the conflict yourself, unfortunately. Generally a good strategy is to remove half of your plugins. When you notice the problem not occurring, the conflict is within the removed half, so you can then add those back half at a time until you isolate the exact plugin.

/e:
Once you've isolated the plugin, if it's open source I can look into why it might be causing issues. If not, you'll have to contact its author.
The reason sorting plugins are usually the culprit is that the upper inventory is a Player inventory, but its slot ordering does not match what plugins that use hardcoded slot ranges expect. As a result, they often try to move items to the "hotbar" but instead copy them out of the hotbar slots into the regular inventory. OpenInv tries its best to make it clear to other plugins what its layout is via SlotTypes available from InventoryView#getSlotType.

commented

I can send you a video of this happening (or at least try to) if you'd like. Other than that, I'll try to see what's causing this.

commented

Like I said, since it's presumably a plugin conflict, it's up to you to isolate it. I believe you and would love to get any potential dupe issues resolved, I just can't really help further for now. I can make educated guesses based on what plugins are likely to do, but I'm not going to locate and download 20-ish plugins (which I also would then have to guess at your versions of) on the off chance that I'll be able to reproduce. Even if I did manage to replicate your exact plugin installation, this could be some particular configuration setting for one of those plugins.

commented

Figured it out. ProtocolLib 5.4.0-SNAPSHOT-741

commented

Gotcha, thank you. Are you testing this in creative (i.e. are any packet-based content errors being "made real" by the client)? Are there any plugins that depend on ProtocolLib, and if so, does it vanish when they are removed but ProtocolLib is not?

/e: Yeah, it's something depending on ProtocolLib, just ProtocolLib is fine. I did recently see a thread with a neat plugin for visually finding dependent plugins that might speed things up for you: https://www.spigotmc.org/threads/pluginvisualizer-interactive-dependency-mapping-for-spigot-servers.678386/

commented

I am in Creative and opening the Inventory of someone in Survival. It does the same thing regardless of my gamemode.

There's one plugin that depends on ProtocolLib but it doesn't interact with inventories in the slightest. Removing it keeps the same issue. I'll check the visualizer.

commented

Figured it out. It was JukeboxExtendedReborn (JEXT). Removed that and the issue went away.

commented

Okay, thank you. I'll poke around through their code (https://github.com/spartacus04/jext-reborn, for future me) and see if I can spot anything. A quick search isn't promising - their packet usage seems aimed specifically at correcting playing record information.
/e: Looks like they use https://github.com/NichtStudioCode/InvUI for managing UIs. I feel like I would have heard about problems if an entire UI framework causes issues, but if I can't find anything that looks off in JEXT-Reborn, I guess that's the next place to look.

commented

I'm not able to reproduce this. Will need more information.

  • Actual server /version
  • OI version
  • Reproduction steps (i.e. do you /give them the item, then /oi them? Do you /oi them, shift-click it? Etc.)

Sometimes problems like this are due to the creative client being authoritative and ignoring contents updates from the server, resulting in the client creating extra items. This happens a lot more on higher ping, but can generally be a problem. Try in survival as well.

commented

Hello there, JEXT-Reborn developer here, I came here from spartacus04/jext-reborn#544. I'm willing to contribute to fixing this issue.

I don't really know why this is happening since my code should only edits music discs and disc fragments.
I could try to check if an inventory was created by OpenInv before interacting with it. Is there a way to do that with your API?

commented

Heya! Sorry, I wasn't going to loop you in yet, I still don't have my ducks in a row. I still haven't made time to actually reproduce myself, and from there I planned to make a dummy plugin that uses the UI framework you're using to see if that also causes issues. Based on that I was going to do a little more code spelunking and see who needed to be contacted.

I don't see any likely problem areas in your code. Any edits you make are purely to an item, you don't interact with slots or slot IDs. This makes me think it has to be the UI framework's listeners somehow, as weird as that sounds.

OpenInv UIs can be checked for with InventoryAccess#getInventory(Inventory). Without a dependency on OpenInv, you can just check if the upper inventory is InventoryType.PLAYER, because the default inventory is InventoryType.CRAFTING.

OI is absolutely a bit of a hack - the Bukkit API doesn't really have a good way to translate what it's doing to plugins, so I've made a couple design choices that I'm not thrilled with but don't see a better alternative to.

OI pre-5.0.0 basically just lied about which slots were which and then translated that internally when an item was set. It was pretty incompatible with plugin edits in general because the outgoing slots didn't line up with the normal player inventory at all and contents calls included the fake slots padding out the inventory size to a multiple of 9. In particular, sorting plugins that blindly sorted upper inventories would just throw garbage everywhere and delete/dupe/drop items.

In 5.0.0, the new UI actually uses our own implementation of PlayerInventory for better plugin interoperability. For a best effort at matching plugin expectations (and possibly the source of the current problems) getContents/getStorageContents return the backing inventory's content directly in the backing inventory's format. That means that plugins that do batch operations like sorting inventories get regular player contents organized in the expected fashion... but it also means that getContents()[0] is not the same slot as getItem(0). This seemed like the best(TM) compromise between expectations and correctness because it supports the aforementioned terrible sorting plugins a lot better, and generally people don't mix and match.

commented

Found it - forgot that since JEXT-Reborn is using Kotlin getters are harder to search for.

https://github.com/spartacus04/jext-reborn/blob/dae54cbee71846b8fb46e57a501403ed1ff34d68/src/main/java/me/spartacus04/jext/listeners/DiscUpdateEvent.kt#L29-L35

I'll PR a fix. Since this isn't directly fixable from OI (at least, not without re-opening the can of worms that is sorting plugins) I guess I'll close this.

commented

Thanks for the help!