search

mfnalex / InvUnload

Spigot plugin that automatically puts your stuff into the right chests
GNU General Public License v3.0
6 stars 8 forks source link

Add a new message that states there is nothing to unload because all items in the inventory are blacklisted #42

Closed hippotastic closed 3 years ago

hippotastic commented 3 years ago

If I run /blacklist add inventory and then /unload, the message I get is the following:

Nothing to unload: There are no chests for the remaining items.

In my opinion, this message does not fit my scenario. I do have items in my inventory. The actual reason why there is nothing to unload is that everything in my inventory is blacklisted.

Therefore, I suggest adding a new message for this scenario:

Nothing to unload: All items in your inventory are blacklisted. Type /blacklist to see it.

mfnalex commented 3 years ago

that will lead to problems. Imagine you have 10 dirt you wanna unload, but you have no chests, and you have 10 obsidian, but those are blacklisted. what message should be displayed now? maybe you should just change the message to

Nothing to unload: There are no chests for the remaining items, or all remaining items are blacklisted.

hippotastic commented 3 years ago

In your scenario, I would simply display the existing "Nothing to unload: There are no chests for the remaining items." message. My suggested new message should only be displayed if all items in my inventory are blacklisted and there is nothing to do.

mfnalex commented 3 years ago

Oh okay, yeah that's a good idea.

hippotastic commented 3 years ago

The reason why I'm suggesting this new message is that I want to remove any ambiguity for the player: It should be simple for them to see if they have something left to unload or not.

If everything is blacklisted, they would get this new message, and can continue farming. 😄

mfnalex commented 3 years ago

Yeah, that's understandable, I'll just finish my current stuff then I'll ad this :)

mfnalex commented 3 years ago

Please try this version: https://repo.jeff-media.de/maven2/de/jeff_media/InvUnload/4.10.0-SNAPSHOT/InvUnload-4.10.0-20210301.175756-2.jar

mfnalex commented 3 years ago

I currently don't have time to test it myself but I am like 80% sure it should work. If it does, please let me know so I can release it

hippotastic commented 3 years ago

Ok, I will test it! Just a note: I noticed that MSG_INVENTORY_EMPTY is not being used anywhere in the code. Did you maybe miss adding a check that triggers this message in the code?

hippotastic commented 3 years ago

Unfortunately, this version doesn't work. When I use /unload it creates an exception:

[20:35:09 ERROR]: null
org.bukkit.command.CommandException: Unhandled exception executing command 'unload' in plugin InvUnload v4.10.0-SNAPSHOT
    at org.bukkit.command.PluginCommand.execute(PluginCommand.java:47) ~[patched_1.16.5.jar:git-Paper-505]
    at org.bukkit.command.SimpleCommandMap.dispatch(SimpleCommandMap.java:159) ~[patched_1.16.5.jar:git-Paper-505]
    at org.bukkit.craftbukkit.v1_16_R3.CraftServer.dispatchCommand(CraftServer.java:810) ~[patched_1.16.5.jar:git-Paper-505]
    at net.minecraft.server.v1_16_R3.PlayerConnection.handleCommand(PlayerConnection.java:2028) ~[patched_1.16.5.jar:git-Paper-505]
    at net.minecraft.server.v1_16_R3.PlayerConnection.c(PlayerConnection.java:1838) ~[patched_1.16.5.jar:git-Paper-505]
    at net.minecraft.server.v1_16_R3.PlayerConnection.a(PlayerConnection.java:1791) ~[patched_1.16.5.jar:git-Paper-505]
    at net.minecraft.server.v1_16_R3.PacketPlayInChat.a(PacketPlayInChat.java:47) ~[patched_1.16.5.jar:git-Paper-505]
    at net.minecraft.server.v1_16_R3.PacketPlayInChat.a(PacketPlayInChat.java:5) ~[patched_1.16.5.jar:git-Paper-505]
    at net.minecraft.server.v1_16_R3.PlayerConnectionUtils.lambda$ensureMainThread$1(PlayerConnectionUtils.java:23) ~[patched_1.16.5.jar:git-Paper-505]
    at net.minecraft.server.v1_16_R3.PlayerConnectionUtils$$Lambda$4123/1334598935.run(Unknown Source) ~[?:?]
    at net.minecraft.server.v1_16_R3.TickTask.run(SourceFile:18) ~[patched_1.16.5.jar:git-Paper-505]
    at net.minecraft.server.v1_16_R3.IAsyncTaskHandler.executeTask(IAsyncTaskHandler.java:136) ~[patched_1.16.5.jar:git-Paper-505]
    at net.minecraft.server.v1_16_R3.IAsyncTaskHandlerReentrant.executeTask(SourceFile:23) ~[patched_1.16.5.jar:git-Paper-505]
    at net.minecraft.server.v1_16_R3.IAsyncTaskHandler.executeNext(IAsyncTaskHandler.java:109) ~[patched_1.16.5.jar:git-Paper-505]
    at net.minecraft.server.v1_16_R3.MinecraftServer.bb(MinecraftServer.java:1138) ~[patched_1.16.5.jar:git-Paper-505]
    at net.minecraft.server.v1_16_R3.MinecraftServer.executeNext(MinecraftServer.java:1131) ~[patched_1.16.5.jar:git-Paper-505]
    at net.minecraft.server.v1_16_R3.IAsyncTaskHandler.awaitTasks(IAsyncTaskHandler.java:119) ~[patched_1.16.5.jar:git-Paper-505]
    at net.minecraft.server.v1_16_R3.MinecraftServer.sleepForTick(MinecraftServer.java:1092) ~[patched_1.16.5.jar:git-Paper-505]
    at net.minecraft.server.v1_16_R3.MinecraftServer.w(MinecraftServer.java:1006) ~[patched_1.16.5.jar:git-Paper-505]
    at net.minecraft.server.v1_16_R3.MinecraftServer.lambda$a$0(MinecraftServer.java:175) ~[patched_1.16.5.jar:git-Paper-505]
    at net.minecraft.server.v1_16_R3.MinecraftServer$$Lambda$3069/100240177.run(Unknown Source) ~[?:?]
    at java.lang.Thread.run(Thread.java:745) [?:1.8.0_51]
Caused by: java.lang.NullPointerException
    at de.jeff_media.InvUnload.CommandUnload.onCommand(CommandUnload.java:141) ~[?:?]
    at org.bukkit.command.PluginCommand.execute(PluginCommand.java:45) ~[patched_1.16.5.jar:git-Paper-505]
    ... 21 more
hippotastic commented 3 years ago

I don't think that it is the cause of the exception, but I noticed that your new loop here is different from the existing one that iterates through the player's inventory. Your new code uses this loop:

for(int i = startSlot; i < endSlot; i++) {

While in InvUtils.java, your iteration code is different (<= instead of <):

for(int i = startSlot; i<=endSlot; i++) {
hippotastic commented 3 years ago

Most likely cause of the exception: In InvUtils.java, you perform a check for if(item==null) continue; and this check is missing in your new code.

Probably it would make sense to refactor iterating through the player's inventory to a common routine to avoid the differences I found.

mfnalex commented 3 years ago

oh shit. I'm sorry, as I said, I didnt have time to test anything, I was doing this from my phone. I will have a look at this again tomorrow again, sorry for the trouble

mfnalex commented 3 years ago

Most likely cause of the exception: In InvUtils.java, you perform a check for if(item==null) continue; and this check is missing in your new code.

Probably it would make sense to refactor iterating through the player's inventory to a common routine to avoid the differences I found.

yeah the whole code is a total mess. I learnt a lot of stuff in the last years, my free plugins are a really messy, sorry about that lol.

mfnalex commented 3 years ago

I will fix this tomorrow or wednesday!

EDIT: If I don't, please nag me everyday in this issue!! (I really mean this)

hippotastic commented 3 years ago

No worries about the code, I really appreciate your work on these plugins! It's incredible that you provide such a great level of support for them. ❤

Ok, I will try to nag politely in case you forget about the issue. :)

mfnalex commented 3 years ago

Added in 4.10.0 :)