Add forge ingot, dust, nugget and storage block tags #424

Merged

Lemon-Juiced merged 6 commits from dev/1.20.1 into dev/1.20.1

2023-08-19 17:32:40 +00:00

Lemon-Juiced commented

2023-08-05 21:30:00 +00:00

(Migrated from github.com)

I have implemented tags for EIO's alloys, both the ingot and nugget forms. This is very good for cross-mod compatibility as it allows other mods, primarily add-ons, to use EIO's resources from a tag in the forge directory as opposed to directly declaring the item. This would also allow mods like Thermal Expansion to have recipes for EIO alloys in its Induction Smelter without having missing tags in JEI for recipes when EIO is not installed.

I'd personally also like to add one for Grains of Infinity (for mods like Mystical Agriculture) however, I do not know how that would go with the style guidelines of the mod and if the grains of infinity are considered a dust or closer to silicon where they have their own tag.

For the SAG Mill recipes, I replicated what was created for ores in 1.12.2 on the raw resources, to be balanced with other current mods like Thermal Expansion and Mekanism. This is technically still under RFC, so I understand it not being implemented, refer to the first paragraph. However, if you do decide to implement it they will be usable in survival until a permanent answer is implemented; we have to be careful if we go this route as players might try to do something auto-crafting wise and this could break that.

This shouldn't break anything as it is just datagen.

My code follows the style guidelines of this project (.editorconfig, most IDEs will use this for you).
I have performed a self-review of my own code.
I have commented my code in areas it may be challenging to understand.
I have made corresponding changes to the documentation.
My changes are ready for review from a contributor.

Firstly, I cannot figure out how to decouple the SAG Mill recipes from the push as I added them over a week ago and can't figure out how to only push the tags. This push was originally only meant to be the tags. I have implemented tags for EIO's alloys, both the ingot and nugget forms. This is very good for cross-mod compatibility as it allows other mods, primarily add-ons, to use EIO's resources from a tag in the forge directory as opposed to directly declaring the item. This would also allow mods like Thermal Expansion to have recipes for EIO alloys in its Induction Smelter without having missing tags in JEI for recipes when EIO is not installed. I'd personally also like to add one for Grains of Infinity (for mods like Mystical Agriculture) however, I do not know how that would go with the style guidelines of the mod and if the grains of infinity are considered a dust or closer to silicon where they have their own tag. For the SAG Mill recipes, I replicated what was created for ores in 1.12.2 on the raw resources, to be balanced with other current mods like Thermal Expansion and Mekanism. This is technically still under RFC, so I understand it not being implemented, refer to the first paragraph. However, if you do decide to implement it they will be usable in survival until a permanent answer is implemented; we have to be careful if we go this route as players might try to do something auto-crafting wise and this could break that. This shouldn't break anything as it is just datagen. - [X] My code follows the style guidelines of this project (.editorconfig, most IDEs will use this for you). - [X] I have performed a self-review of my own code. - [X] I have commented my code in areas it may be challenging to understand.  - [X] I have made corresponding changes to the documentation. - [x] My changes are ready for review from a contributor.

Rover656 commented

2023-08-18 23:10:21 +00:00

(Migrated from github.com)

I am quite happy with the approach to adding the tags for ingots and nuggets, and I would suggest the addition of the block tag storage_blocks too.
As for the ore conversions, I think that these are fairly sensible recipes, however I'd like to separate these two "features" out separately, as tags are completely fine and basically already ready to go in, but the raw ores might need some back and forth.

So my suggestion would be to move the raw ores to another PR for further discussion as I think you're nearly there, then add the storage_blocks tag for the alloy blocks, then this can be merged in.

Thanks a lot for contributing!

I am quite happy with the approach to adding the tags for ingots and nuggets, and I would suggest the addition of the block tag `storage_blocks` too. As for the ore conversions, I think that these are fairly sensible recipes, however I'd like to separate these two "features" out separately, as tags are completely fine and basically already ready to go in, but the raw ores might need some back and forth. So my suggestion would be to move the raw ores to another PR for further discussion as I think you're nearly there, then add the `storage_blocks` tag for the alloy blocks, then this can be merged in. Thanks a lot for contributing!

Rover656 commented

2023-08-18 23:11:15 +00:00

(Migrated from github.com)

Also to address grains of infinity, I think it'd be reasonable to have dusts/infinity (or grains_of_infinity if infinity is too general)

Rover656 (Migrated from github.com) requested changes

2023-08-18 23:12:27 +00:00

Rover656 (Migrated from github.com) left a comment

Requested changes as listed in github discussion (github made me add a message :P)

Lemon-Juiced commented

2023-08-19 00:41:49 +00:00

(Migrated from github.com)

You're very welcome for the contribution, Ender IO has always been a favorite of mine and I'm happy to see it's coming back.

I completely understand the ores need back and forth, I couldn't figure out how to only give one commit instead of both. I'll look into adding the other tags tonight. I'm just a bit busy at the moment (I'll also see if I can remove the ore conversions from the request). You're very welcome for the contribution, Ender IO has always been a favorite of mine and I'm happy to see it's coming back.

Lemon-Juiced commented

2023-08-19 02:29:58 +00:00

(Migrated from github.com)

These changes should have addressed the issues, outlined above. I just removed the SAG mill recipes from the git instead of trying to save the code separately. I also went ahead and added dust tags for the other grains while I was at it.

Rover656 (Migrated from github.com) reviewed

2023-08-19 10:02:33 +00:00

Rover656 (Migrated from github.com) left a comment

This looks good to me, thanks a lot!!