Fix 1.21 issues relating to gameplay-enchantment support (#1089)

neoforged · Jun 13, 2024 · c9c4d98 · c9c4d98
1 parent 52526da
commit c9c4d98
Show file tree

Hide file tree

Showing 16 changed files with 290 additions and 125 deletions.
diff --git a/patches/net/minecraft/advancements/critereon/ItemEnchantmentsPredicate.java.patch b/patches/net/minecraft/advancements/critereon/ItemEnchantmentsPredicate.java.patch
@@ -1,14 +1,19 @@
 --- a/net/minecraft/advancements/critereon/ItemEnchantmentsPredicate.java
 +++ b/net/minecraft/advancements/critereon/ItemEnchantmentsPredicate.java
-@@ -48,6 +_,11 @@
-             super(p_333967_);
+@@ -52,6 +_,16 @@
+         public DataComponentType<ItemEnchantments> componentType() {
+             return DataComponents.ENCHANTMENTS;
          }
-
-+        @Override // Neo: use getAllEnchantments for enchantments
++
++        // Neo: use IItemExtension#getAllEnchantments for enchantments when testing this predicate.
++        @Override
 +        public boolean matches(ItemStack p_333958_) {
-+            return matches(p_333958_, p_333958_.getAllEnchantments());
++            var lookup = net.neoforged.neoforge.common.CommonHooks.resolveLookup(net.minecraft.core.registries.Registries.ENCHANTMENT);
++            if (lookup != null) {
++                return matches(p_333958_, p_333958_.getAllEnchantments(lookup));
++            }
++            return super.matches(p_333958_);
 +        }
-+
-         @Override
-         public DataComponentType<ItemEnchantments> componentType() {
-             return DataComponents.ENCHANTMENTS;
+     }
+
+     public static class StoredEnchantments extends ItemEnchantmentsPredicate {
diff --git a/patches/net/minecraft/core/Holder.java.patch b/patches/net/minecraft/core/Holder.java.patch
@@ -5,26 +5,10 @@
  import net.minecraft.tags.TagKey;
 
 -public interface Holder<T> {
-+public interface Holder<T> extends net.neoforged.neoforge.registries.datamaps.IWithData<T> {
++public interface Holder<T> extends net.neoforged.neoforge.common.extensions.IHolderExtension<T> {
      T value();
 
      boolean isBound();
-@@ -41,6 +_,15 @@
-         return this.unwrapKey().map(p_316542_ -> p_316542_.location().toString()).orElse("[unregistered]");
-     }
-
-+    /**
-+     * {@return the holder that this holder wraps}
-+     *
-+     * Useful for holders that delegate to another holder.
-+     */
-+    default Holder<T> getDelegate() {
-+        return this;
-+    }
-+
-     static <T> Holder<T> direct(T p_205710_) {
-         return new Holder.Direct<>(p_205710_);
-     }
 @@ -220,6 +_,14 @@
              }
          }
@@ -40,11 +24,12 @@
          public void bindTags(Collection<TagKey<T>> p_205770_) {
              this.tags = Set.copyOf(p_205770_);
          }
-@@ -232,6 +_,18 @@
-         @Override
+@@ -233,6 +_,28 @@
          public String toString() {
              return "Reference{" + this.key + "=" + this.value + "}";
-+        }
+         }
++
++        // Neo Start
 +
 +        // Neo: Add DeferredHolder-compatible hashCode() and equals() overrides
 +        @Override
@@ -56,6 +41,15 @@
 +        public boolean equals(Object obj) {
 +            if (this == obj) return true;
 +            return obj instanceof Holder<?> h && h.kind() == Kind.REFERENCE && h.unwrapKey().orElseThrow() == this.key();
-         }
++        }
++
++        @Override
++        @org.jetbrains.annotations.Nullable
++        public HolderLookup.RegistryLookup<T> unwrapLookup() {
++            return this.owner instanceof HolderLookup.RegistryLookup<T> rl ? rl : null;
++        }
++
++        // Neo End
 
          public static enum Type {
+             STAND_ALONE,
diff --git a/patches/net/minecraft/world/item/ArrowItem.java.patch b/patches/net/minecraft/world/item/ArrowItem.java.patch
@@ -1,13 +1,21 @@
 --- a/net/minecraft/world/item/ArrowItem.java
 +++ b/net/minecraft/world/item/ArrowItem.java
-@@ -24,4 +_,10 @@
+@@ -24,4 +_,18 @@
          arrow.pickup = AbstractArrow.Pickup.ALLOWED;
          return arrow;
      }
 +
-+    public boolean isInfinite(ItemStack stack, ItemStack bow, net.minecraft.world.entity.LivingEntity livingEntity) {
-+        // TODO 1.21 - probably needs to decide based on tags/other datapack logic
-+        int enchant = net.minecraft.world.item.enchantment.EnchantmentHelper.getItemEnchantmentLevel(livingEntity.registryAccess().registryOrThrow(net.minecraft.core.registries.Registries.ENCHANTMENT).getHolderOrThrow(net.minecraft.world.item.enchantment.Enchantments.INFINITY), bow);
-+        return enchant > 0 && this.getClass() == net.minecraft.world.item.ArrowItem.class;
++    /**
++     * Called to determine if this arrow will be infinite when fired. If an arrow is infinite, then the arrow will never be consumed (regardless of enchantments).
++     * <p>
++     * Only called on the logical server.
++     *
++     * @param ammo The ammo stack (containing this item)
++     * @param bow  The bow stack
++     * @param livingEntity The entity who is firing the bow
++     * @return True if the arrow is infinite
++     */
++    public boolean isInfinite(ItemStack ammo, ItemStack bow, net.minecraft.world.entity.LivingEntity livingEntity) {
++        return false;
 +    }
  }
diff --git a/patches/net/minecraft/world/item/ItemStack.java.patch b/patches/net/minecraft/world/item/ItemStack.java.patch
@@ -140,6 +140,24 @@
              return list;
          }
      }
+@@ -897,6 +_,17 @@
+         return !this.getOrDefault(DataComponents.ENCHANTMENTS, ItemEnchantments.EMPTY).isEmpty();
+     }
+
++    /**
++     * Gets all enchantments from NBT. Use {@link ItemStack#getAllEnchantments} for gameplay logic.
++     */
++    public ItemEnchantments getTagEnchantments() {
++        return getEnchantments();
++    }
++
++    /**
++     * @deprecated Neo: Use {@link #getTagEnchantments()} for NBT enchantments, or {@link #getAllEnchantments} for gameplay.
++     */
++    @Deprecated
+     public ItemEnchantments getEnchantments() {
+         return this.getOrDefault(DataComponents.ENCHANTMENTS, ItemEnchantments.EMPTY);
+     }
 @@ -933,6 +_,8 @@
      }
 

diff --git a/patches/net/minecraft/world/item/ProjectileWeaponItem.java.patch b/patches/net/minecraft/world/item/ProjectileWeaponItem.java.patch
@@ -9,12 +9,13 @@
      }
 
      protected static List<ItemStack> draw(ItemStack p_331565_, ItemStack p_330406_, LivingEntity p_330823_) {
-@@ -115,7 +_,7 @@
+@@ -115,7 +_,8 @@
      }
 
      protected static ItemStack useAmmo(ItemStack p_331207_, ItemStack p_331434_, LivingEntity p_330302_, boolean p_330934_) {
 -        int i = !p_330934_ && !p_330302_.hasInfiniteMaterials() && p_330302_.level() instanceof ServerLevel serverlevel
-+        int i = !p_330934_ && !(p_330302_.hasInfiniteMaterials() || (p_331434_.getItem() instanceof ArrowItem && ((ArrowItem) p_331434_.getItem()).isInfinite(p_331434_, p_331207_, p_330302_))) && p_330302_.level() instanceof ServerLevel serverlevel
++        // Neo: Adjust this check to respect ArrowItem#isInfinite, bypassing processAmmoUse if true.
++        int i = !p_330934_ && p_330302_.level() instanceof ServerLevel serverlevel && !(p_330302_.hasInfiniteMaterials() || (p_331434_.getItem() instanceof ArrowItem ai && ai.isInfinite(p_331434_, p_331207_, p_330302_)))
              ? EnchantmentHelper.processAmmoUse(serverlevel, p_331207_, p_331434_, 1)
              : 0;
          if (i > p_331434_.getCount()) {

diff --git a/patches/net/minecraft/world/item/enchantment/Enchantment.java.patch b/patches/net/minecraft/world/item/enchantment/Enchantment.java.patch
@@ -1,29 +1,29 @@
 --- a/net/minecraft/world/item/enchantment/Enchantment.java
 +++ b/net/minecraft/world/item/enchantment/Enchantment.java
-@@ -504,6 +_,26 @@
-         return new Enchantment.Builder(p_345873_);
+@@ -132,6 +_,10 @@
+         return this.definition.slots().stream().anyMatch(p_345027_ -> p_345027_.test(p_345146_));
      }
 
-+    // TODO 1.21: Make IEnchantmentExtension additions data-driven along with these methods:
++    /**
++     * @deprecated Neo: Use {@link ItemStack#isPrimaryItemFor(Holder)}
++     */
++    @Deprecated
+     public boolean isPrimaryItem(ItemStack p_336088_) {
+         return this.isSupportedItem(p_336088_) && (this.definition.primaryItems.isEmpty() || p_336088_.is(this.definition.primaryItems.get()));
+     }
+@@ -503,6 +_,15 @@
+     public static Enchantment.Builder enchantment(Enchantment.EnchantmentDefinition p_345873_) {
+         return new Enchantment.Builder(p_345873_);
+     }
 +
-+//    /**
-+//     * This applies specifically to applying at the enchanting table. The other method {@link #canEnchant(ItemStack)}
-+//     * applies for <i>all possible</i> enchantments.
-+//     * @param stack
-+//     * @return
-+//     */
-+//    public boolean canApplyAtEnchantingTable(ItemStack stack) {
-+//        return stack.canApplyAtEnchantingTable(this);
-+//    }
-+//
++//    TODO: Reimplement. Not sure if we want to patch EnchantmentDefinition or hack this in as an EnchantmentEffectComponent.
 +//    /**
 +//     * Is this enchantment allowed to be enchanted on books via Enchantment Table
 +//     * @return false to disable the vanilla feature
 +//     */
 +//    public boolean isAllowedOnBooks() {
 +//        return true;
 +//    }
-+
+ 
      public static class Builder {
          private final Enchantment.EnchantmentDefinition definition;
-         private HolderSet<Enchantment> exclusiveSet = HolderSet.direct();
diff --git a/patches/net/minecraft/world/item/enchantment/EnchantmentHelper.java.patch b/patches/net/minecraft/world/item/enchantment/EnchantmentHelper.java.patch
@@ -1,33 +1,62 @@
 --- a/net/minecraft/world/item/enchantment/EnchantmentHelper.java
 +++ b/net/minecraft/world/item/enchantment/EnchantmentHelper.java
-@@ -48,8 +_,7 @@
+@@ -47,7 +_,19 @@
+ import org.apache.commons.lang3.mutable.MutableObject;
 
  public class EnchantmentHelper {
++    /**
++     * @deprecated Neo: Use {@link #getTagEnchantmentLevel(Holder, ItemStack)} for NBT enchantments, or {@link ItemStack#getEnchantmentLevel(Holder)} for gameplay.
++     */
++    @Deprecated
      public static int getItemEnchantmentLevel(Holder<Enchantment> p_346179_, ItemStack p_44845_) {
--        ItemEnchantments itemenchantments = p_44845_.getOrDefault(DataComponents.ENCHANTMENTS, ItemEnchantments.EMPTY);
--        return itemenchantments.getLevel(p_346179_);
++        // Neo: To reduce patch size, update this method to always check gameplay enchantments, and add getTagEnchantmentLevel as a helper for mods.
 +        return p_44845_.getEnchantmentLevel(p_346179_);
++    }
++
++    /**
++     * Gets the level of an enchantment from NBT. Use {@link ItemStack#getEnchantmentLevel(Holder)} for gameplay logic.
++     */
++    public static int getTagEnchantmentLevel(Holder<Enchantment> p_346179_, ItemStack p_44845_) {
+         ItemEnchantments itemenchantments = p_44845_.getOrDefault(DataComponents.ENCHANTMENTS, ItemEnchantments.EMPTY);
+         return itemenchantments.getLevel(p_346179_);
      }
-
-     public static ItemEnchantments updateEnchantments(ItemStack p_331034_, Consumer<ItemEnchantments.Mutable> p_332031_) {
-@@ -120,7 +_,7 @@
-     }
-
+@@ -122,6 +_,12 @@
      private static void runIterationOnItem(ItemStack p_345425_, EnchantmentHelper.EnchantmentVisitor p_345023_) {
--        ItemEnchantments itemenchantments = p_345425_.getOrDefault(DataComponents.ENCHANTMENTS, ItemEnchantments.EMPTY);
-+        ItemEnchantments itemenchantments = p_345425_.getAllEnchantments(); // Neo: Allow gameplay enchantments to run too.
+         ItemEnchantments itemenchantments = p_345425_.getOrDefault(DataComponents.ENCHANTMENTS, ItemEnchantments.EMPTY);
 
++        // Neo: Respect gameplay-only enchantments when doing iterations
++        var lookup = net.neoforged.neoforge.common.CommonHooks.resolveLookup(net.minecraft.core.registries.Registries.ENCHANTMENT);
++        if (lookup != null) {
++            itemenchantments = p_345425_.getAllEnchantments(lookup);
++        }
++
          for (Entry<Holder<Enchantment>> entry : itemenchantments.entrySet()) {
              p_345023_.accept(entry.getKey(), entry.getIntValue());
-@@ -131,7 +_,7 @@
-         ItemStack p_44852_, EquipmentSlot p_345566_, LivingEntity p_345792_, EnchantmentHelper.EnchantmentInSlotVisitor p_345683_
+         }
+@@ -132,6 +_,10 @@
      ) {
          if (!p_44852_.isEmpty()) {
--            ItemEnchantments itemenchantments = p_44852_.get(DataComponents.ENCHANTMENTS);
-+            ItemEnchantments itemenchantments = p_44852_.getAllEnchantments(); // Neo: Allow gameplay enchantments to run too.
+             ItemEnchantments itemenchantments = p_44852_.get(DataComponents.ENCHANTMENTS);
++
++            // Neo: Respect gameplay-only enchantments when doing iterations
++            itemenchantments = p_44852_.getAllEnchantments(p_345792_.registryAccess().lookupOrThrow(net.minecraft.core.registries.Registries.ENCHANTMENT));
++
              if (itemenchantments != null && !itemenchantments.isEmpty()) {
                  EnchantedItemInUse enchantediteminuse = new EnchantedItemInUse(p_44852_, p_345566_, p_345792_);
 
+@@ -417,6 +_,12 @@
+     public static boolean hasTag(ItemStack p_345665_, TagKey<Enchantment> p_345928_) {
+         ItemEnchantments itemenchantments = p_345665_.getOrDefault(DataComponents.ENCHANTMENTS, ItemEnchantments.EMPTY);
+
++        // Neo: Respect gameplay-only enchantments when enchantment effect tag checks
++        var lookup = net.neoforged.neoforge.common.CommonHooks.resolveLookup(net.minecraft.core.registries.Registries.ENCHANTMENT);
++        if (lookup != null) {
++            itemenchantments = p_345665_.getAllEnchantments(lookup);
++        }
++
+         for (Entry<Holder<Enchantment>> entry : itemenchantments.entrySet()) {
+             Holder<Enchantment> holder = entry.getKey();
+             if (holder.is(p_345928_)) {
 @@ -484,7 +_,7 @@
 
      public static int getEnchantmentCost(RandomSource p_220288_, int p_220289_, int p_220290_, ItemStack p_220291_) {
@@ -46,3 +75,14 @@
          if (i <= 0) {
              return list;
          } else {
+@@ -575,7 +_,9 @@
+     public static List<EnchantmentInstance> getAvailableEnchantmentResults(int p_44818_, ItemStack p_44819_, Stream<Holder<Enchantment>> p_345348_) {
+         List<EnchantmentInstance> list = Lists.newArrayList();
+         boolean flag = p_44819_.is(Items.BOOK);
+-        p_345348_.filter(p_344529_ -> p_344529_.value().isPrimaryItem(p_44819_) || flag).forEach(p_344478_ -> {
++        // Neo: Rewrite filter logic to call isPrimaryItemFor instead of hardcoded vanilla logic.
++        // The original logic is recorded in the default implementation of IItemExtension#isPrimaryItemFor.
++        p_345348_.filter(p_44819_::isPrimaryItemFor).forEach(p_344478_ -> {
+             Enchantment enchantment = p_344478_.value();
+
+             for (int i = enchantment.getMaxLevel(); i >= enchantment.getMinLevel(); i--) {
diff --git a/src/main/java/net/neoforged/neoforge/client/ClientHooks.java b/src/main/java/net/neoforged/neoforge/client/ClientHooks.java
@@ -90,12 +90,15 @@
 import net.minecraft.client.sounds.SoundEngine;
 import net.minecraft.core.BlockPos;
 import net.minecraft.core.Direction;
+import net.minecraft.core.HolderLookup.RegistryLookup;
+import net.minecraft.core.Registry;
 import net.minecraft.locale.Language;
 import net.minecraft.network.Connection;
 import net.minecraft.network.chat.ChatType;
 import net.minecraft.network.chat.Component;
 import net.minecraft.network.chat.FormattedText;
 import net.minecraft.network.chat.PlayerChatMessage;
+import net.minecraft.resources.ResourceKey;
 import net.minecraft.resources.ResourceLocation;
 import net.minecraft.server.packs.resources.ReloadableResourceManager;
 import net.minecraft.util.Mth;
@@ -1048,4 +1051,14 @@ public static void fireClientTickPre() {
     public static void fireClientTickPost() {
         NeoForge.EVENT_BUS.post(new ClientTickEvent.Post());
     }
+
+    @Nullable
+    @SuppressWarnings("resource")
+    public static <T> RegistryLookup<T> resolveLookup(ResourceKey<? extends Registry<T>> key) {
+        ClientLevel level = Minecraft.getInstance().level;
+        if (level != null) {
+            return level.registryAccess().lookup(key).orElse(null);
+        }
+        return null;
+    }
 }
diff --git a/src/main/java/net/neoforged/neoforge/common/CommonHooks.java b/src/main/java/net/neoforged/neoforge/common/CommonHooks.java
@@ -42,6 +42,7 @@
 import net.minecraft.core.Direction;
 import net.minecraft.core.Holder;
 import net.minecraft.core.HolderLookup;
+import net.minecraft.core.HolderLookup.RegistryLookup;
 import net.minecraft.core.HolderSet;
 import net.minecraft.core.Registry;
 import net.minecraft.core.SectionPos;
@@ -63,6 +64,7 @@
 import net.minecraft.network.syncher.EntityDataSerializer;
 import net.minecraft.resources.ResourceKey;
 import net.minecraft.resources.ResourceLocation;
+import net.minecraft.server.MinecraftServer;
 import net.minecraft.server.level.ServerLevel;
 import net.minecraft.server.level.ServerPlayer;
 import net.minecraft.server.packs.PackType;
@@ -124,6 +126,7 @@
 import net.minecraft.world.level.block.Blocks;
 import net.minecraft.world.level.block.GameMasterBlock;
 import net.minecraft.world.level.block.entity.BlockEntity;
+import net.minecraft.world.level.block.state.BlockBehaviour;
 import net.minecraft.world.level.block.state.BlockState;
 import net.minecraft.world.level.block.state.pattern.BlockInWorld;
 import net.minecraft.world.level.chunk.ChunkAccess;
@@ -142,6 +145,8 @@
 import net.neoforged.fml.ModList;
 import net.neoforged.fml.ModLoader;
 import net.neoforged.fml.i18n.MavenVersionTranslator;
+import net.neoforged.fml.loading.FMLEnvironment;
+import net.neoforged.neoforge.client.ClientHooks;
 import net.neoforged.neoforge.common.conditions.ConditionalOps;
 import net.neoforged.neoforge.common.extensions.IEntityExtension;
 import net.neoforged.neoforge.common.loot.IGlobalLootModifier;
@@ -193,6 +198,7 @@
 import net.neoforged.neoforge.fluids.FluidType;
 import net.neoforged.neoforge.registries.NeoForgeRegistries;
 import net.neoforged.neoforge.resource.ResourcePackLoader;
+import net.neoforged.neoforge.server.ServerLifecycleHooks;
 import net.neoforged.neoforge.server.permission.PermissionAPI;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
@@ -1380,4 +1386,24 @@ public static boolean canMobEffectBeApplied(LivingEntity entity, MobEffectInstan
         var event = new MobEffectEvent.Applicable(entity, effect);
         return NeoForge.EVENT_BUS.post(event).getApplicationResult();
     }
+
+    /**
+     * Attempts to resolve a {@link RegistryLookup} using the current global state.
+     * <p>
+     * Prioritizes the server's lookup, only attempting to retrieve it from the client if the server is unavailable.
+     * 
+     * @param <T> The type of registry being looked up
+     * @param key The resource key for the target registry
+     * @return A registry access, if one was available.
+     */
+    @Nullable
+    public static <T> RegistryLookup<T> resolveLookup(ResourceKey<? extends Registry<T>> key) {
+        MinecraftServer server = ServerLifecycleHooks.getCurrentServer();
+        if (server != null) {
+            return server.registryAccess().lookup(key).orElse(null);
+        } else if (FMLEnvironment.dist.isClient()) {
+            return ClientHooks.resolveLookup(key);
+        }
+        return null;
+    }
 }