Seperate furnace recipes by furnace type, dynamically determine isAFuel for furnaces #1344
Conversation
| * Precompute lists of all recipes available for furnace, blast_furnace and smoker/campfire | ||
| * | ||
| */ | ||
| public static void setupSmeltableItemLists() { |
There was a problem hiding this comment.
We used to do this, but it caused some very major issues. Servers stalling on startup, various versions of Spigot where large numbers of recipes were missing (and therefore things worked very unreliably), etc.
My understanding is the stability of the recipe API has only gotten worse over time, so these issues are likely more of a problem than ever. I'm not sure this is something worth doing due to this.
There was a problem hiding this comment.
In my limited testing (on paper and spigot with versions 1.21.3 and 1.21.4) it worked, but i of course don't know if it will work in all cases.
When was a dynamic solution last used?
There was a problem hiding this comment.
i don't recall specifically the last time, but it's been tried many times in the past. it works in some instances, but significantly reduces the stability of the plugin, making it really a non-starter
There was a problem hiding this comment.
So is there further testing I could do to check the reliability? (i. e. do you remember configurations that caused problems?)
Or if not has my other pr (#1345) a chance of beeing merged? (which would require it to be rewritten)
There was a problem hiding this comment.
Sorry, 1.21.5 has taken priority from reviewing any existing stuff
I'd be much more comfortable if this PR didn't contain the dynamic stuff via the recipe iterator, and then if you wanted to put that through as its own separate PR we could discuss it there
There was a problem hiding this comment.
This pr refering to #1344 and the isFuel check, fixing of isFurnacable spelling and seperation of furnaceable and blastable recipes?
Should this also include the addition of resin_clumb and leaves -> leaf_litter?
There was a problem hiding this comment.
Yeah, that one- although the spelling change is probably worth skipping. Both are technically valid (unsure if that’s a english variant thing or not), and it’s not really worth the API break for a small change like spelling
isFuel, blastable separation, and those additions all sound like changes that should be pretty easily mergeable- thanks :)
There was a problem hiding this comment.
I updated the pr
Although compiling/merging would require the dependencies in build.gradle to be updated as well which i was unable to do
| if (recipe instanceof FurnaceRecipe) { | ||
| smeltableItems.put(((FurnaceRecipe)recipe).getInput().getType(), ((FurnaceRecipe)recipe).getResult()); | ||
| } | ||
| else if (recipe instanceof BlastingRecipe) { |
There was a problem hiding this comment.
Please make sure to follow the style standards
There was a problem hiding this comment.
Refering to the line wrapping?
Does the second commit fix this issue?
There was a problem hiding this comment.
i was referring to the
if {
}
else
rather than
if {
} else {
we follow the Oracle Java Conventions, https://www.oracle.com/docs/tech/java/codeconventions.pdf
|
I forgot to add all choices of smelting recipes with a taglist as input (e. g. |
ANRAR4
changed the title
The changes replace the switch case statements for determining items smeltable and cookable inside a furnace as well as fuels for furnaces, which are used to check if an item can be put inside a furnace, with comparisons to lists of the available smelting/cooking/blastsmelting recipes or rather a check of the
Material.isFuelmethod for fuels, as the hardcoded lists tend to be outdated (e. g. missingresin_clumpin 1.21.4 and the upcoming recipe for leaves (toleaf_litter)).These changes also add further tests to check the exclusion of items that should not be allowed into the specific type of furnace, although testing seems to be currently disabled.