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

* Fix LivingEntityFeatureRenderEvents.ALLOW_CAPE_RENDER * Make test better

…MC#4911) * Sort debug entries * Fix sorting logic * Also search by namespace

* Add refreshCommandCompletions method to fcapi * Implement requested changes * Add some comments and feedback

* Conventional armor tags * fill new tags

* Remove all @NotNull annotations * Missed one

* Start working on packet splitting * Fix checkstyle * Allow for split C2S packets, require setting max packet size (not validated on receiving end yet) * Remove old init * Finish up generic packet splitting, * Add fake passthough packet to avoid encoding splittable packets twice * Validate maxPacketSize * Source split size from CustomPayloadX2YPackets * No more magic numbers, don't send empty SplitData pacckets, some extra comments, use correct packet ids in error messages * Simplify packet splitting to single payload type, remove unnecessary metadata. * Replace finished flag with just appended packet length, add extra check for validating packet type * Correct name of genericPacketSplitter method * Fix not full rename oops * Apply player's suggestion (cherry picked from commit 57cb96d) (cherry picked from commit b095d8d)

(cherry picked from commit 19a99b0)

(cherry picked from commit 3706cdf)

(cherry picked from commit 68e881b)

* Add the Dimensions Modifications API * Fix test cloud color * Simplify dimensions modifications using events * Start the test server at noon * PR feedback * Remove logging * Switch to client game test for dimensions modifications * Remove unused logger from DimensionModificationImpl * Remove unused logger imports from DimensionModificationImpl

…abricMC#5129) * Create EventResult * Migrate EntitySleepEvents from InteractionResult to EventResult.

* Add advancement renderers * Fix import ordering * Move context classes into inner classes * Add missing renderer javadocs * Rename accessor methods * Remove accesswidener * Fix renderer registration check and exceptions * Use named locals for args

@APinote

* feat: Debug API * docs(EntityDebugSubscriptionRegistry): clarify it's server-side only * chore: apply licenses * docs: `Constructor` -> `Factory` terminology on factory interfaces * fix: depend `fabricloader` `>=0.18.4` * refactor: merge `MobMixin` & `EntityMixin` * use @APinote in javadoc and use package-private Mixins Co-authored-by: Juuz <6596629+Juuxel@users.noreply.github.com> * refactor!: `net.fabricmc.fabric.api.debug.v1.client` -> `….api.client.debug.v1` --------- Co-authored-by: Juuz <6596629+Juuxel@users.noreply.github.com>

* change att sync packet to only handle one attachment * add large attachments * fix gametest failure due to persistence problem * woops fix int length from testing * sync gametest logger name

* Separately flush UI elements with connected primitives * Update test * spotlessApply

* Use local variable names for all mixin locals * Add checkstyle rule + more fixes * More fixes * Renames * Cleanup * Prevent using name for args only locals --------- Co-authored-by: joe <burtonjae@hotmail.co.uk>

…ricMC#5162)

…s to fix crash (FabricMC#5176)

…faces (FabricMC#5177) * Add transitive access wideners for Enchantment functional interfaces * Revert "Add transitive access wideners for Enchantment functional interfaces" This reverts commit e7e5099. * Modify template generation instead of file directly

…abricMC#5178) * deferred sync for BEs * fix imports * avoid extra deferred sync during initial load * checkstyle * simplify sending deferred changes * spotlessApply * Avoid more unnecessary deferred syncing * add test for BE deferred sync * sync deferred removals too * avoid bundling packets if only one change

# 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

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

modmuss50 and others added 22 commits October 2, 2025 15:18

1.21.10-rc1

9983246

1.21.10

43cd8b7

Improve access to resource reloaders profiling. (FabricMC#4913)

8cc86c1

Fix LivingEntityFeatureRenderEvents.ALLOW_CAPE_RENDER (FabricMC#4912)

fde3702

* Fix LivingEntityFeatureRenderEvents.ALLOW_CAPE_RENDER * Make test better

Improve debug entry sorting and allow searching by namespace. (Fabric…

91152d8

…MC#4911) * Sort debug entries * Fix sorting logic * Also search by namespace

Add a conventional item tag for copper nuggets (FabricMC#4905)

552403e

Implement item FRAPI in Indigo (FabricMC#4904)

d7df915

Add refreshCommandCompletions() to ClientCommandManager (FabricMC#4893)

c0ab2d5

* Add refreshCommandCompletions method to fcapi * Implement requested changes * Add some comments and feedback

Bump version

b3db480

25w41a

65ffd04

Update to Minecraft snapshot 25w42a (FabricMC#4930)

bf431ac

[1.21.11] Conventional item tags for armor (FabricMC#4918)

a54a246

* Conventional armor tags * fill new tags

Add a checkstyle rule for nested code blocks (FabricMC#4925)

f6b7d89

Remove all @NotNull annotations (FabricMC#4924)

1af5c5a

* Remove all @NotNull annotations * Missed one

Bump version

62564a1

Add getDataOrDefault method to FabricRenderState (FabricMC#4917)

8a5e531

(cherry picked from commit 19a99b0)

Fix active renderer category (FabricMC#4928)

82066aa

(cherry picked from commit 3706cdf)

Update AttachmentSync.java (FabricMC#4921)

30aa58d

(cherry picked from commit 68e881b)

Bump version

f083603

25w43a

35c69dd

25w44a

8a1eabd

Hugman76 and others added 21 commits January 29, 2026 13:57

Create EventResult enum, an simpler alternative to InteractionResult (F…

ece0632

…abricMC#5129) * Create EventResult * Migrate EntitySleepEvents from InteractionResult to EventResult.

Add large attachment syncing (FabricMC#5143)

f6128de

* change att sync packet to only handle one attachment * add large attachments * fix gametest failure due to persistence problem * woops fix int length from testing * sync gametest logger name

Separately flush UI elements with connected primitives (FabricMC#5157)

76eeb7f

* Separately flush UI elements with connected primitives * Update test * spotlessApply

Mixin cleanup part 2 (FabricMC#5160)

c82f046

* Use local variable names for all mixin locals * Add checkstyle rule + more fixes * More fixes * Renames * Cleanup * Prevent using name for args only locals --------- Co-authored-by: joe <burtonjae@hotmail.co.uk>

defer calling setChanged() if BE's chunk is not fully loaded yet (Fab…

51ad026

…ricMC#5162)

Suggest a max line length of 100 chars (FabricMC#5163)

8651697

Update names, replace getIdOrThrow with access widener. (FabricMC#5169)

18b981c

Fix build

35dc1c2

Bump version

da59c67

make lookup api context extend nullable object (FabricMC#5172)

834cb82

Add fabric-registry-sync-v0 to fabric-gametest-api-v1 dependencie…

15326d6

…s to fix crash (FabricMC#5176)

Bump version

efb2898

26.1-snapshot-6

00a1fba

Bump version

45071d9

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


		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!

20 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