search

NewbieOrange / clicksort

Minecraft Bukkit plugin to sort inventories with a simple mouse click
https://dev.bukkit.org/projects/clicksort
Other
13 stars 5 forks source link

Sorting Mule's inventory forces items in saddle slot and deletes items. #17

Open meridianrealms opened 3 months ago

meridianrealms commented 3 months ago

This may also affect other animals with inventories, I haven't tested. When sorting a mule's inventory it will put any item in the saddle slot and allow a player to control it as if a saddle were there. More importantly, sometimes it will erase items completely. There is no way currently I know of to disable sorting since it seems it's detected as a chest inventory type.

NewbieOrange commented 2 months ago

This is because mule and chested horse have their inventory of type CHEST, but with a size of 17 (first item being the saddle, second is null? remaining 15 is the actual inventory. Working on a fix.

meridianrealms commented 2 months ago

So the null slot would explain the items disappearing then. That's good to know. Thanks for looking in to this!

NewbieOrange commented 2 months ago

clicksort-snapshot.zip

@meridianrealms hi, please check if this version fixed the issue for you. You need to unzip it first (Github requires uploading zips)

NewbieOrange commented 2 months ago

@meridianrealms can you check if it is working? so I can make a new release

minoneer commented 3 weeks ago

@NewbieOrange I just checked with a local build including #22. For mules, the first slot of the inventory is skipped when sorting (top left). I suppose this is due to mules not having an armor slot, unlike regular horses.

NewbieOrange commented 3 weeks ago

@NewbieOrange I just checked with a local build including #22. For mules, the first slot of the inventory is skipped when sorting (top left). I suppose this is due to mules not having an armor slot, unlike regular horses.

Thanks for the quick fix! The whole horse-inventory thing is a bit confusing as Bukkit API doesn't seem to have anyway to know where is the saddle / armor slot (or if whether there is such a slot).

Spigot Wiki showed an image of donkey and horse inventory, which looks like both are 0=saddle, 1=armor (donkey have no armor, but the "slot" is still there). I am not sure why the first actual slot is skipped for Donkeys / Mules..

https://www.spigotmc.org/wiki/raw-slot-ids/

minoneer commented 3 weeks ago

@NewbieOrange I just checked with a local build including #22. For mules, the first slot of the inventory is skipped when sorting (top left). I suppose this is due to mules not having an armor slot, unlike regular horses.

Thanks for the quick fix! The whole horse-inventory thing is a bit confusing as Bukkit API doesn't seem to have anyway to know where is the saddle / armor slot (or if whether there is such a slot).

Spigot Wiki showed an image of donkey and horse inventory, which looks like both are 0=saddle, 1=armor (donkey have no armor, but the "slot" is still there). I am not sure why the first actual slot is skipped for Donkeys / Mules..

https://www.spigotmc.org/wiki/raw-slot-ids/

I agree, this seems strange. I double checked and verified a few things:

This seems like an API issue to me. I dug a bit in to the source and found this commit: https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/commits/c2ccc46ec3601b1805fab9d753d16cb5c5964ba3#src%2Fmain%2Fjava%2Forg%2Fbukkit%2Fcraftbukkit%2Fentity%2FCraftHorse.java It seems that body armor is internally stored as a separate thing now. Not sure about the saddle. I'm yet to completely understand if this is related, but it could explain the offset by 1.

Edit: Horse inventories (without chests) have a similar issue:

NewbieOrange commented 3 weeks ago

@minoneer The commit you refer to is committed at May 10. Not sure if it is something mojang changed during a version and spigot has to fix (the Bukkit API part).

If it is the case, should we just change the begin slot to slot 1 for (chested) horses, would that work? or would the last slot (rawSlot 16, slot 9 of the player inventory) be excluded from sorting? (in which case, perhaps you can submit a bug report to spigot)

minoneer commented 2 weeks ago

@minoneer The commit you refer to is committed at May 10. Not sure if it is something mojang changed during a version and spigot has to fix (the Bukkit API part).

If it is the case, should we just change the begin slot to slot 1 for (chested) horses, would that work? or would the last slot (rawSlot 16, slot 9 of the player inventory) be excluded from sorting? (in which case, perhaps you can submit a bug report to spigot)

Yes, I believe Mojang changed how armor is stored internally.

Starting to sort from slot 1 should fix the actual sorting (slot 0 is saddle).

It might just sort the wrong inventory when clicking on the last slot - but I think we can live with that until Spigot fixes its mapping. We should create a bug report for that.

That's the theory at least - can't test right now.

Edit: it's pbl. a 1.20.5/6 change if you want to preserve backwards compatibility

minoneer commented 5 days ago

@NewbieOrange It might be good to release an update, even if this mule inventory situation is unresolved (we could also temporarily disable any horse-related inventories until this has been fixed). People are already forking the plugin on the spigot forums due to a lack of update.

NewbieOrange commented 4 days ago

@NewbieOrange It might be good to release an update, even if this mule inventory situation is unresolved (we could also temporarily disable any horse-related inventories until this has been fixed). People are already forking the plugin on the spigot forums due to a lack of update.

Yeah, I will make a release today.