Fupery / ArtMap

Bukkit Plugin - Draw directly onto Minecraft maps in-game.
18 stars 26 forks source link

Potential Off-Hand Duplication fix. #67

Open LogGits opened 6 years ago

LogGits commented 6 years ago

This is a potential (I am saying potential because I cant compile the plugin to test it) fix for the off-hand duplication method present in this plugin. Please test before considering merging.

LogGits commented 6 years ago

@LogGits Also this does not compile against 1.12.2 spigot. You are missing imports as it can't find symbols on compile.

Yes I am aware of this most of the maven repositories he is using aren't working and therefore even I was unable to compile it. There are workarounds but im not too sure.

@LogGits Since the primary dev happens to be inactive, could you perhaps look into fixing this issue with deprecated api use for recipe construction?

I will look into it. I am very busy atm but we will see.

getItemInHand is also deprecated, should use explicit methods in PlayerInventory.

Will update :)

What exactly is the off-hand duplication issue? When I test getting on an easel with an item in my offhand and get off the easel, the item isn't kept in my off-hand but is instead moved to main inventory or dropped if inventory is full. There is no duplication, nor can I grab any items from the artkit via off-hand slot out of the easel.

Hold an easel in your offhand and right click on the ground. It isn't taking the easel out because its looking for the item in their main hand (he didn't take into account that the easel is in the second/off hand).

mibby commented 6 years ago

Yes I am aware of this most of the maven repositories he is using aren't working and therefore even I was unable to compile it.

I'm able to compile the main upstream source by removing many of the optional dependencies from the pom and compatibility modules, such as Residence. As well as compile it against 1.12.2 spigot/bukkit fine after doing so. Doing the same with your commit, it isn't able to find the methods defined so you seem to be missing an important bukkit import. I tried adding import org.bukkit.inventory.PlayerInventory; but that wasn't it. Also tried import org.bukkit.inventory.* but that only resolved 1 of the 4 missing symbols.

Hold an easel in your offhand and right click on the ground. It isn't taking the easel out because its looking for the item in their main hand (he didn't take into account that the easel is in the second/off hand).

Oh! That's what you meant by off-hand duplication! Yep, can confirm that is a problem. Only seems to happen when your main hand is empty (since you can't place items set in off-hand if you have an item in your main hand as well).

However I was also able to experience another issue upon trying to test off-hand issues. If every slot in your inventory is full and you have 1 item in your off-hand, upon sitting on an easel, the off-hand item is brought into the artkit inventory when sitting.

This works with any item in your off-hand. I can't seem to reproduce any issues with it duplicating the item, only losing the item. As well as this only happens when your entire inventory is full.

I will look into it. I am very busy atm but we will see.

Would be much appreciated!

mibby commented 6 years ago

Regarding the deprecated recipe API use;

This is a pretty trivial fix for all plugins, and just requires passing a NamespacedKey to the recipe they're creating https://hub.spigotmc.org/javadocs/spigot/org/bukkit/NamespacedKey.html

LogGits commented 6 years ago

@mibby Do you have a discord account or spigot account that I can message you about this?

mibby commented 6 years ago

@LogGits You can reach me on spigot via @mibby, if not on github.

LogGits commented 6 years ago

Yeah I will work on a full fix/update soon when I have the time. Got a bunch of assignments and exams to study for and complete first. After that I'll fix it. Its a relatively simple fix theoretically. Just need to know how to properly compile it, to test.

LogGits commented 6 years ago

For reference, this is how the Magic plugin solved the recipe issue. Hopefully it is quite trivial to fix and import a similar implementation. :(

I know how to fix the issue as it is quite easy. I am just having troubles compiling.

mibby commented 6 years ago

I know how to fix the issue as it is quite easy. I am just having troubles compiling.

What issues are you having compiling? I'm able to compile the main upstream branch fine. If you can push the commit to fix the issue, I can check and see if I can compile your fork with some of the changes noted here https://github.com/LogGits/ArtMap/commit/8193e77b779ddebd5d6875efd632ce5d12a55460#commitcomment-26954834.