Entry Removals in json tag files by cputnam-a11y · Pull Request #4721 · FabricMC/fabric-api

cputnam-a11y · 2025-06-25T01:33:14Z

adds a c:remove field to json tag files allowing removals from existing contents.

{
  "replace": false,
  "c:remove": [
    "brick"
  ],
  "values": [
    "brick",
    "snowball"
  ]
}

fabric-tag-api-v1/src/main/java/net/fabricmc/fabric/impl/tag/FabricTagEntryImpl.java

fabric-tag-api-v1/src/main/java/net/fabricmc/fabric/mixin/tag/TagGroupLoaderMixin.java

Earthcomputer · 2025-06-25T01:52:22Z

fabric-tag-api-v1/src/main/java/net/fabricmc/fabric/mixin/tag/TagEntryMixin.java

+	private boolean required;
+
+	@Unique
+	private final boolean removed;


Not too keen on this implementation, as is breaks assumptions that other mods may use for the TagEntry class. Instead, maybe TagFile should maintain a separate list for c:remove, in a way that is closer to the actual file representation.

The reason it's done like this is to avoid adding a field to TagFile. We'll have to discuss alternatives. I don't really like extending TagEntry, but maybe that is preferable. I considered doing more patching to fake the field better, but decided against it for now.

Whats the concern with adding a Field to TagFile? is it the fact its a mixin? Its not a deal breaker if we are careful.

fabric-tag-api-v1/src/main/java/net/fabricmc/fabric/mixin/tag/TagEntryMixin.java

modmuss50

I havent done an indepth review of the impl just yet. Overall I am happy with the propsoal.

I think we should start by having this as fabric:remove. As I said on discord I am hesitant to merge a c:remove without first being 100% sure that neo are also going to adopt this, we have been burnt by this before. Lets concenrate on ourselves first as we can easily add support for c:remove later when the stars have all aligned.

Data generation support is also required for this, and I think there is a lot of scope to improve the tests.

This looks like a great start though 👍

modmuss50 · 2025-06-30T18:41:48Z

fabric-tag-api-v1/src/main/java/net/fabricmc/fabric/impl/tag/FabricTagEntryImpl.java

+
+import net.fabricmc.fabric.impl.tag.util.WrapperCodec;
+
+public final class FabricTagEntryImpl {


FabricTagEntryImpl but it doesnt implement FabricTagEntry?

I'm not sure what to call it, as it just contains the mixin context thread local and the codec we merge. It can implement FabricTagEntry, but then the mixin doesn't implement FabricTagEntryImpl, and if we make it an interface so the mixin can implement it, then we lose encapsulation

Perhaps FabricTagEntryInternals is clearer to what it actually does?

modmuss50 · 2025-06-30T18:43:10Z

fabric-tag-api-v1/src/main/java/net/fabricmc/fabric/mixin/tag/TagEntryMixin.java

+	private boolean required;
+
+	@Unique
+	private final boolean removed;


Whats the concern with adding a Field to TagFile? is it the fact its a mixin? Its not a deal breaker if we are careful.

modmuss50 · 2025-06-30T18:43:51Z

fabric-tag-api-v1/src/testmod/java/net/fabricmc/fabric/test/tag/TagAliasTest.java

 	// Test 1: Alias two non-empty tags
-	public static final TagKey<Item> GEMS = tagKey(RegistryKeys.ITEM, "gems");
-	public static final TagKey<Item> EXPENSIVE_ROCKS = tagKey(RegistryKeys.ITEM, "expensive_rocks");
+	public static final TagKey<Item> GEMS = TagTestUtils.tagKey(RegistryKeys.ITEM, "gems");


Nit: Static import TagTestUtils, will make the diff a little nicer.

Re:

Whats the concern with adding a Field to TagFile? is it the fact its a mixin? Its not a deal breaker if we are careful.

It is a record, and there currently isn't a good way to add a record field with a mixin. With current jvm behavior it wouldn't be the end of the world, but I'd prefer to avoid it if possible. I have some unpushed commits that help with this a little, I think, so I'll re request review when I push.

Git didn't give me the option to respond to the actual comment for some reason

# Conflicts: # fabric-tag-api-v1/src/main/resources/fabric-tag-api-v1.mixins.json # fabric-tag-api-v1/src/testmod/java/net/fabricmc/fabric/test/tag/TagAliasTest.java

ChrysanthCow · 2026-02-05T04:35:35Z

Hello, I will be taking over this PR for the time being.

I'd like to ask about changing the JSON format for this PR a bit,

I do like the simplicity of the current system, but I'd like to offer a more extensible system for any future tag related PRs, for example, ordering related fields.

My main concern is that if we keep adding new fields to the root, it leaves little room for other tag PRs to add their own contents, and makes files messier.

I'd also like to suggest a shortened format for remove entries if this is the case. !. This is always prefixed first within the tag entry, before the ID and #. No other short-forms should exist as they cannot be known at a surface level.

{
    // Default vanilla values.
    // It is preferable to use this over 'fabric:values' for any vanilla supported entries.
    // Especially when developing alongside vanilla/multiloader contexts.
    "values": [
        // This exact tag is why an ordering system for tags would be nice to have.
        "#minecraft:tooltip_order"
    ],
    // Any Fabric extended values.
    // I feel this shouldn't be put into "values" mainly to avoid potential breakage within vanilla compatible datapacks.
    // If the Conventional Tags people like this, they are free to adopt this system. 
    "fabric:values": [
        // Removes Mending from this tag, throws if the entry was never within the tag or not removed by somebody else first.
        "!minecraft:mending",
        {
            "id": "minecraft:mending",
            "fabric:remove": true
        },
        // Removes othercoolmod:frost_aspect from this tag, ignored if the content was never in the tag to begin with.
        {
            "id": "othercoolmod:frost_aspect",
            "required": false,
            "fabric:remove": true
        },
        // Order related operations.
        // No short-hand form for before/after.
        {
            "id": "mycoolmod:my_new_intro", // 
            "fabric:order": 0
        },
        {
            "id": "mycoolmod:pharaoh_curse",
            "fabric:before": [
                "#minecraft:curses"
            ]
        },
        {
            "id": "mycoolmod:frost_aspect",
            "fabric:after": [
                "minecraft:fire_aspect"
            ]
        },
        {
            "id": "mycoolmod:pharaoh_curse",
            "fabric:before": [
                "minecraft:vanishing_curse"
            ],
            "fabric:after": [
                "othercoolmod:obabo_curse"
            ]
        }
    ]
}

ChrysanthCow · 2026-02-05T04:39:31Z

This is probably going into mega overengineered territory, but I thought I'd suggest the full thing to see if anybody else has a better way to handle this sort of tag extension system.

…lient tags.

ChrysanthCow · 2026-02-05T04:44:29Z

@cputnam-a11y Could you please change the target to 26.1 with these changes?

ChrysanthCow · 2026-02-05T04:54:59Z

Summary of Discord discussion with @cassiancc:

I need to update the wording on some of the comments.
fabric:values is better than fabric_extended_values
Short-hand should only be used for removal operations, not order based ones.

cputnam-a11y · 2026-02-05T05:20:36Z

@cputnam-a11y Could you please change the target to 26.1 with these changes?

I don't have the dropdown to do so. googling suggested it must be done by someone with write perms to the target repo
(Edit Feb 8): github is just having a skill issue

ChrysanthCow · 2026-02-05T23:35:52Z

Honestly, I've done some thinking about this.

I think fabric:remove is fine, mainly because I doubt we'll have anybody extending fabric:values to add their own tag data.

The order stuff could be fabric:order as a different field with its own syntax instead.

Commit

9ac8893

cputnam-a11y changed the title ~~Commit~~ Entry Removals in json tag files Jun 25, 2025

Earthcomputer suggested changes Jun 25, 2025

View reviewed changes

cputnam-a11y added 3 commits June 25, 2025 23:19

encapsulate REMOVED. remove nullability

ffab78a

encapsulate REMOVED. remove nullability

b1d9ad2

appease checkstyle

c2fff93

modmuss50 reviewed Jun 30, 2025

View reviewed changes

cputnam-a11y changed the base branch from 1.21.6 to 1.21.9 September 18, 2025 03:50

ChrysanthCow added 2 commits February 5, 2026 12:35

Merge branch '26.1' into remove-tag-entries-1.21.6

6d9d110

# Conflicts: # fabric-tag-api-v1/src/main/resources/fabric-tag-api-v1.mixins.json # fabric-tag-api-v1/src/testmod/java/net/fabricmc/fabric/test/tag/TagAliasTest.java

Mojmap-ify tag entry removal.

19a1cfc

Rename c:remove to fabric:remove, allow the system to work with c…

0e00f63

…lient tags.

cputnam-a11y marked this pull request as ready for review February 5, 2026 05:14

cputnam-a11y marked this pull request as draft February 5, 2026 05:15

cputnam-a11y changed the base branch from 1.21.9 to 26.1 February 9, 2026 02:58

ChrysanthCow added 2 commits February 9, 2026 17:18

Cleanup for ClientTagTest.

3254735

Update exception messages.

bf3e6f4


		import net.fabricmc.fabric.impl.tag.util.WrapperCodec;

		public final class FabricTagEntryImpl {

Conversation

cputnam-a11y commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

modmuss50 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChrysanthCow commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ChrysanthCow commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ChrysanthCow commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ChrysanthCow commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cputnam-a11y commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ChrysanthCow commented Feb 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cputnam-a11y commented Jun 25, 2025 •

edited

Loading

ChrysanthCow commented Feb 5, 2026 •

edited

Loading

ChrysanthCow commented Feb 5, 2026 •

edited

Loading

ChrysanthCow commented Feb 5, 2026 •

edited

Loading

ChrysanthCow commented Feb 5, 2026 •

edited

Loading

cputnam-a11y commented Feb 5, 2026 •

edited

Loading