From 2745531f08bf159e1346f1b69cbe757ed7e8d67b Mon Sep 17 00:00:00 2001 From: LexManos Date: Wed, 19 Jul 2023 10:21:05 -0700 Subject: [PATCH] Fix tag removal functionality that broke during the 1.19 update. Closes #9053 and #8949 --- .../net/minecraft/core/HolderSet.java.patch | 2 +- .../data/tags/TagsProvider.java.patch | 10 ++-- .../net/minecraft/tags/TagFile.java.patch | 17 +++++++ .../net/minecraft/tags/TagLoader.java.patch | 39 ++++++++++++++ .../data/minecraft/tags/blocks/test_tag.json | 11 ++++ .../minecraft/tags/items/music_discs.json | 6 +++ .../data/minecraft/tags/items/planks.json | 6 +++ .../net/minecraftforge/common/ForgeHooks.java | 18 +------ .../IForgeIntrinsicHolderTagAppender.java | 38 +++++--------- .../extensions/IForgeRawTagBuilder.java | 35 +++---------- .../debug/RemoveTagDatagenTest.java | 51 ++++++++++++++----- 11 files changed, 146 insertions(+), 87 deletions(-) create mode 100644 patches/minecraft/net/minecraft/tags/TagFile.java.patch create mode 100644 src/generated_test/resources/data/minecraft/tags/items/music_discs.json create mode 100644 src/generated_test/resources/data/minecraft/tags/items/planks.json diff --git a/patches/minecraft/net/minecraft/core/HolderSet.java.patch b/patches/minecraft/net/minecraft/core/HolderSet.java.patch index 67e5ab6fff..9d91acd7c6 100644 --- a/patches/minecraft/net/minecraft/core/HolderSet.java.patch +++ b/patches/minecraft/net/minecraft/core/HolderSet.java.patch @@ -24,7 +24,7 @@ public boolean m_207277_(HolderOwner p_256542_) { return this.f_254711_.m_254921_(p_256542_); + } -+ // FORGE: Keep a list of invalidation callbacks so they can be run when tags rebind ++ // FORGE: Keep a list of invalidation callbacks so they can be run when tags rebind + private List invalidationCallbacks = new java.util.ArrayList<>(); + public void addInvalidationListener(Runnable runnable) { + invalidationCallbacks.add(runnable); diff --git a/patches/minecraft/net/minecraft/data/tags/TagsProvider.java.patch b/patches/minecraft/net/minecraft/data/tags/TagsProvider.java.patch index a3a5a174eb..4f5ce5ecd8 100644 --- a/patches/minecraft/net/minecraft/data/tags/TagsProvider.java.patch +++ b/patches/minecraft/net/minecraft/data/tags/TagsProvider.java.patch @@ -66,9 +66,12 @@ Predicate predicate = (p_255496_) -> { return registrylookup.m_254902_(ResourceKey.m_135785_(this.f_254716_, p_255496_)).isPresent(); }; -@@ -77,12 +_,13 @@ +@@ -75,14 +_,16 @@ + ResourceLocation resourcelocation = p_255499_.getKey(); + TagBuilder tagbuilder = p_255499_.getValue(); List list = tagbuilder.m_215904_(); - List list1 = list.stream().filter((p_274771_) -> { +- List list1 = list.stream().filter((p_274771_) -> { ++ List list1 = java.util.stream.Stream.concat(list.stream(), tagbuilder.getRemoveEntries()).filter((p_274771_) -> { return !p_274771_.m_215940_(predicate, predicate1); - }).toList(); + }).filter(this::missing).toList(); // Forge: Add validation via existing resources @@ -77,7 +80,8 @@ } else { - JsonElement jsonelement = TagFile.f_215958_.encodeStart(JsonOps.INSTANCE, new TagFile(list, false)).getOrThrow(false, f_126541_::error); - Path path = this.f_236439_.m_245731_(resourcelocation); -+ JsonElement jsonelement = TagFile.f_215958_.encodeStart(JsonOps.INSTANCE, new TagFile(list, tagbuilder.isReplace())).getOrThrow(false, f_126541_::error); ++ var removed = tagbuilder.getRemoveEntries().toList(); ++ JsonElement jsonelement = TagFile.f_215958_.encodeStart(JsonOps.INSTANCE, new TagFile(list, tagbuilder.isReplace(), removed)).getOrThrow(false, f_126541_::error); + Path path = this.getPath(resourcelocation); + if (path == null) return CompletableFuture.completedFuture(null); // Forge: Allow running this data provider without writing it. Recipe provider needs valid tags. return DataProvider.m_253162_(p_253684_, jsonelement, path); diff --git a/patches/minecraft/net/minecraft/tags/TagFile.java.patch b/patches/minecraft/net/minecraft/tags/TagFile.java.patch new file mode 100644 index 0000000000..fdaf24d1ec --- /dev/null +++ b/patches/minecraft/net/minecraft/tags/TagFile.java.patch @@ -0,0 +1,17 @@ +--- a/net/minecraft/tags/TagFile.java ++++ b/net/minecraft/tags/TagFile.java +@@ -4,8 +_,12 @@ + import com.mojang.serialization.codecs.RecordCodecBuilder; + import java.util.List; + +-public record TagFile(List f_215959_, boolean f_215960_) { ++public record TagFile(List f_215959_, boolean f_215960_, List remove) { + public static final Codec f_215958_ = RecordCodecBuilder.create((p_215967_) -> { +- return p_215967_.group(TagEntry.f_215911_.listOf().fieldOf("values").forGetter(TagFile::f_215959_), Codec.BOOL.optionalFieldOf("replace", Boolean.valueOf(false)).forGetter(TagFile::f_215960_)).apply(p_215967_, TagFile::new); ++ return p_215967_.group(TagEntry.f_215911_.listOf().fieldOf("values").forGetter(TagFile::f_215959_), Codec.BOOL.optionalFieldOf("replace", Boolean.valueOf(false)).forGetter(TagFile::f_215960_), TagEntry.f_215911_.listOf().optionalFieldOf("remove", List.of()).forGetter(TagFile::remove)).apply(p_215967_, TagFile::new); + }); ++ ++ public TagFile(List f_215959_, boolean f_215960_) { ++ this(f_215959_, f_215960_, List.of()); ++ } + } diff --git a/patches/minecraft/net/minecraft/tags/TagLoader.java.patch b/patches/minecraft/net/minecraft/tags/TagLoader.java.patch index 3744d3f62e..4be770aefc 100644 --- a/patches/minecraft/net/minecraft/tags/TagLoader.java.patch +++ b/patches/minecraft/net/minecraft/tags/TagLoader.java.patch @@ -1,5 +1,34 @@ --- a/net/minecraft/tags/TagLoader.java +++ b/net/minecraft/tags/TagLoader.java +@@ -59,6 +_,7 @@ + tagfile.f_215959_().forEach((p_215997_) -> { + list.add(new TagLoader.EntryWithSource(p_215997_, s)); + }); ++ tagfile.remove().forEach(e -> list.add(new TagLoader.EntryWithSource(e, s, true))); + } catch (Exception exception) { + f_13445_.error("Couldn't read tag list {} from {} in data pack {}", resourcelocation1, resourcelocation, resource.m_215506_(), exception); + } +@@ -69,16 +_,17 @@ + } + + private Either, Collection> m_215978_(TagEntry.Lookup p_215979_, List p_215980_) { +- ImmutableSet.Builder builder = ImmutableSet.builder(); ++ var builder = new java.util.HashSet(); + List list = new ArrayList<>(); + + for(TagLoader.EntryWithSource tagloader$entrywithsource : p_215980_) { +- if (!tagloader$entrywithsource.f_216042_().m_215927_(p_215979_, builder::add)) { ++ if (!tagloader$entrywithsource.f_216042_().m_215927_(p_215979_, tagloader$entrywithsource.remove() ? builder::remove : builder::add)) { ++ if (!tagloader$entrywithsource.remove()) // Treat all removals as optional at runtime. If it was missing, then it could of never been added. + list.add(tagloader$entrywithsource); + } + } + +- return list.isEmpty() ? Either.right(builder.build()) : Either.left(list); ++ return list.isEmpty() ? Either.right(List.copyOf(builder)) : Either.left(list); + } + + public Map> m_203898_(Map> p_203899_) { @@ -100,7 +_,7 @@ }); dependencysorter.m_284430_((p_284682_, p_284683_) -> { @@ -9,3 +38,13 @@ }).ifRight((p_216001_) -> { map.put(p_284682_, p_216001_); }); +@@ -112,7 +_,8 @@ + return this.m_203898_(this.m_144495_(p_203901_)); + } + +- public static record EntryWithSource(TagEntry f_216042_, String f_216043_) { ++ public static record EntryWithSource(TagEntry f_216042_, String f_216043_, boolean remove) { ++ public EntryWithSource(TagEntry f_216042_, String f_216043_) { this(f_216042_, f_216043_, false); } + public String toString() { + return this.f_216042_ + " (from " + this.f_216043_ + ")"; + } diff --git a/src/generated_test/resources/data/minecraft/tags/blocks/test_tag.json b/src/generated_test/resources/data/minecraft/tags/blocks/test_tag.json index f72d209df7..cfe909e22b 100644 --- a/src/generated_test/resources/data/minecraft/tags/blocks/test_tag.json +++ b/src/generated_test/resources/data/minecraft/tags/blocks/test_tag.json @@ -1,3 +1,14 @@ { + "remove": [ + "minecraft:dirt", + "minecraft:oak_door", + "minecraft:dark_oak_door", + "minecraft:anvil", + "minecraft:basalt", + "minecraft:polished_andesite", + "#minecraft:beehives", + "#minecraft:banners", + "#minecraft:beds" + ], "values": [] } \ No newline at end of file diff --git a/src/generated_test/resources/data/minecraft/tags/items/music_discs.json b/src/generated_test/resources/data/minecraft/tags/items/music_discs.json new file mode 100644 index 0000000000..cc0935b930 --- /dev/null +++ b/src/generated_test/resources/data/minecraft/tags/items/music_discs.json @@ -0,0 +1,6 @@ +{ + "remove": [ + "minecraft:music_disc_cat" + ], + "values": [] +} \ No newline at end of file diff --git a/src/generated_test/resources/data/minecraft/tags/items/planks.json b/src/generated_test/resources/data/minecraft/tags/items/planks.json new file mode 100644 index 0000000000..0af77b3df5 --- /dev/null +++ b/src/generated_test/resources/data/minecraft/tags/items/planks.json @@ -0,0 +1,6 @@ +{ + "remove": [ + "minecraft:spruce_planks" + ], + "values": [] +} \ No newline at end of file diff --git a/src/main/java/net/minecraftforge/common/ForgeHooks.java b/src/main/java/net/minecraftforge/common/ForgeHooks.java index bf2e8a6c56..6d2c26f908 100644 --- a/src/main/java/net/minecraftforge/common/ForgeHooks.java +++ b/src/main/java/net/minecraftforge/common/ForgeHooks.java @@ -1081,22 +1081,8 @@ public class ForgeHooks return false; } - public static void deserializeTagAdditions(List list, JsonObject json, List allList) - { - if (json.has("remove")) - { - for (JsonElement entry : GsonHelper.getAsJsonArray(json, "remove")) - { - String s = GsonHelper.convertToString(entry, "value"); - TagEntry dummy; - if (!s.startsWith("#")) - dummy = TagEntry.optionalElement(new ResourceLocation(s)); - else - dummy = TagEntry.tag(new ResourceLocation(s.substring(1))); - allList.removeIf(e -> e.equals(dummy)); - } - } - } + @Deprecated(forRemoval = true, since = "1.20.1") // Tags use a codec now This was never used in 1.20 + public static void deserializeTagAdditions(List list, JsonObject json, List allList) {} @Nullable public static EntityDataSerializer getSerializer(int id, CrudeIncrementalIntIdentityHashBiMap> vanilla) diff --git a/src/main/java/net/minecraftforge/common/extensions/IForgeIntrinsicHolderTagAppender.java b/src/main/java/net/minecraftforge/common/extensions/IForgeIntrinsicHolderTagAppender.java index e38de07534..f4eaf2139d 100644 --- a/src/main/java/net/minecraftforge/common/extensions/IForgeIntrinsicHolderTagAppender.java +++ b/src/main/java/net/minecraftforge/common/extensions/IForgeIntrinsicHolderTagAppender.java @@ -10,8 +10,7 @@ import net.minecraft.resources.ResourceKey; import net.minecraft.resources.ResourceLocation; import net.minecraft.tags.TagKey; -public interface IForgeIntrinsicHolderTagAppender extends IForgeTagAppender -{ +public interface IForgeIntrinsicHolderTagAppender extends IForgeTagAppender { private IntrinsicHolderTagsProvider.IntrinsicTagAppender self() { return (IntrinsicHolderTagsProvider.IntrinsicTagAppender) this; } @@ -24,8 +23,7 @@ public interface IForgeIntrinsicHolderTagAppender extends IForgeTagAppender remove(final T entry) - { + default IntrinsicHolderTagsProvider.IntrinsicTagAppender remove(final T entry) { return remove(this.getKey(entry)); } @@ -36,78 +34,66 @@ public interface IForgeIntrinsicHolderTagAppender extends IForgeTagAppender remove(final T first, final T...entries) - { + default IntrinsicHolderTagsProvider.IntrinsicTagAppender remove(final T first, final T...entries) { this.remove(first); for (T entry : entries) - { this.remove(entry); - } return self(); } @Override @SuppressWarnings("unchecked") - default IntrinsicHolderTagsProvider.IntrinsicTagAppender addTags(TagKey... values) - { + default IntrinsicHolderTagsProvider.IntrinsicTagAppender addTags(TagKey... values) { IForgeTagAppender.super.addTags(values); return self(); } @Override - default IntrinsicHolderTagsProvider.IntrinsicTagAppender replace() - { + default IntrinsicHolderTagsProvider.IntrinsicTagAppender replace() { IForgeTagAppender.super.replace(); return self(); } @Override - default IntrinsicHolderTagsProvider.IntrinsicTagAppender replace(boolean value) - { + default IntrinsicHolderTagsProvider.IntrinsicTagAppender replace(boolean value) { IForgeTagAppender.super.replace(value); return self(); } @Override - default IntrinsicHolderTagsProvider.IntrinsicTagAppender remove(final ResourceLocation location) - { + default IntrinsicHolderTagsProvider.IntrinsicTagAppender remove(final ResourceLocation location) { IForgeTagAppender.super.remove(location); return self(); } @Override - default IntrinsicHolderTagsProvider.IntrinsicTagAppender remove(final ResourceLocation first, final ResourceLocation... locations) - { + default IntrinsicHolderTagsProvider.IntrinsicTagAppender remove(final ResourceLocation first, final ResourceLocation... locations) { IForgeTagAppender.super.remove(first, locations); return self(); } @Override - default IntrinsicHolderTagsProvider.IntrinsicTagAppender remove(final ResourceKey resourceKey) - { + default IntrinsicHolderTagsProvider.IntrinsicTagAppender remove(final ResourceKey resourceKey) { IForgeTagAppender.super.remove(resourceKey); return self(); } @Override @SuppressWarnings("unchecked") - default IntrinsicHolderTagsProvider.IntrinsicTagAppender remove(final ResourceKey firstResourceKey, final ResourceKey... resourceKeys) - { + default IntrinsicHolderTagsProvider.IntrinsicTagAppender remove(final ResourceKey firstResourceKey, final ResourceKey... resourceKeys) { IForgeTagAppender.super.remove(firstResourceKey, resourceKeys); return self(); } @Override - default IntrinsicHolderTagsProvider.IntrinsicTagAppender remove(TagKey tag) - { + default IntrinsicHolderTagsProvider.IntrinsicTagAppender remove(TagKey tag) { IForgeTagAppender.super.remove(tag); return self(); } @Override @SuppressWarnings("unchecked") - default IntrinsicHolderTagsProvider.IntrinsicTagAppender remove(TagKey first, TagKey...tags) - { + default IntrinsicHolderTagsProvider.IntrinsicTagAppender remove(TagKey first, TagKey...tags) { IForgeTagAppender.super.remove(first, tags); return self(); } diff --git a/src/main/java/net/minecraftforge/common/extensions/IForgeRawTagBuilder.java b/src/main/java/net/minecraftforge/common/extensions/IForgeRawTagBuilder.java index 3fddfbb213..e43669f8d3 100644 --- a/src/main/java/net/minecraftforge/common/extensions/IForgeRawTagBuilder.java +++ b/src/main/java/net/minecraftforge/common/extensions/IForgeRawTagBuilder.java @@ -5,37 +5,22 @@ package net.minecraftforge.common.extensions; -import java.util.stream.Stream; - -import com.google.gson.JsonArray; import com.google.gson.JsonObject; import net.minecraft.resources.ResourceLocation; import net.minecraft.tags.TagBuilder; import net.minecraft.tags.TagEntry; -public interface IForgeRawTagBuilder -{ - default TagBuilder getRawBuilder() - { +public interface IForgeRawTagBuilder { + default TagBuilder getRawBuilder() { return (TagBuilder)this; } /** - * internal, called when a raw builder is written to json to add forge additions (e.g. the remove list) + * @deprecated Never used, tags use a Codec now, so remove this later. */ - default void serializeTagAdditions(final JsonObject tagJson) - { - TagBuilder rawBuilder = this.getRawBuilder(); - Stream removeEntries = rawBuilder.getRemoveEntries(); - JsonArray removeEntriesAsJsonArray = new JsonArray(); - //removeEntries.forEach(proxy ->proxy.serializeTo(removeEntriesAsJsonArray)); // TODO: Figure out what the replacement for this is. -C - if (removeEntriesAsJsonArray.size() > 0) - { - tagJson.add("remove", removeEntriesAsJsonArray); - } - - } + @Deprecated(forRemoval = true, since = "1.20.1") + default void serializeTagAdditions(final JsonObject tagJson) {} /** * Adds a tag entry to the remove list. @@ -43,8 +28,7 @@ public interface IForgeRawTagBuilder * @param source The source of the caller for logging purposes (generally a modid) * @return The builder for chaining purposes */ - default TagBuilder remove(final TagEntry tagEntry, final String source) - { + default TagBuilder remove(final TagEntry tagEntry, final String source) { return this.getRawBuilder().remove(tagEntry); } @@ -54,20 +38,17 @@ public interface IForgeRawTagBuilder * @param source The source of the caller for logging purposes (generally a modid) * @return The builder for chaining purposes */ - default TagBuilder removeElement(final ResourceLocation elementID, final String source) - { + default TagBuilder removeElement(final ResourceLocation elementID, final String source) { return this.remove(TagEntry.element(elementID), source); } - /** * Adds a tag to the remove list. * @param tagID The ID of the tag to add to the remove list * @param source The source of the caller for logging purposes (generally a modid) * @return The builder for chaining purposes */ - default TagBuilder removeTag(final ResourceLocation tagID, final String source) - { + default TagBuilder removeTag(final ResourceLocation tagID, final String source) { return this.remove(TagEntry.tag(tagID), source); } } diff --git a/src/test/java/net/minecraftforge/debug/RemoveTagDatagenTest.java b/src/test/java/net/minecraftforge/debug/RemoveTagDatagenTest.java index 908a4860bd..ae3d76b7d0 100644 --- a/src/test/java/net/minecraftforge/debug/RemoveTagDatagenTest.java +++ b/src/test/java/net/minecraftforge/debug/RemoveTagDatagenTest.java @@ -7,9 +7,13 @@ package net.minecraftforge.debug; import net.minecraft.core.HolderLookup; import net.minecraft.data.DataGenerator; +import net.minecraft.data.tags.ItemTagsProvider; import net.minecraft.resources.ResourceLocation; import net.minecraft.tags.BlockTags; +import net.minecraft.tags.ItemTags; import net.minecraft.tags.TagKey; +import net.minecraft.world.item.Item; +import net.minecraft.world.item.Items; import net.minecraft.world.level.block.Block; import net.minecraft.world.level.block.Blocks; import net.minecraftforge.common.data.BlockTagsProvider; @@ -21,36 +25,55 @@ import net.minecraftforge.data.event.GatherDataEvent; import net.minecraftforge.registries.ForgeRegistries; @Mod(RemoveTagDatagenTest.MODID) -public class RemoveTagDatagenTest -{ +public class RemoveTagDatagenTest { + public static final String MODID = "remove_tag_datagen_test"; public static final TagKey TEST_TAG = BlockTags.create(new ResourceLocation("test_tag")); - public RemoveTagDatagenTest() - { + public RemoveTagDatagenTest() { IEventBus modBus = FMLJavaModLoadingContext.get().getModEventBus(); modBus.addListener(this::onGatherData); } - private void onGatherData(GatherDataEvent event) - { + private void onGatherData(GatherDataEvent event) { DataGenerator generator = event.getGenerator(); ExistingFileHelper helper = event.getExistingFileHelper(); - generator.addProvider(true, new BlockTagsProvider(generator.getPackOutput(), event.getLookupProvider(), MODID, helper) - { + var blocks = new BlockTagsProvider(generator.getPackOutput(), event.getLookupProvider(), MODID, helper) { @SuppressWarnings("unchecked") @Override - protected void addTags(HolderLookup.Provider provider) - { + protected void addTags(HolderLookup.Provider provider) { this.tag(TEST_TAG) - .remove(ForgeRegistries.BLOCKS.getKey(Blocks.DIRT)) - .remove(ForgeRegistries.BLOCKS.getKey(Blocks.OAK_DOOR), ForgeRegistries.BLOCKS.getKey(Blocks.DARK_OAK_DOOR)) - .remove(ForgeRegistries.BLOCKS.getKey(Blocks.ANVIL)) - .remove(ForgeRegistries.BLOCKS.getKey(Blocks.BASALT), ForgeRegistries.BLOCKS.getKey(Blocks.POLISHED_ANDESITE)) + .remove(key(Blocks.DIRT)) + .remove(key(Blocks.OAK_DOOR), key(Blocks.DARK_OAK_DOOR)) + .remove(key(Blocks.ANVIL)) + .remove(key(Blocks.BASALT), key(Blocks.POLISHED_ANDESITE)) .remove(BlockTags.BEEHIVES) .remove(BlockTags.BANNERS, BlockTags.BEDS); } + }; + + generator.addProvider(event.includeServer(), blocks); + + generator.addProvider(event.includeServer(), new ItemTagsProvider(generator.getPackOutput(), event.getLookupProvider(), blocks.contentsGetter(), MODID, helper) { + @Override + protected void addTags(HolderLookup.Provider provider) { + // This is for testing if it is functional, remove spruce_planks from planks, which makes us unable to craft beds with them. + this.tag(ItemTags.PLANKS).remove(key(Blocks.SPRUCE_PLANKS)); + // This is for testing deep values, removing a entry in a tag that is referenced by another tag + // Remove MUSIC_DISC_CAT from the MUSIC_DISCS tag, which is added by MUSIC_DISCS reference to the CREEPER_DROP_MUSIC_DISCS tag + // This will make MUSIC_DISK_CAT unable to be played in a jukebox. + // It will still remove the record from the player because RecordItem just blindly deletes, but it won't play music. + this.tag(ItemTags.MUSIC_DISCS).remove(key(Items.MUSIC_DISC_CAT)); + } }); } + + private static ResourceLocation key(Block value) { + return ForgeRegistries.BLOCKS.getKey(value); + } + + private static ResourceLocation key(Item value) { + return ForgeRegistries.ITEMS.getKey(value); + } }