[1.21.1] Sync remaining c tags with NeoForge
#4161
Conversation
| getOrCreateTagBuilder(ConventionalBiomeTags.IS_LUSH) | ||
| .add(BiomeKeys.LUSH_CAVES); | ||
| getOrCreateTagBuilder(ConventionalBiomeTags.IS_MAGICAL); | ||
| getOrCreateTagBuilder(ConventionalBiomeTags.IS_RARE) |
There was a problem hiding this comment.
Im not sure about the rare biomes or the ore rates, as they have nothing to do with the tags. You could have a another datapack that invalidates these. Whats a use case?
There was a problem hiding this comment.
Much like storage block tags that can be invalidated by datapacks, it is on the modpack maker to adjust any relevant tags when they are customizing their modpacks so personally, that angle, I think is fine.
For biome rarity, some mods seems to target these biomes for uncommon ores or mobs. Not many but a few. Like Alex's Mobs seems to add Mungus to these rare tagged biomes. There's Create Arcane Engineering that adds Aurum, Ferrum, and other ores to rare biomes. Resourceful Bees spawns their easter egg Oreo Bee in rare biomes. Ice and Fire spawns their Pixie Village in rare biomes as well. So there's some use cases for targeting biome that are infrequent/difficult to find by default. And then if pack makers make a biome more or less common, they would adjust the is_rare tag to reflect the new change,
For ore_rates, I had the same thought as you of who even uses this. Turns out Tinker's uses it quite a bit. I'll ping @KnightMiner to come talk about his usages for that. RCXcrafter said he should be using ore rates tag but haven't done so yet. If packs changes the rates, they should adjust the tags too.
| @@ -166,6 +195,46 @@ private ConventionalBlockTags() { | |||
| */ | |||
| public static final TagKey<Block> HIDDEN_FROM_RECIPE_VIEWERS = register("hidden_from_recipe_viewers"); | |||
|
|
|||
| /** | |||
| * Blocks which are often replaced by deepslate ores, i.e. the ores in the tag {@link #ORES_IN_GROUND_DEEPSLATE}, during world generation | |||
There was a problem hiding this comment.
Question: Why is a sperate tag needed for these?
There was a problem hiding this comment.
This is more of the actual ground block itself (deepslate, stone, netherrack) rather than the ores themselves. Twilight Forest uses these tags for their Ore Magnet. Not sure how they use it but I can ask them to come comment here
There was a problem hiding this comment.
The original source of the ore_bearing_ground tags came from here. Pinging @SuperMartijn642 if they want to elaborate more about it. Does seem like mods make use of it. Not many but some do.
MinecraftForge/MinecraftForge#7891 (comment)
There was a problem hiding this comment.
The reasoning behind these tags was that some mods generated ore related recipes from the ore tags and that some machines, for example the pulverizer from Thermal Expansion, may drop the block the ore is in (so stone up to 1.17) as a byproduct.
In 1.17, deepslate variants of ores were added to vanilla, hence there was a need to differentiate between ores in stone and ores in deepslate. As there are mods which add additional ore bearing blocks, the idea was that if there's a way to identify the ground block the ore is in, recipes for those ores could be generated automatically as well.
So, for example, a mod may add a marble block and a marble coal ore block.
Then the marble coal ore block would go in ore_in_ground/marble and the marble block would go in ore_bearing_ground/marble.
Some mod which may want to find all coal ores and their corresponding ground block could then get all coal ore blocks from ores/coal. Then for each, check in which ore_in_ground/x tag the ore block is and then obtain the ground block from ore_bearing_ground/x.
I personally haven't used these tags and I am not sure whether these tags have actually been adopted by mods since they were added to NeoForge.
There was a problem hiding this comment.
I have an item that I have yet to port for my embers rekindled port called metallurgic dust.
When used on an ore vein it transmutes the entire vein into a random different ore from the same stone type, each ore block also has a chance of just turning into stone.
Having these tags would be great for making this item work out of the box with modded ores.
There was a problem hiding this comment.
Twilight Forest uses these tags for their Ore Magnet. Not sure how they use it but I can ask them to come comment here
For clarity, it is used for procedurally determining the block that we replace at an Ore's location when translocating the Ore block via the Ore Magnet / Tree of Mining. Thanks to the tags, there is zero hardcoding for the relationships between "source blocks" and the actual ore blocks
|
Just saw this issue report: #4150 This PR would close that issue report |
|
Seems to be crashing when running: |
|
@modmuss50 yes. I asked a few times in discord what to do about the naming of the biome fields but got no feedback from others. Basically, do we change the field name of all the biome fields which is a breaking change, change just this biome tag field to be inconsistent with the other fields, or find a way to exclude this field like how the others are excluded from the check |
Ah sorry, I am just catching up on things now. The PR looks good, as I have said before im happy for you to take the lead on this stuff and do what you feel is right. |
|
Thanks! Through I am not sure where the special casing is done. I’ll make a 1.21.3 pr in a bit to have this work be on there as well but I won’t be able to do the new wood stuff because that requires special handling with feature flags that’s outside my skill set |
I can maybe help with this, im not too sure what needs doing hopefully nothing too extreame. |
TelepathicGrunt
changed the title
c tags with NeoForgec tags with NeoForge
|
On hold until Tag Alias PR is backported to 1.20.1 and this PR updated with the latest changes from the 1.21.4 Tag Sync PR |
|
I would strongly suggest getting this PR updated to 1.21.4, let me know if you need help with that. Back porting the tag aliases is not trivial. |
|
@modmuss50 This PR is already on 1.21.4 and waiting for review lol |
|
Ah! my bad, I got confused by there being 2 PRs with the same name. Ill take a look at it shortly, we can get that in ASAP hopefully. 👍 |
Fabric 1.21.4 Port: #4186
Neoforge 1.21.1 PR: neoforged/NeoForge#1593
In the original unify tag PR, the aim was to sync majority of tags but leave out the super niche tags in order to not bug down the unify work with too much nitpicking. I think it is now a good time to work on discussing these niche tags on which to fully sync between Fabric and Neoforge and which might be time to retire in 1.22 instead of adding to the other loader.
I ran a small Python script and collected all the tags that are in NeoForge but not Fabric and vice versa. I made this PR to NeoForge to add
ctags to it that Fabric had: neoforged/NeoForge#1593This current PR is for
ctags that are in Neoforge but not Fabric. HOWEVER, I will point out, there is quite a handful of tags added and I honestly don't expect them all to be liked. Please take a look and give feedback which tag looks good, or missing entries, or is inconsistent, or you feel is too niche or pointless.Neo side is willing to discuss this. Like I am yeeting the
c:is_modifiedbiome tag out of Neo in 1.22 (marking deprecated now) instead of syncing it because that tag is outdated and lost all meaning with today's worldgen systems.I did split every tag into a separate commit to help reviewers out. It should be easier to look through all the tags in this PR than the original unify PR that lagged GitHub lol. Also ran the game locally to make sure that there is no tag logspam.
Again, no rush on this PR. Lets give it time needed for review. This PR should be easy to cherry pick into 1.21.2 as well in theory. I would like to get it in for both 1.21.1 and future versions since a lot of modpacks are gonna to settle on 1.21.1 for a while.
Side note: Looks like the
c:is_coldtag was missingc:is_cold/endtag. Fixed now in this PR.