From d498ec4a4580d6a806cbaea071258b74af1ebeb7 Mon Sep 17 00:00:00 2001 From: LexManos Date: Wed, 10 Jul 2024 11:46:23 -0700 Subject: [PATCH] Fix Global Loot Modifier deserialization not having registry access. Closes #10042 and #10043 --- .../resources/HolderSetCodec.java.patch | 9 +++ .../common/ForgeInternalHandler.java | 2 +- .../common/loot/LootModifierManager.java | 12 ++- .../loot/GlobalLootModifiersTest.java | 76 ++++++++++++++----- 4 files changed, 76 insertions(+), 23 deletions(-) diff --git a/patches/minecraft/net/minecraft/resources/HolderSetCodec.java.patch b/patches/minecraft/net/minecraft/resources/HolderSetCodec.java.patch index c459573e61..aa7b8bb364 100644 --- a/patches/minecraft/net/minecraft/resources/HolderSetCodec.java.patch +++ b/patches/minecraft/net/minecraft/resources/HolderSetCodec.java.patch @@ -39,6 +39,15 @@ ); return dataresult.map(p_326149_ -> Pair.of((HolderSet)p_326149_, (T)p_326147_.getSecond())); } +@@ -90,7 +_,7 @@ + } + + private DataResult, T>> decodeWithoutRegistry(DynamicOps p_206671_, T p_206672_) { +- return this.elementCodec.listOf().decode(p_206671_, p_206672_).flatMap(p_206666_ -> { ++ return this.homogenousListCodec.decode(p_206671_, p_206672_).flatMap(p_206666_ -> { // Forge: Match encodeWithoutRegistry's use of the homogenousListCodec + List> list = new ArrayList<>(); + + for (Holder holder : p_206666_.getFirst()) { @@ -106,6 +_,9 @@ } diff --git a/src/main/java/net/minecraftforge/common/ForgeInternalHandler.java b/src/main/java/net/minecraftforge/common/ForgeInternalHandler.java index 89c22a0895..ebf7a241b1 100644 --- a/src/main/java/net/minecraftforge/common/ForgeInternalHandler.java +++ b/src/main/java/net/minecraftforge/common/ForgeInternalHandler.java @@ -101,7 +101,7 @@ public class ForgeInternalHandler { @SubscribeEvent public void onResourceReload(AddReloadListenerEvent event) { - INSTANCE = new LootModifierManager(); + INSTANCE = new LootModifierManager(event.getRegistryAccess()); event.addListener(INSTANCE); } diff --git a/src/main/java/net/minecraftforge/common/loot/LootModifierManager.java b/src/main/java/net/minecraftforge/common/loot/LootModifierManager.java index 30cb3f9ed9..619b69b675 100644 --- a/src/main/java/net/minecraftforge/common/loot/LootModifierManager.java +++ b/src/main/java/net/minecraftforge/common/loot/LootModifierManager.java @@ -23,6 +23,7 @@ import net.minecraft.server.packs.resources.SimpleJsonResourceReloadListener; import net.minecraft.util.profiling.ProfilerFiller; import net.minecraft.server.packs.resources.ResourceManager; import net.minecraft.util.GsonHelper; +import net.minecraft.core.RegistryAccess; import net.minecraft.resources.ResourceLocation; public class LootModifierManager extends SimpleJsonResourceReloadListener { @@ -30,10 +31,12 @@ public class LootModifierManager extends SimpleJsonResourceReloadListener { private static final Gson GSON = new GsonBuilder().setPrettyPrinting().disableHtmlEscaping().create(); private static final String FOLDER = "loot_modifiers"; + private final RegistryAccess registries; private Map modifiers = ImmutableMap.of(); - public LootModifierManager() { + public LootModifierManager(RegistryAccess registries) { super(GSON, FOLDER); + this.registries = registries; } @Override @@ -68,12 +71,13 @@ public class LootModifierManager extends SimpleJsonResourceReloadListener { @Override protected void apply(Map resources, ResourceManager resourceManagerIn, ProfilerFiller profilerIn) { Builder builder = ImmutableMap.builder(); + var ops = registries.createSerializationContext(JsonOps.INSTANCE); resources.forEach((location, json) -> { - IGlobalLootModifier.DIRECT_CODEC.parse(JsonOps.INSTANCE, json) + IGlobalLootModifier.DIRECT_CODEC.parse(ops, json) // log error if parse fails - .resultOrPartial(errorMsg -> LOGGER.warn("Could not decode GlobalLootModifier with json id {} - error: {}", location, errorMsg)) + .ifError(error -> LOGGER.warn("Could not decode GlobalLootModifier with json id {} - error: {}", location, error.message())) // add loot modifier if parse succeeds - .ifPresent(modifier -> builder.put(location, modifier)); + .ifSuccess(modifier -> builder.put(location, modifier)); }); this.modifiers = builder.build(); } diff --git a/src/test/java/net/minecraftforge/debug/gameplay/loot/GlobalLootModifiersTest.java b/src/test/java/net/minecraftforge/debug/gameplay/loot/GlobalLootModifiersTest.java index 5c4d5b8f0c..8779d8bac7 100644 --- a/src/test/java/net/minecraftforge/debug/gameplay/loot/GlobalLootModifiersTest.java +++ b/src/test/java/net/minecraftforge/debug/gameplay/loot/GlobalLootModifiersTest.java @@ -106,15 +106,18 @@ public class GlobalLootModifiersTest extends BaseTestMod { private static final RegistryObject TEST_ITEM = ITEMS.register("test", () -> new BlockItem(TEST_BLOCK.get(), new Item.Properties())); public GlobalLootModifiersTest() { - testItem(lookup -> getTestItem(lookup.lookup(Registries.ENCHANTMENT).get())); + testItem(lookup -> getSmelterAxe(lookup.lookup(Registries.ENCHANTMENT).get(), true)); } - private static ItemStack getTestItem(HolderLookup lookup) { - var smelt = new ItemStack(Items.IRON_AXE); - var enchants = new ItemEnchantments.Mutable(EnchantmentHelper.getEnchantmentsForCrafting(smelt)); + private static ItemStack getSmelterAxe(HolderLookup lookup, boolean enchanted) { + var item = new ItemStack(Items.IRON_AXE); + if (!enchanted) + return item; + + var enchants = new ItemEnchantments.Mutable(EnchantmentHelper.getEnchantmentsForCrafting(item)); enchants.set(lookup.getOrThrow(SMELT), 1); - smelt.set(DataComponents.ENCHANTMENTS, enchants.toImmutable()); - return smelt; + item.set(DataComponents.ENCHANTMENTS, enchants.toImmutable()); + return item; } @SubscribeEvent @@ -128,24 +131,36 @@ public class GlobalLootModifiersTest extends BaseTestMod { event.getGenerator().addProvider(event.includeServer(), new LootProvider(out, lookup)); } + // Tests the Enchantment condition, as well as the ability to completely override the returned values. @GameTest(template = "forge:empty3x3x3") - public static void test_smelting_modifier(GameTestHelper helper) { - var center = new BlockPos(1, 1, 1); - helper.setBlock(center, Blocks.OAK_LOG); - - var smelt = getTestItem(helper.getLevel().holderLookup(Registries.ENCHANTMENT)); - + public static void smellting(GameTestHelper helper) { var player = helper.makeMockServerPlayer(); - player.setItemSlot(EquipmentSlot.MAINHAND, smelt); - player.gameMode.destroyBlock(helper.absolutePos(center)); + var center = new BlockPos(1, 1, 1); + var enchants = helper.getLevel().holderLookup(Registries.ENCHANTMENT); + var smelt = getSmelterAxe(enchants, true); + var normal = getSmelterAxe(enchants, false); + // Harvesting with normal axe should just dorp log + player.setItemSlot(EquipmentSlot.MAINHAND, normal); + helper.setBlock(center, Blocks.OAK_LOG); + player.gameMode.destroyBlock(helper.absolutePos(center)); + helper.assertItemEntityPresent(Items.OAK_LOG, center, 1.0); + + //Reset + helper.killAllEntities(); + + // Test smelting enchantment, should result in charcoal + player.setItemSlot(EquipmentSlot.MAINHAND, smelt); + helper.setBlock(center, Blocks.OAK_LOG); + player.gameMode.destroyBlock(helper.absolutePos(center)); helper.assertItemEntityPresent(Items.CHARCOAL, center, 1.0); helper.succeed(); } + // Tests the table name condition by duplicating the drops for our test block, but not for anything else. @GameTest(template = "forge:empty3x3x3") - public static void table_name_condition(GameTestHelper helper) { + public static void condition_table_name(GameTestHelper helper) { var center = new BlockPos(1, 1, 1); var player = helper.makeMockServerPlayer(); @@ -162,6 +177,29 @@ public class GlobalLootModifiersTest extends BaseTestMod { helper.succeed(); } + + @GameTest(template = "forge:empty3x3x3") + public static void silk_reentrant(GameTestHelper helper) { + var center = new BlockPos(1, 1, 1); + var player = helper.makeMockServerPlayer(); + var bamboo = new ItemStack(Items.BAMBOO); + var normal = new ItemStack(Items.IRON_AXE); + + // Should drop nothing + helper.setBlock(center, Blocks.GLASS); + player.setItemSlot(EquipmentSlot.MAINHAND, normal); + player.gameMode.destroyBlock(helper.absolutePos(center)); + helper.assertItemEntityNotPresent(Items.GLASS, center, 1.0); + + // Should drop the glass + helper.setBlock(center, Blocks.GLASS); + player.setItemSlot(EquipmentSlot.MAINHAND, bamboo); + player.gameMode.destroyBlock(helper.absolutePos(center)); + helper.assertItemEntityPresent(Items.GLASS, center, 1.0); + + helper.succeed(); + } + private static class ModifierProvider extends GlobalLootModifierProvider { public ModifierProvider(PackOutput output, String modid, CompletableFuture registries) { super(output, modid, registries); @@ -171,6 +209,7 @@ public class GlobalLootModifiersTest extends BaseTestMod { protected void start(HolderLookup.Provider registries) { var smelt = registries.lookupOrThrow(Registries.ENCHANTMENT).getOrThrow(SMELT); + // Tested vis breaking a log and checking if we get charcoal add("smelting", new SmeltingEnchantmentModifier( new LootItemCondition[]{ MatchTool.toolMatches( @@ -185,6 +224,8 @@ public class GlobalLootModifiersTest extends BaseTestMod { }) ); + + // This has no game test because it relies on random number generation and I can't be bothered to try and force that right now. add("wheat_harvest", new WheatSeedsConverterModifier( new LootItemCondition[] { MatchTool.toolMatches(ItemPredicate.Builder.item().of(Items.SHEARS)).build(), @@ -193,13 +234,14 @@ public class GlobalLootModifiersTest extends BaseTestMod { 3, Items.WHEAT_SEEDS, Items.WHEAT) ); + // Tested by verifying that we drop 2 test blocks when broken instead of the default 1 add("multiply_loot", new MultiplyDropsModifier( new LootItemCondition[] { LootTableIdCondition.builder(TEST_BLOCK.get().getLootTable().location()).build() }, 2) ); - // Bamboo silk harvests everything + // Bamboo silk harvests everything, Tested by breaking glass and seeing if we get glass back. add("bamboo_silk", new SilkTouchTestModifier( new LootItemCondition[] { MatchTool.toolMatches(ItemPredicate.Builder.item().of(Items.BAMBOO)).build() @@ -275,7 +317,6 @@ public class GlobalLootModifiersTest extends BaseTestMod { /** * When harvesting blocks with bamboo, this modifier is invoked, via the silk_touch_bamboo loot_modifier json - * */ private static class SilkTouchTestModifier extends LootModifier { public static final Supplier> CODEC = Suppliers.memoize(() -> @@ -317,7 +358,6 @@ public class GlobalLootModifiersTest extends BaseTestMod { /** * When harvesting wheat with shears, this modifier is invoked via the wheat_harvest loot_modifier json
* This modifier checks how many seeds were harvested and turns X seeds into Y wheat (3:1) - * */ private static class WheatSeedsConverterModifier extends LootModifier { public static final Supplier> CODEC = Suppliers.memoize(() -> RecordCodecBuilder.mapCodec(inst -> codecStart(inst).and(