Evaluate feasibility of recipe output when activating factory

[dquist]

Summary of changes

Currently, a factory will activate and run even when there is no room for the output to be placed in the output chest. This is because a recipe only evaluates whether the output will fit when it is applying the recipe effect after completion of the run. This results in wasted charcoal and unnecessarily runs the recipe even if it will always fail.

This PR introduces a new recipe method evaluateEffectFeasibility() that evaluates whether a recipe can be applied before activating the factory. This means that the factory will never turn in the first place if there is no room for the contents in the chest. The feasibility check doesn't need to be limited to a chest inventory check, but could be made to check any other recipe-specific condition prior to factory activation.

By default, this method returns a truthy result to imitate current behavior, but I updated the Production and Decompaction recipes with implementations to check if the results fit, since these are the two recipes most likely to run unnecessarily.

Testing

Ran on my local dev server and verified the factories don't run if the chests are full. Upon removing a stack, the factory can then be activated again.

Closes #1

Showing the factory chest is full, with some compacted stone.

Factory wont' activate.

Factory activates after removing a stack.

[awoo-civ]

The feasibility check doesn't need to be limited to a chest inventory check, but could be made to check any other recipe-specific condition prior to factory activation.

Is this a hypothetical or are there other known reasons?

If it's the former, I think this is overabstracted since the abstraction only provides customizability of the reason string which does not actually change right now.

I'd suggest:

public EffectFeasibility evaluateEffectFeasibility(Inventory inputInv, Inventory outputInv)

=>

public boolean enoughStorageAvailable(Inventory inputInv, Inventory outputInv)

The name enoughStorageAvailable matches the name of a function above in the code enoughMaterialAvailable.

This would simplify the code greatly and would probably drop the added lines of code by half. If a need arises in the future to add more reason strings, then the code can be changed.

[dquist]

Is this a hypothetical or are there other known reasons? The name enoughStorageAvailable matches the name of a function above in the code enoughMaterialAvailable.

It's a valid point - the only thought I had was supporting something like the nether portal factories from late civ 2.0 which doesn't appear to be in this code base anymore. The reason I worded it this way was to match the applyEffect() method which actually produces the output and is already written in a way to be abstract from a simple storage-oriented recipe.

There is currently only one recipe that returns false in applyEffect() not related to chest space which is the RandomOutputRecipe, when a random output can't be computed. That is probably the one recipe in the current code base that would define a different reason string.

I'm not necessarily opposed to calling it enoughStorageAvailable () and removing the reason string if we think it unlikely we'll be adding recipes with more 'complex' outputs, but it seems to be inconsistent with the existing 'effect' terminology. I'll leave that decision up to the maintainers.