2 faults in ItemUtil.java (CraftBook)

[LadyCailinBot]

CRAFTBOOK-3007 - Reported by AnorZaken

Was looking at the source and spotted 2 faults in
file: ItemUtil.java
method: public static boolean areItemsIdentical(ItemStack item, ItemStack item2)
faulty code begins at:
https://github.com/sk89q/craftbook/blob/a534703d6967ad5430e6cc114002b53364f826e2/src/main/java/com/sk89q/craftbook/util/ItemUtil.java#L323

Fault/error/scenario info can be found here:
http://forum.sk89q.com/threads/custom-recipes-wont-overwrite-vanilla-recipes.15374/#post-27308
(sorry, I posted there first)

Also included code that I believe will fix the two faults. (in the forum post)
(Not pull-ready, it's "similar" code from my own work related to craftbook - read the forum thread for more info.)

[LadyCailinBot]

Comment by AnorZaken

Also very very small (debug only) fault at:
https://github.com/sk89q/craftbook/blob/a534703d6967ad5430e6cc114002b53364f826e2/src/main/java/com/sk89q/craftbook/util/ItemUtil.java#L154
(Says "same size" but only the height, not the width of the recipes has been compared.)

[LadyCailinBot]

Comment by me4502

I'll look into fixing that bug when I get a chance, but I'm not sure if that is the underlying cause of the recipe issues. That code was rewritten a few weeks ago to make it simpler, this bug probably wasn't in there at that stage.

[LadyCailinBot]

Comment by AnorZaken

No the bug isn't directly related to the discussion in the thread. It's just something I happened to stumbled upon.
I have however almost completed a recipe helper class that probably will fix recipe issues!
Most notably it contains:
/**

• Adds a bunch of {@link Recipe}s to a {@link Server}, checking each for collision against an existing {@link Recipe}.
• @param server
• @param allowOverwriteExisting if this is true collisions will be resolved by removal of the existing recipe
• @param recipes
• @return a {@link List}<{@link Recipe}> - if {@code allowOverwriteExisting} is true this will
• contain all the old recipes that where removed, if {@code allowOverwriteExisting} is false this will
• return all the new recipes that wasn't added to the {@link Server} due to collisions. In other words it returns all
• the "casualties" of collisions.
  */
  public static List addRecipesToServer(final Server server, final boolean allowOverwriteExisting, final Recipe... recipes) {

It's purely bukkit-api, no craftbukkit or hacks (I haven't tested it yet though...)

EDIT:
So the helper class RecipeHelper is almost completed (and well documented), but I read a bit more about CraftBook, and I guess maybe you aren't adding recipes but only handling the Prepare...-events?
If so then maybe this is the method you are interested in instead?
/**

• Checks if trying to add a {@link Recipe} to a {@link Server} would silently fail due to collision with any existing
• {@link Recipe}s and optionally removes the existing {@link Recipe}s it collides with.

• Note1: Multiple collisions can happen only if the {@link Recipe} argument is a wildcard {@link Recipe}.

• Note2: The {@link Recipe} argument is not added to the server by this operation!

• Note3: Meta-data comparisons will only be performed if an {@link IMetaChecker} has been set!
• @param server
• @param removeIfFound if this is false this operation is read-only
• @param recipe
• @return null if {@link Recipe} argument is null, otherwise a {@link List}<{@link Recipe}>
• with the {@link Recipe}s on the {@link Server} that the {@link Recipe} argument collided with.
  */
  public static List serverHasSimilarIngredientRecipe(final Server server, final boolean removeIfFound, final Recipe recipe)

(...but I still haven't tested it xD)
(Regarding the IMetaChecker I supports setting an arbitrary class with reflection - it is specifically intended to be compatible with ItemUtil.java without requiring any change to ItemUtil.)

Sorry for this not being overly enjoyable to read in plain-text.

[LadyCailinBot]

Comment by me4502

CraftBook is adding recipes to bukkit, it uses the prepare events to handle metadata, bukkits recipe system only handled ID and data values.

[LadyCailinBot]

Comment by AnorZaken

Here it is, "presumably" completed (as in not tested yet).
https://github.com/AnorZaken/aztb/blob/master/src/nu/mine/obsidian/aztb/bukkit/recipes/v1_0/RecipeHelper.java
It's LGPL3 so it's compatible with the CraftBook-project (LGPL3 can be converted to GPL3)
If you want / need any other methods let me know and I'll probably add them (unless I'm too busy). :)

I'm doing this for purely selfish reasons:

1. I like to code ^^
2. I want craftbook to support overwriting / removing existing recipes as per the forum topic
   So don't hesitate to ask for additions or changes (I will just consider it helpful feedback :P)

[LadyCailinBot]

Comment by me4502

Thanks, I'll hopefully be able to look into this by the end of the week.