fabric
fabric copied to clipboard
Unify tag namespace and conventions between Neoforge and Fabric for 1.21 MC
Background:
The modding community has been split between two tag namespaces. The c tag and the forge tag with one using folders and the other not. Now that Neoforge is a thing and willing to have discussions about switching namespaces for their tags, we are at a point were we can finally unify both loaders to the same namespace for tags and conventions!
Neoforge PR: https://github.com/neoforged/NeoForge/pull/135
Preface:
This large tag PR is not perfect but it is a great first steps toward a unified tag namespace and convention to ease the burden between both loader's ecosystems and allow for datapacks or other ecosystems to hop in as well to our namespace and conventions if they desire.
There is a ton of moving parts in this PR. Each of these parts I felt are interconnected and are best done all together in a single to help make this switch to the new namespace/conventions as best as possible. The sections below will help break down the PR and go into detail of each part and the reasons for each part.
If after reading the reasons, you still have concerns or suggestions, please voice them on this PR so we can have a discussion!
Goals:
- Unify both modloaders to the same namespace for tags which is ~~
common~~ in this PR. EDIT: We will be sticking withcnamespace
- Add or rename tags to better match each other on both loaders and to match the convention of using folders for better organization.
- All past tags have a modern equivalent in this PR with backwards compat using optional tag entries. No tags were entirely removed.
- ~~Datagen translations for modloader common tags under the key format of
tag.<registry>.common.<tagname>where slashes are turned into periods.~~ EDIT: Will be a future PR instead
- Log short warning by default to console if old tag namespace is detected in a modder's dev environment. Config exists to either turn it off or make it verbose to say all found legacy tags. (Short dev message on by default)
- ~~Log short warning by default to console if untranslated modded item tags is detected in a modder's dev environment. Have config to either turn it off or make it verbose to say all found untranslated modded item tags. (Off by default)~~EDIT: Will be a future PR instead
Details And Reasons Of Each Goal:
Namespace choice:
Edit: We are sticking with c as we should not let this PR die over namespace concerns. It is important that modding ecosystems all agree on a single standard to make the lives of datapackers, modders, players, and pack makers easier. However, folder conventions will have to stay as I cannot get the other ecosystems to accept non-folder convention. So some work will still need to be done with fabric modder to migrate to new tags as well as spread the word to help clean up the currently polluted c namespace.
~~There are a few reasons to go with common namespace as oppose to the currently established c namespace on Fabric.~~
~~First, the word common is much more clear to new modders or to players/modpack makers that this namespace is shared across many mods. c doesn't actually mean anything to people not previously familiar with it. Which can lead to confusion on what c is for. common appears best for conveying the meaning of the namespace to people. Not a powerful reason for this but it is a reason.~~
~~Second, the c namespace is in disaster in conventions at the moment. I'm not talking within Fabric API. I mean as Fabric ecosystem as a whole. To help show what I mean, here is a sheet of all tag usages from the top 1000 Fabric/Quilt mods on Modrinth as of August 4th, 2023: https://docs.google.com/spreadsheets/d/1KBn2hKOz3nnIYh3KAeqv5vezMwFSDO-tL8TUAKYTiBE/edit?usp=sharing~~
~~In there, you can find some people are using folders under c. Some people are not. Some people are properly pluralizing their tag names. Some are not. Basically, it's a mess and I feel this was partially because some modders don't know where to find existing tags and some were not able to figure out what the conventions are when looking at Fabric API's tags. To help facilitate Goal 5, switching off of c to common makes that goal of notifying people of the new convention we are switching to a million times easier. More info about this logging is in the section below about the logging goal. But yes, this was a big reason for my choice of picking common. If we go back to c instead, please provide me an alternative solution for notifying people of the new conventions.~~
~~NOTE: I know people are going to point out that common is a valid modid unlike c and that this makes common bad. The truth is, that's actually a non-issue. Say if some random mod is made using common modid, the modding community will just ignore that mod anyway and it doesn't impact modpacks or anything. Modders will just tell the owner of that mod to change the modid and that'll be that. If it is a heavy concern still, modloaders themselves can just include a dummy common modid mod to reserve that namespace to prevent anyone else from using it. Super simple solution. I will leave the choice of reserving the common modid up to the modloaders themselves where they can make a PR after this one if we stick with common namespace.~~
Renaming and moving tags to same conventions
Fabric's c convention was to never use folders. Forge's convention was to use folders. And Minecraft started out with not using folders but then now does with its block mineable tags.
What it looks like to me is that Minecraft starts with no folders originally but as they grew, they started seeing the value of grouping similar tags together by folders. But they still keep the non-folder tags for backwards compatibility. Forge right away saw that trying to mimic vanilla would be an issue for scalability in a modded ecosystem so they went with folders. Fabric however, stuck with the legacy non-folder convention that vanilla did and this lead to a lot of messiness that lead to the current situation with the c namespace tag conventions: https://docs.google.com/spreadsheets/d/1KBn2hKOz3nnIYh3KAeqv5vezMwFSDO-tL8TUAKYTiBE/edit#gid=0
If you check what fabric currently does with dyes, all of them are in the format of c:<color>_dyes which just bloats the top level view of the c tags: https://github.com/FabricMC/fabric/tree/93d8cb82e85c3d5744716430516daef393cf5815/fabric-convention-tags-v1/src/generated/resources/data/c/tags/items
Instead, if we go with the folder convention, these tags would've be grouped under c:dyes/<color> which is a lot more organized and cleaner. It is also far more scalable for a modded ecosystem as more tags can be added to the folder without making the top level view harder to find certain tags. Example of what a folder dye tag looks like:
https://github.com/neoforged/NeoForge/tree/1.20.x/src/generated/resources/data/forge/tags/items/dyes
That is why the convention of this PR is plural + folders. Because of this, many tags from both c and forge were shuffled around so that tags that mean the same thing on both loaders now matches each other in the c namespace so multi-loader modders only need to pull from 1 tag instead of 2.
Backwards compat with old tags
I put a lot of work into making sure that all old tags from the modloader are added as an optional tag entry in the modern equivalent of the tag. This will allow built jars and datapacks using the old tag names to continue to have compatibility throughout 1.21 Minecraft. Then these backwards compatibility can be deleted in 1.22 Minecraft to clean up this remaining tech debt as everyone had an entire major Minecraft version to update their tags.
~~However, because of all the tag renaming and moving around, the tag fields in code were also rename to match the changes. This means this PR is indeed a breaking change for code references to these fields. This is why this PR is marked for 1.21 Minecraft as that is a good time for doing breaking changes. The reason I did not keep the old field names nor make dummy fields that references the new fields is this would create a lot of tech debt that I felt the modloaders don't want to take on. If this is a deal breaker and that the dummy fields should be added to keep code compat, let me know and I'll update this PR to not be breaking for code even with the tech debt.~~ Edit: New commit now makes the old tag module marked as deprecated with comments redirecting to v2 of the tag module with the common tags. This means this PR is no longer a breaking change for code usage.
As you read this PR's code, you'll notice I put the optional tag entries at the bottom of all the datagen classes for tags. This is so that the optional entry shows up last in the tag's json (cleaner to me and shows it is not super important to viewers) and also makes it very easy to delete in 1.22 Minecraft. Just select the code block at bottom and delete. Done. No more backwards compat. Hopefully this helps!
More info on individual tags can be found in the Misc section.
Datagenning tag translations
EDIT: Will be a future PR instead
~~This might seem strange at first but it turns out that recipe viewer mods are making use of translations for item tags. I asked about this and another person said their own mod would also benefit from item tag translations. So there is a demand for translations of these tags and currently, every mod that does will need to add translations for all the c and forge tag entries from the modloaders. Doesn't seem right.~~
~~So instead, I added translations for the tags and I figured, since I am here making item tag translations, might as well add translations for the rest of the common tags for completeness. I did not add translations for minecraft item tags but if desired, I can update this PR to include datagen translations for Minecraft's item tags. Not sure if this is something a Modloader wants to handle from Minecraft. But at the very least, modloaders should be translating their own tags to help out modders and reduce duplicate translations between mods.~~
~~Also, TagUtil.getTagTranslationKey is added as a helper method that modders can use to make it easier to datagen tag translations. Just takes the TagKey and spits out what the lang key is.~~
Log warning for legacy namespace presence
This part is critical to getting modders to update to the new namespace and conventions. Basically, there's no way we can reach out to every modder through Discord or through changelogs as people just update their modloader/minecraft version and start porting. There are too many ways that people can slip through the cracks.
So to have a solution that gets this new namespace and convention change out to all modders that actually need this info, this PR include code that will grab all the tags from all Minecraft registries and check for the presence of the legacy namespace. If it is found, it will log a short message that states:
Dev warning - Legacy Tags detected. Please migrate your old `c` tags to our new `c` tags that follows better conventions!
See classes under net.fabricmc.fabric.api.tag.convention.v2 package for all tags.
NOTE: Many tags have been moved around or renamed. Some new ones were added so please review the new tags.
And make sure you follow tag conventions for new tags! The convention is `c` with nouns generally being plural and adjectives being singular.
You can disable this message by this system property to your runs: `-Dfabric-tag-conventions-v1.legacyTagWarning=SILENCED`.
To see individual legacy tags found, set the system property to `-Dfabric-tag-conventions-v1.legacyTagWarning=DEV_VERBOSE` instead. Default is `DEV_SHORT`.
If set to verbose, the above message will print out this format:
Dev warning - Legacy Tags detected. Please migrate your old `c` tags to our new `c` tags that follows better conventions!
See classes under net.fabricmc.fabric.api.tag.convention.v2 package for all tags.
NOTE: Many tags have been moved around or renamed. Some new ones were added so please review the new tags.
And make sure you follow tag conventions for new tags! The convention is `c` with nouns generally being plural and adjectives being singular.
You can disable this message by this system property to your runs: `-Dfabric-tag-conventions-v1.legacyTagWarning=SILENCED`.
To see individual legacy tags found, set the system property to `-Dfabric-tag-conventions-v1.legacyTagWarning=DEV_VERBOSE` instead. Default is `DEV_SHORT`.
Legacy tags:
TagKey[minecraft:block / c:barrel] -> TagKey[minecraft:block / c:barrels]
TagKey[minecraft:item / c:potato] -> TagKey[minecraft:item / c:potatoes]
TagKey[minecraft:item / c:blue_dyes] -> TagKey[minecraft:item / c:dyes/blue]
This is set to print only in DEV_SHORT mode by default so that mod's development environment after world load will see only the shortened message. The modes are SILENCED, DEV_SHORT, DEV_VERBOSE, ~~PROD_SHORT, PROD_VERBOSE. Prod is included as an opt-in option in case modloader maintainers want to see how many legacy tags are in use for 1.21 modpacks. The prod options can be removed from PR if desired.~~ EDIT: Prod option removed
Hopefully this isn't too intrusive. But at least the message says how to turn it off as well as where to find the new tags. This should greatly help speed up modders switching to the new tags and help them see what the desired conventions are. ~~The end goal is a common namespace that is not a wild-west of conventions like the c namespace is. Will this be successful? I don't know but I have hope that it will. Only way to know for sure is to try this out.~~
Log warning for untranslated item tags presence
NOTE: This part is a bit more controversial but if desired, this logging can be chopped out of this PR.
EDIT: This is now set to off by default and has to be opt-in by the modder in the config file.
The idea is since recipe viewers and other mods rely on item tags to be translated, a short message will be logged if any are found to be untranslated from mods. I did not do the other tag types because there is no demand for those to be translated and thus, doesn't make sense to push for translated tags of other stuff. Only item tags.
The code in this PR will grab all the item tags and if running on client side, check if the tag is translated. If it is not, it will log a short message that states:
Dev warning - Untranslated Item Tags detected. Please translate your item tags so other mods such as recipe viewers can properly display your tag's name.
The format desired is tag.item.<namespace>.<path> for the translation key with slashes in path turned into periods.
You can disable this message in Fabric API's properties config file by setting log-untranslated-item-tag-warning to "SILENCED" or see individual tags with "DEV_VERBOSE".
Setting it to verbose basically prints out all the item tags like so:
Dev warning - Untranslated Item Tags detected. Please translate your item tags so other mods such as recipe viewers can properly display your tag's name.
The format desired is tag.item.<namespace>.<path> for the translation key with slashes in path turned into periods.
You can disable this message in Fabric API's properties config file by setting log-untranslated-item-tag-warning to "SILENCED" or see individual tags with "DEV_VERBOSE".
Untranslated item tags:
modded1:pies
modded2:potatoes
modded2:paintings/1x1
~~This is set to print only in DEV_SHORT mode by default so that mod's development environment after world load will see only the shortened message.~~ The default is SILENCED mode and has to be opt-in. The modes are SILENCED, DEV_SHORT, DEV_VERBOSE, PROD_SHORT, PROD_VERBOSE. Prod is included as an opt-in option in case modloader maintainers want to see how many untranslated modded item tags are in use for 1.21 modpacks. The prod options can be removed from PR if desired.
Misc
So for the individual tags in this PR, there's a lot of changes done. This is not a comprehensive list but some notable points.
- The deprecated tag fields for buckets and tools were deleted since this PR is already a code breaking change. Might as well do a small cleanup. These can be added back if no code-breaking change is wanted.
- Folders are added and many tags migrated to them.
- block tag c:movement_restricted is renamed to c:relocation_not_supported since that tag name is what many forge mods were using and it covers teleporting blocks to another location. Movement restricted sounded like it was for piston pushing stuff when in reality, it is meant for any kind of block relocating behavior.
is_prefix is added to the biome tags to match the Neoforge PR and how vanilla names their biome tags.
c:tools/bowswas split intoc:tools/bowsandc:tools/crossbowsbecause these are two separate tools and has different behaviors. Mods that don't care can pull from both tags. But mods that do now can pull exactly what they need. Also,c:tools/fishing_rodswas added as well since it is a tool and I seen quite a few modded Fishing Rods made.
- Brand new
c:dyed_blocksandc:dyed_itemstag was added and has a folder for individual colors in it. I added this because the Forge tags had 32 colors tags for Stained Glass and Stained Glass Panes. It's awful. Imagine if this was extended to every colored block... My solution was to remove those colored glass tags and instead, create new overarching tags that collect all dyed color blocks and items. And the subfolder for these dyed_blocks and dyed_items lists each individual color. The goal is now if you wanted Red Stained Glass Blocks, you would check if a block is in thec:glass_blockstag and is also in thec:dyed_blocks/redtag. If so, there's your collection of Red Stain Glass Blocks! This is now universal and can be used for any dyed color block! Much cleaner and far more useful. I hope color-based mods like Spectrum will make good use of this!
c:capturing_not_supportedentity type tag now added because there's over 20 mods that I know of that lets players carry an entity or stuff them into an item to move around. I added one entity myself and had to add it to so many mod's disallow tags for picking up into items. We definitely need to standardize this so entity creators only have 1 tag to add to and other mods can pull this tag into their own disallow tag for their stuff.
- Not every tag from the Neoforge PR is ported over to this PR. I only ported the tags that I assumed would be very useful to have such as
c:tools/crossbows. The reason for this is some tags in the other loader could be questionable so rather than bog this PR down with arguments over these various tags, I'll leave it up to another person's PR to attempt to add tags from one loader to another after my PR. All I did was make sure that the tags in each loader follows a convention and tags for the same thing matches up.
Conclusion
Again, please let me know your feedback or suggestions or thoughts about the various parts of this PR. It is not perfect and getting loaders to use the same namespace and convention for tags is a controversial topic. That is why I am making this PR early long before 1.21 so we can have enough time for discussions.
Please also checkout the discussions on the PR for the other loader too as what is talked about there could also impact this PR. No matter what namespace is chosen in the end, both PRs will use the same namespace. That is a hard requirement of this PR so please make sure others from the other loader are involved in the discussion.
Neoforge PR: https://github.com/neoforged/NeoForge/pull/135
Thank you for you time to read this! I know it was long. But this is very important for the modding ecosystems as what is decided for these PRs will have long-lasting impact.
@TelepathicGrunt I recommend not dropping a breaking-change PR directly, without any prior discussion... I also wonder what's the Quilt's position on this.
@apple502j I had some discussions already with some Fabric maintainers. But best shown in PR what the idea is and the PR can change if needed. As stated, if desired, I can adjust PR to not have the breaking changes with the fields but comes at cost of more tech debt.
Quilt pulls from Fabric so it doesn't matter if they want their own thing, they still have to be compatible with Fabric anyway. So discussion will be here if Quilt wants to talk about it.
Edit: If the concern is about the wording of "both loaders", it's because I consider Quilt as a subset of Fabric in terms of tag compatibility. So it wouldn't make sense to PR Quilt when they will pull this PR in anyway from Fabric. For the purpose of this discussion, Fabric/Quilt are considered the same ecosystem.
Edit: If the concern is about the wording of "both loaders", it's because I consider Quilt as a subset of Fabric in terms of tag compatibility. So it wouldn't make sense to PR Quilt when they will pull this PR in anyway from Fabric. For the purpose of this discussion, Fabric/Quilt are considered the same ecosystem.
This is not true at all. Quilt can just not support this for Quilt-exclusive mods and make a hack so it converts back to c.
This is not true at all. Quilt can just not support this for Quilt-exclusive mods and make a hack so it converts back to
c.
If that is what Quilt ends up wanting to do, that is totally fine. I can't stop anyone or any modloader from what decision they ultimately choose. But lets focus back to this PR please.
Please keep this discussion on topic, this is about what we are doing for Fabric. 👍
Second, the
cnamespace is in disaster in conventions at the moment.
AFAIK this is somewhat due to Forge->Fabric mod ports just changing the namespace and using those conventions on the wrong platform. I'm a bit worried that if a new namespace is introduced, people will just move their existing tags to a new namespace, further adding to the tag soup:
- Proper
common:tags (common:tools/pickaxes) - Old
forge:tags (forge:tools/pickaxe->common:tools/pickaxe) - Old
c:tags (c:pickaxes->common:pickaxes)
In my opinion, c is still a good namespace choice here due to
- It not being a valid mod ID
- The short name also making the namespace feel unimportant in tag (etc) IDs
I've also seen the c namespace being used in non-tag contexts such as #3129, so this change wouldn't only affect tags.
What it looks like to me is that Minecraft starts with no folders originally but as they grew, they started seeing the value of grouping similar tags together by folders. But they still keep the non-folder tags for backwards compatibility.
As far as I can tell, there are only three examples of folder tags:
mineabletags- Banner pattern item tags
has_structuretags
(This comes from a quick search through the Minecraft Wiki list of tags, it's possible I missed something)
Many "folderable" tags have been added in 1.19/1.20 (cherry wood, mob spawn tags for biomes etc) since the mineables and other folder tags were introduced, so it's not purely backwards compat.
Since the folder naming pattern is inconsistent with vanilla, I feel like a nice solution for that would be to support some sort of tag aliases where c:ingots/copper and c:copper_ingots could refer to the same tag.
Log warning for untranslated item tags presence
IMO this is a bit questionable since it affects a non-modding feature (data packs), making the modding API create log spam for untranslated tags that can be added by data packs – which can't provide translations in the first place.
Note with my latest push I just did: common:capturing_not_supported entity type tag now added because there's over 20 mods that I know of that lets players carry an entity or stuff them into an item to move around. I added one entity myself and had to add it to so many mod's disallow tags for picking up into items. We definitely need to standardize this so entity creators only have 1 tag to add to and other mods can pull this tag into their own disallow tag for their stuff.
Picture of my pain with this lmao: https://imgur.com/svORGdt
Just some thoughts, not a review, I also don't particularly care if tags are folders or not so long as the tags exist and are used.
c was chosen as it was (apparently) common for datapacks to use at the time. I don't know how true that is, but that was part of the logic for choosing it. Has that changed?
Currently, FAPI uses c to mean "conventional," hence the API name... it would be weird to have a "conventional tag api for common tags." I'd rather keep the c namespace.
I'm still not sure how the top level view of a datagenned datapack folder is of any consequence - especially when the API part is still "bloated." Maybe we need an API for getting tags within folders?
I'd rather have the compat issue be dealt with by tag aliases (some form of API that says tag A and B are the same).
For consistency with Yarn, the most deeply nested element of a tag key should come first in its field name; for example, the
minecraft:mineable/pickaxetag key is assigned to theBlockTags.PICKAXE_MINEABLEfield.
For this, I matched that convention in new PR but I will say it is really awful for biome tags. Biome tag fields now look like this
I made a issue report to Yarn to choose a new format for Biome tags because the examples in BiomeTags currently are really bad to read. If that issue report gets resolved, I'll convert this PR's biome tag fields to whatever new format yarn choose for them if they do make a change https://github.com/FabricMC/yarn/issues/3661
Being able to use the same recipes and tags sounds great, so many times I've forgotten to edit the files when coping changes between the two loaders.
I'm not sure about the common:dyed_items tags; If you wanted red glass, how would you use it in a recipe?
As far as I'm aware they can't filter a tag, so you couldn't say in common:glass_blocks and common:dyed_items/red
So either you'd use the item ignoring the tag system, or create a new tag that would still be missing mod added items.
The recipe json would need to support "and" (and maybe "or") blocks; "and" requiring the item to be in all tags ("or" is any item form the tags)
"key": {
"R": { "tag_and": [ "common:glass_blocks", "common:dyed_items/red" ] },
"G": { "tag_or": [ "common:glass_blocks", "common:glass_pannes" ] },
},
Alternatively have tag additions filterable, then they can be created and use in recipes as needed. red_staind_glass.json
{
"replace": false,
"values": [
{
"id": "#common:glass_blocks",
"filter": "#common:dyed_items/red"
}
]
}
@ShetiPhian Just to clarify for you, the dyed_items tag intersection checks are already accessible to both modders and players/modpack makers. Fabric and Neo both implement custom ingredients that allow for matching items that exist in both tags. The Fabric PR is here: https://github.com/FabricMC/fabric/pull/2586/files#diff-706c7797c8302729d8d33b83bf7882a0e14b564e43e8ffe766a8784ff1d64ad6R37
https://github.com/FabricMC/fabric/pull/2586/files#diff-e23003198861ce8b8037728aec48f17a88b4ba99dcd1ddd57c00f77cccc7c348R17
Players/Pack makers can also have easy access to these custom ingredients as well through kubejs/craftweaker.
So the dyed tags should be good and usable in recipes. If you need help with getting the custom ingredients to work, I think there's some folks in Fabric Discord that should be able to help you out.
@TelepathicGrunt well may awareness was off ;) That solved the only potential issue/concern I had.
Since the folder naming pattern is inconsistent with vanilla, I feel like a nice solution for that would be to support some sort of tag aliases where
c:ingots/copperandc:copper_ingotscould refer to the same tag.
Agreed. Currently, tags can only have a one-way inclusion relationship. We can implement such an equivalence if we mod tagmanager to support treating multiple identifiers as aliases via some sort of data like mcmeta, and if such a system exists, we don't need to worry too much about the conventions either.
@Juuxel Going to give some of my thoughts on your comment. There's a lot of stuff floating around in my head. Yeah, there is concern of people grandfathering in their old conventions, but that's what goal number 5 is supposed to address. By logging to those users where to see the new tags and explicitly telling them to follow the new conventions, my hope this will make people actual visually look at the new tags and see what the correct naming is.
If even after all that, they still decide to use the wrong naming, that's on the modder at that point. As a modloader API, we did all we could by datagenning the new tags, having an API of the tags, and literally poking them in the logs to look at those tags. There isn't anything more that can be done but I feel the logging that is done could be a huge difference! The Fabric Tag Wiki existed but wasn't able to resolve the tag issue due to visibility issues (I never learned about the wiki till much long after it was made) but the logging impacts all modders right in their log right as they port so visibility issue is resolved with that.
So if we go with c instead, I don't know how we would even do the logging properly. Maybe it could check for presence of all the old c tags that was renamed/moved in Fabric API? But that won't catch people rolling out their own c tags with wrong conventions. The logging won't be able to reach them to recheck the conventions. That's a big concern I have over that. Otherwise we would need to put together some sort of "Convention Team" that scans mod sites for mods messing up the c namespace and contact those modders but this require time, manpower, and effort. I doubt anyone is going to volunteer to do that.
But it sounds like @dexman545 and @liach want's to do aliases but I don't know to make those and not sure how they would function. If it is where Fabric API has the common tags but they also act like c tags, doesn't having the c tags as optional entries in the common tag already fulfill that role for backwards compat? But if it is that Fabric API keeps the c tags but they also act like common tags, then that defeats the entire purpose of a unifying namespace across modloaders. It'll also create significant confusion as modders would look into Fabric API and see c tag JSONs or APIs and cause modders to continue to make new tags under c rather than properly switching to common to also work with other modloaders/datapacks. Now aliases on the folder vs non-folder forms... maybe? I guess that could work but why not just include the non-folder forms as optional tags in the folder forms?
Ultimately, the whole concept of aliases weirds me out as it clashes with the entire purpose of unifying namespaces and how tags works. I rather both PRs either stick with common or stick with c. If c is desired, you can pitch your concerns to Neoforge's PR and see if you can convince them to stick to c.
But yeah on the c not being a valid modid, common can too be not valid by modloaders reserving it. So whether a namespace is valid or not doesn't really argue for or against any namespace. Lastly, "The short name also making the namespace feel unimportant in tag (etc) IDs", what do you mean by that? I am not sure I understand that sentence but if you can clarify, I can give my thoughts about it.
The packet/networking stuff can indeed stick with c. But tags are exposed to users and modders and I feel common would be best for tags. It is a difference between the two parts but I think that's ok as long as the namespace fits the role that it is chosen for to get maximum benefit.
@dexman545 I never actually heard about c being chosen because of datapacks. I always heard it was initially chosen mainly because it wasn't a valid modid. Though What I am seeing is datapacks are adding both c and forge namespaces to try and be compat with multiple loaders. So if we unify namespaces, that's less typing datapackers have to do which should make them happy.
But the funny thing is the part where you said that c means conventional, I did not know that and it seems most people did not either. As far as I could tell, most people thought c actually stood for common. Which does bring up my first reasoning for using common which is it is much more clear of what it is and actually means than a single letter. Since it seems people are split on what c really stands for outside of people who originally discussed adding c to Fabric API in the first place. I like being more explicit than implicit tbh but that may just be me. If needed, we can always rename the API from conventional to common but will wait for feedback from Fabric API maintainers on if that should be done or not if common is stuck with.
Tag aliasing is to make tags A and B equivalent. This differs from adding A to B as an optional entry, as anyone using A will also get what was added to B, as if they had used B in the first place.
For me, tag aliases solve the same problem as unifying the namespace and conventions - instead of forge and fabric changing what they do for all the mods and datapacks, they are made functionally equivalent in practice. So mod M could register fabric tag A and forge tag B as equivalent when porting, and the same recipes/tags will work for both. I'd argue FAPI does have a place for providing some of the more common aliases. Obviously, it could also be used to ease the porting to the new conventions. It would also have the benefit of not needing datapacks to update, as they aren't really going to be notified of this change.
If visibility of new conventions is needed, v1 of conventional tags could be deprecated and we get a v2 with the new ones. v1 can be kept around as its very low maintenance. This way we can also keep c. The log warning could also show only in dev, as its the modders who mainly need to see this.
As for c diminishing the importance of the namespace in the tags, to me it as such: it does not matter who is providing the tag, so the namespace is irrelevant. Deemphasizing it by making it one letter is essentially to work around the fact that we need some namespace for the tag. The important part is the name of the tag.
The history of c is nebulous at best. It was a community convention, not something defined by fapi until this API was made years later. It being used in datapacks was something I heard in a discord conversation a very long time ago. Maybe that was important for some modders, and others valued it not being a valid modid more. If you ask the modders who used tags at that time, each one will probably give a different answer.
c was for common initially. Arguably still is. Fapi has taken it to mean conventional now, at least so far as this API is named. I don't think anyone else does though. @Technici4n suggested the change, I believe, so he'll have to give you the reason (I think it was something to do with server/client/common conflict? I don't remember). I do not believe there is any confusion on what c means. I'd be fine with changing the API name to Common[...] if confusion is an issue, barring the reason it was changed initially, but I don't think it is particularly relevant in this case as both just mean shared tags. ~~The PR for this API was even "common-tag-v1"~~
I am primarily a datapack developer, and I support the movement for a merged common namespace. It is quite annoying having to support both c and forge tags for compatibility. I also am not able to easily duplicate the change to the other namespace, as the current conventions are different. For example, Fabric has c:floral, but there is no equivalent in Forge. Additionally, the difference in names is annoying to deal with. Forge has forge:is_mountain, forge:is_peak, and forge:is_slope; but Fabric has c:mountain, c:mountain_peak, and c:mountain_slope. In fact, when writing this out, I completely missed the forge:is_slope tag when searching for the equivalents.
Tl;dr, I want the same classification and experience when my datapacks are used in conjunction with Fabric and Forge mods, but the lack of tag unity limits this.
I'm not sure how important this really is to the conversation, but I will also mention I have not heard of the c namespace being chosen due to datapacks. I've always considered c as "common" - used by Fabric - and forge for Forge, and us datapackers simply use them for compatibility within ourselves and other mods, or just make some under our own namespace. Whether or not c was chosen with datapacks in mind, I agree with many of TelepathicGrunt's points. I look forward to seeing things like tags standardized - it may be extra work now, but will make life easier for everyone in the future.
@catter1 Are you exporting objects to these common tags or importing objects from these common tags?
A temporary workaround for these can be, that assume you have your desired tag mypack:my_tag for both purposes, and you want to connect to c:common_tag and forge:forge_tag. Then, you can make an extra mypack:my_tag/read (exclusively for importing purposes, used in commands) tag that includes #c:common_tag and #forge:forge_tag, while you put everything you want to add to mypack:my_tag (which is only used for exporting purposes, i.e. only defined, not read by commands etc.) but you do not read from it in commands.
The structure looks like: (a -> b means a is included in b)
mypack:my_tag -> #forge:forge_tag
mypack:my_tag -> #c:common_tag
mypack:my_tag -> (whatever other common tags there are)
#forge:forge_tag -> mypack:my_tag/read
#c:common_tag -> mypack:my_tag/read
(whatever other common tags there are) -> mypack:my_tag/read
@liach That’s the problem. Everyone is currently making their own tags that’s imports c and forge tags to try and collect them for their own usage. Ideally with these PRs settling on a single namespace, that resolves the issue of needing personal tags for consolidating multiple standards.
Less typing extra entries in tags. And possibly less tag files overall needed. That’s what catter was pointing out.
Edit: To be clear, any mod doing code references to a tag should have their own personal tag so pack makers can change it without affecting the common tag. But I can see datapacker’s stance for not needing a personal tag as they are already entirely configurable and whatever is using the tag in the datapack can be edited by modpack makers to point to another tag.
I think that the current tags are fine and should only be extended as needed. Moving from c to common is just moving from one standard for tags to another. The changes that need to be made for cross-platform compatibility cannot happen in this repository.
any mod doing code references to a tag should have their own personal tag so pack makers can change it without affecting the common tag
Does it mean that mods should use their own tags that include the common tags at use sites (like in command, code isIn, etc.)? Or should the items be defined to mod's tags and have that one mod tag defined to common tags? Either way, it significantly reduces the usefulness of this patch.
I think that even with this integrated, we still have the need to enhance tag itself: to allow aliasing (equivalence), allow specific tag overriding (such as removal from tag building without replacing everything), which IMO are actually more useful than the work done here.
And finally, on the naming conventions. The conventions ask for using plurals without exact reasons (aside from Mojang tendencies, but yarn contributors have seen how bad they organize their code). I personally would prefer singular, like dye/red, like the package names per Google Java Style.
My last thoughts on the aliasing talk as I was talking with others who haven’t commented here yet. They made a great point.
If Fabric aliases c:coal_ores to c:ores/coal, and a modpack maker sees a mod using the first tag and decides to empty out or add to that tag to affect just that mod, the change is done to the other tag as well. And they will look at c:ores/coal and see no references to c:coal_ores.
Their thoughts? “What the heck is going on??? How is this change in one tag even affecting the other?”
It’ll look like a bug to players and modpack makers because the tag files do not reference each other because the aliasing is invisible.
Furthermore, aliasing just allows for incorrect standards to continue to proliferate and cause more confusion as now people might make their own c tags in either aliased formats and expecting it to aliased together when it won’t (im assuming the aliasing had to be explicitly done for specific tags and not some automated system trying to figure out what tags are equivalent) and if automated, may lead to false positives which leads back to more confusion for players and pack makers on strange behaviors they see
If aliasing is to be done, it should be a separate pr to Fabric API and if merged in, I’ll see about editing this pr to use it. But otherwise, I’m personally not a fan of aliasing the more I think about it. Let’s have the tag aliasing talk be moved into its own PR if any of you get a chance to make it
If Fabric aliases c:coal_ores to c:ores/coal, and a modpack maker sees a mod using the first tag and decides to empty out or add to that tag to affect just that mod, the change is done to the other tag as well. And they will look at c:ores/coal and see no references to c:coal_ores.
This problem is not exclusive to tag aliasing; simple tag inheritance is similarly invisible. Despite tag inheritance suffering from a similar problem to what you mention on paper, the actual impact of unclear relations to other tags has been minimal enough that it should not be used to disregard tag aliases entirely.
Their thoughts? “What the heck is going on??? How is this change in one tag even affecting the other?”
Why would you ever think that aliasing will be code-exclusive, when the code cannot even always fetch the tags (given they might not exist)? Of course it will be defined in data packs, presumably as some extra key in the JSON format or some extra metadata properties in .mcmeta files.
decides to empty out or add to that tag to affect just that mod
Why would any of these 2 things happen? They are both incorrect usages of tags.
-
A tag should never be emptied out. If you don't want to use it, you can choose to never refer to it.
-
Why would you change a common tag to affect ONE mod? This falls back to the rabbit hole you described initially:
Everyone is currently making their own tags that’s imports c and forge tags to try and collect them for their own usage.
And what's the implication? It renders this PR meaningless, since everyone will go back to use their own tag for reading than read directly from the new common tags.
Hey, I'm rx97 speaking as a datapack developer. I wholeheartly support the common convention as a way to unify modloaders / the ecosystem in general especially for datapackers.
cwas chosen as it was (apparently) common for datapacks to use at the time. I don't know how true that is, but that was part of the logic for choosing it. Has that changed?
I was surprised to hear this as I've never heard of datapacks even today using them (maybe stardust labs).
common would really benefit datapacks as a standard and could really be embraced in the conventions and compatibility standards (something I'm contributing a lot of energy towards via the @SmithedMC Project). I don't see datapacks choosing these tag conventions if the ecosystem is split on c and forge with inconsistent backlinking and confusing (re)implementations across mods and more.
tag-aliases seems like a footgun and a mistake to consider for this, basically nullifying this as a standard.
I am completely in favor of this PR - I haven't checked the details yet, but I approve of the direction.
Yes it is a breaking change, but it's for the best.
Tag aliases are a no-go for me.
At worst, tag aliases are a solution to a problem that doesn't exist. I'm still not convinced that switching namespaces from c to common will affect anything. Namespace pollution concerns are not fixed by changing the namespace. Even with the namespace of fabric that was and is used for functionality only, mod developers added their own tags.
Nor was this pull request properly reviewed, as all the discussion seems to have happened entirely outside of Fabric development spaces. The standard being discussed here seems to be more of an attempt to redo the exact strategy of creating a conventional tag standard that Fabric already has done with its conventional tags module. Why does Fabric have to adopt a new convention when the existing one still exists? The stated reasons do not convince me of the necessity.
Nor was this pull request properly reviewed,
Haykam, I think you are heavily confused of what a PR is. A PRs shouldn’t be only reviewed outside of GitHub. A PR IS the review and discussion. That’s what a PR is entirely for. That’s the purpose of a PR lmao. It’s to have this talk here instead of it isolated to a discord server that not everyone is in behind closed doors. That’s why I went and made the PRs finally so the discussion can be dragged out into the open
Yes, creating the pull request is helpful so that the flaws of the pull request can be properly assessed. The number of people seemingly blindly approving the pull request indicates that discussions happened elsewhere, though, and now further feedback won't be considered due to these people already having made up their mind.
I think we should keep "c" namespace, because it doesn't stand for fabric, instead, common.
@TelepathicGrunt Thanks for your lengthy response to my questions :tiny_potato:
If even after all that, they still decide to use the wrong naming, that's on the modder at that point.
I'd note that it's like that even now. Fabric API and vanilla provide the current naming standards of c tags, so all kinds of weird Forge-ism c:fruits/banana or singular c:tin_ingot tags are examples of mods not following the examples.
The documentation is pretty lacking for the current convention if you look at the Fabric Wiki as an example:
Common tags should be named with the syntax c:yourtaghere, with c standing for common. When making the file, use the file path src/main/resources/data/c/tags/ and then blocks, items or fluids You should separate words with underscores, and tags should be plural.
This could do with some examples, and it doesn't help that c:yourtaghere goes against the rules...
I'm also not inherently against a folder system anymore as I was last time (although there the idea was copy-pasting Forge's unnatural tag naming system with mixed singular and plural names, which was worse than your proposal)
Lastly, "The short name also making the namespace feel unimportant in tag (etc) IDs", what do you mean by that? I am not sure I understand that sentence but if you can clarify, I can give my thoughts about it.
It wasn't really an important point (more of an observation/side note), but I meant that c takes up less space visually than common in tag IDs: compare c:buckets vs common:buckets.
The packet/networking stuff can indeed stick with c. But tags are exposed to users and modders and I feel common would be best for tags. It is a difference between the two parts but I think that's ok as long as the namespace fits the role that it is chosen for to get maximum benefit.
Splitting a namespace intended for "common" usage is a bit ironic 😅 I feel like it would be best to use the same NS everywhere for consistency, so the chosen NS would be the choice of NS for all shared IDs, whether it's tags, networking, consent APIs etc.
If c is desired, you can pitch your concerns to Neoforge's PR and see if you can convince them to stick to c.
I'm a Fabric triage team member so I'll only be reviewing this from that POV (I probably won't have time to review this on the Neo side 😛)
When it comes to log warnings, while the idea is good, I'm also just a bit worried that they'll have trouble reaching some modders anyway. There are log warnings for missing models or broken recipes etc too, and those show up in most packs.
I don't know if there are more effective solutions though, aside from crashing when outdated common tags are found (which is not a good idea at all, of course)