DutchJelly / CraftEnhance

A Minecraft Plugin that allows server owners to create custom recipes in-game
Apache License 2.0
13 stars 10 forks source link

i found a thing to improve #30

Closed Johvu closed 2 years ago

Johvu commented 3 years ago

So I found a thing to improve in the code at CraftEnhance there is getserver.resetRecipes(); that makes some plugins that interfere crafting to duplicate items

LoneDev6 commented 3 years ago

Basically some plugins register the recipes via the Bukkit api but also save them in their own data structures to handle them later. Removing them using the bukkit api via Bukkit.resetRecipes() breaks these plugins.

An easy fix to avoid breaking other plugins is that one, basically you just have to check if the recipe has CraftEnhance namespace to avoid removing other plugins recipes (code not tested but should work if the namespace is correct) .

Iterator<Recipe> it = Bukkit.getServer().recipeIterator();
while (it.hasNext()) {
  Recipe r = it.next();
  // do your comparisions with the recipe
  if(recipe instanceof Keyed)
  {
    if(((Keyed) recipe).getKey().getNamespace().equals("craftenhance"))
    {
      it.remove();
    }
  }
}

Here is the code that should be changed, I think it's the only problematic one https://github.com/DutchJelly/CraftEnhance/blob/8b1a8050631f1efb278dfc0304ab638bcf121518/src/main/java/com/dutchjelly/craftenhance/crafthandling/RecipeLoader.java#L116

Also refer to this warning https://github.com/DutchJelly/CraftEnhance/blob/8b1a8050631f1efb278dfc0304ab638bcf121518/src/main/java/com/dutchjelly/craftenhance/CraftEnhance.java#L107

DutchJelly commented 3 years ago

Thank you for your suggestion. I had the impression that the iterator was immutable, that's why I was using Bukkit.resetRecipes(), but I'm sure you're right in that it's not.

LoneDev6 commented 3 years ago

Should be modifiable, I'm quite sure since I think I use a similar method in my plugins, to avoid breaking others :D

Let me know if you need more help

DutchJelly commented 2 years ago

I've added this. Thanks for the suggestion.