Refactor Fire Crafting Handler #749

New issue

Open

opened 2024-07-18 21:28:56 +00:00 by Rover656 · 0 comments

Rover656 commented

2024-07-18 21:28:56 +00:00

(Migrated from github.com)

Copying from HenryLoenwind's comment on #740:

I just had a look at the handler out of curiosity, and it looks a bit wasteful. (Also very much as I remember it, so this probably is my fault.)

I'd suggest a couple of changes:

Put 65f3b09b23/enderio-base/src/main/java/com/enderio/base/common/handler/FireCraftingHandler.java (L70-L88) into a getMatchingRecipe() method
Put if (getMatchingRecipe() != null) { ...} return; inside if (isFire) {
Surround spawnInfinityDrops(...) with a if (getMatchingRecipe() instanceof FireCraftingRecipe matchingRecipe) { (or if ((FireCraftingRecipe matchingRecipe = getMatchingRecipe()) != null) {)
Move the // Grab useful fields. stuff inside the two if blocks just above where it's needed. Sadly this means duplicating it, but this is not an event to be picky---NeighborNotifyEvents are fired very often.
I think the if (FIRE_TRACKER.isEmpty() && !isFire) { now has no benefit anymore.

And then ignore the complicated description above and look at what I cobbled up in a text editor: https://gist.github.com/HenryLoenwind/e276c7f437120011f1c84175496c531d

I think this would greatly reduce the time spent in this event handler.

Copying from HenryLoenwind's comment on #740: I just had a look at the handler out of curiosity, and it looks a bit wasteful. (Also very much as I remember it, so this probably is my fault.) I'd suggest a couple of changes: - Put https://github.com/Team-EnderIO/EnderIO/blob/65f3b09b23cb805049cf24b5016936ee06764319/enderio-base/src/main/java/com/enderio/base/common/handler/FireCraftingHandler.java#L70-L88 into a getMatchingRecipe() method - Put `if (getMatchingRecipe() != null) { ...} return;` inside `if (isFire) {` - Surround `spawnInfinityDrops(...)` with a `if (getMatchingRecipe() instanceof FireCraftingRecipe matchingRecipe) {` (or `if ((FireCraftingRecipe matchingRecipe = getMatchingRecipe()) != null) {`) - Move the `// Grab useful fields.` stuff inside the two if blocks just above where it's needed. Sadly this means duplicating it, but this is not an event to be picky---NeighborNotifyEvents are fired very often. - I think the `if (FIRE_TRACKER.isEmpty() && !isFire) {` now has no benefit anymore. And then ignore the complicated description above and look at what I cobbled up in a text editor: https://gist.github.com/HenryLoenwind/e276c7f437120011f1c84175496c531d I think this would greatly reduce the time spent in this event handler.