KnightMiner / Inspirations

Mod adding various smaller features to Minecraft
MIT License
44 stars 18 forks source link

Simple item smashing with an anvil #73

Closed tommyTT closed 4 years ago

tommyTT commented 6 years ago

I re-implemented the system to smash items with the anvil (mostly) according to the specs we discussed on discord:

I implemented everything similar to the cauldron recipes and used RecipeMatch. However I needed to implement a custom sub class in order to allow multiple inputs. Maybe this is something that already exists and I didn't find it? Is there a better way to implement it? Otherwise that class could also be moved to Mantle.

The JEI implementation is very basic at the moment. Because of limited space it is limited to only display 3 inputs and 3 outputs. The requirements for the fall height and the block state are shown via tooltips.

I also included a few example recipes in the default config to demonstrate and test the configuration.

Please let me know what should be changed, especially the logic for applying the recipes I'm not too sure about. I included a hard limit for the number of times recipes are applied per "anvil operation" in order to limit the number of item entities as well as the computation time.

This PR replaces #69 which can be closed.

KnightMiner commented 6 years ago

I looked at this briefly and left a few comments on improvements. Overall I like this approach a lot better.


One thing I am considering is if it might actually be a good idea to combine both anvil recipes into one, so in addition to the item output, have a state output (which could be the same as the input, maybe use null to signify no change). This means the block smashing functions could be redirected to the item smashing object now (which could be just a smashing wrapper), and the possibility of smashing an item into a block to change it exists (like we smash blaze powder into stone and it becomes heated stone)

I am not sure how I feel about multiple recipes running when landing. I like a stack of items (like a stack of bones) all processing if any are there, but if you have multiple different inputs its too easy to get a race condition (if one recipe takes ender pearls and one takes ender pearls and blaze powder). Maybe have the recipe only match if the items list matches exactly? This would mean too many items would prevent a recipe. If combined with above, block smashing would be dependent upon there being no items (though we could after running the item smash check try a second time with an empty list of items to get block smashing that ignores items)

tommyTT commented 6 years ago

Thanks for the review. I moved the methods around according to your comments. Also I removed the nulls and replaced them with Optionals to make the usage clearer.

Merging the normal block smashing recipes with the item smashing ones would definitely be possible, but that should be a later step. At the moment the item smashing recipes are applied as well as the block smashing ones, so e.g. smashing a stack of bones on top of stone would result in bone meal as well as turn the stone to cobble. Of course having an output state in the recipe would also work when unifying everything, but requires handling of empty output stacks. "Infusion" recipes like the one you mentioned would then be possible, although I think this would make the normal configuration syntax too complicated and needs to be handled by CraftTweaker.

I changed the algorithm for applying recipes as well, so per anvil drop the first matching recipe will be applied as often as possible. This might need to be changed further, since I'm kind of battling against RecipeMatch a bit. At the moment I'm applying the RecipeMatch in a loop because the match method will only return one match. I think a RecipeMatch.matchAll method would be required that will return a Match object that tells me the amount of times the recipe was actually matched. With that in hand, I can then generate a multiple of the output.

I also changed the JEI representation of the recipes to show the input block state below the anvil and the input item stacks above it. The fall height requirement is also rendered as a text directly, but I left the tool tip as a clarification what that number actually means. Tell me what you think, in my opinion the "normal" recipes without block state requirements have too much empty space now. This is another proposal of how to maybe show a recipe which would require a custom background (this is a quick edit of an image only): jei_example