Electroblob's Wizardry

Electroblob's Wizardry

Mods
18M Downloads

lag when mass crafting

Mari023 opened this issue · 3 comments

commented

Minecraft version: 1.12.2
Wizardry version: 4.3.4
Environment: Server

Issue details:
lag when mass crafting anything (e.g. in a ae2 molecular assembler) due to wizardry looping over every recipe on a crafting event
probably gets worse the more recipes there are

Wizardry/src/main/java/electroblob/wizardry/misc/RecipeRechargeWithFlask.java

Lines 89 to 103 in 486d55f

public static void onItemCraftedEvent(PlayerEvent.ItemCraftedEvent event){
// getCraftingResult seems to only work for the result that's displayed, not once it is actually taken
// This means that although I no longer have to replace the result every tick, I still need to do it here
// ... I thought the whole point of the new recipe system was so that I DIDN'T have to do this?!
if(event.craftMatrix instanceof InventoryCrafting){
for(IRecipe recipe : CraftingManager.REGISTRY){
if(recipe instanceof RecipeRechargeWithFlask
&& recipe.matches((InventoryCrafting)event.craftMatrix, event.player.world)){
// Have to modify the itemstack in the actual event, it cannot be replaced
((RecipeRechargeWithFlask)recipe).rechargeItemAndCopyNBT(event.crafting, (InventoryCrafting)event.craftMatrix);
}
}
}
}
}

commented

As the comment in that code snippet points out, the only reason I have to receive the event here is due to a bug/oversight in Forge. This is made worse by the fact that there is no easy way of accessing the active recipe object from the event, which is why I have to check all the recipes and match the items against them.

It is possible that Forge has fixed the underlying problem since I wrote this bit of code, I will test whether this is the case. Otherwise, I'm not really sure what I can do about it without breaking the recharging functionality.

commented

I'm not up on the current approach to handling recipes, but looping through every single recipe every time something is crafted seems like a really bad idea. Even if you just maintained a list of all the instances of RecipeRechargeWithFlask, and just looped through those, that would cut out a huge part of that loop. An even more efficient approach would be to use a structure that could be indexed into based on the chargeable item, rather than looping through recipes looking for a match.

I'm inclined to think that storing distinct recipes for each combination of chargeable item and size of flask isn't a clean or efficient approach, and it would be better to have a single, smarter recipe to handle it, but again, my understanding of the recipe system is a bit out of date.

  • 👍1
commented

Even if you just maintained a list of all the instances of RecipeRechargeWithFlask, and just looped through those, that would cut out a huge part of that loop. An even more efficient approach would be to use a structure that could be indexed into based on the chargeable item, rather than looping through recipes looking for a match.

... wow I really should have thought of this 😅 Yeah that's obviously the way to do it, thank you.

I'm inclined to think that storing distinct recipes for each combination of chargeable item and size of flask isn't a clean or efficient approach, and it would be better to have a single, smarter recipe to handle it, but again, my understanding of the recipe system is a bit out of date.

I'm not entirely sure if this is possible or whether it's the 'correct' approach, I'll have to brush up on how the recipe system works.