Remove Attack Range and Reach Distance and add Block Reach and Entity Reach (#9361)

src/main/java/net/minecraftforge/common/ForgeMod.java

@@ -145,30 +145,45 @@ public class ForgeMod
             ArgumentTypeInfos.registerByClass(ModIdArgument.class,
                     SingletonArgumentInfo.contextFree(ModIdArgument::modIdArgument)));
 
-    public static final RegistryObject<Attribute> SWIM_SPEED = ATTRIBUTES.register("swim_speed", () -> new RangedAttribute("forge.swimSpeed", 1.0D, 0.0D, 1024.0D).setSyncable(true));
-    public static final RegistryObject<Attribute> NAMETAG_DISTANCE = ATTRIBUTES.register("nametag_distance", () -> new RangedAttribute("forge.nameTagDistance", 64.0D, 0.0D, 64.0).setSyncable(true));
+    public static final RegistryObject<Attribute> SWIM_SPEED = ATTRIBUTES.register("swim_speed", () -> new RangedAttribute("forge.swim_speed", 1.0D, 0.0D, 1024.0D).setSyncable(true));
+    public static final RegistryObject<Attribute> NAMETAG_DISTANCE = ATTRIBUTES.register("nametag_distance", () -> new RangedAttribute("forge.name_tag_distance", 64.0D, 0.0D, 64.0).setSyncable(true));
     public static final RegistryObject<Attribute> ENTITY_GRAVITY = ATTRIBUTES.register("entity_gravity", () -> new RangedAttribute("forge.entity_gravity", 0.08D, -8.0D, 8.0D).setSyncable(true));
 
     /**
-     * Reach Distance represents the distance at which a player may interact with the world.  The default is 4.5 blocks.  Players in creative mode have an additional 0.5 blocks of reach distance.
-     * @see IForgePlayer#getReachDistance()
-     * @see IForgePlayer#canInteractWith(BlockPos, double)
-     * @see IForgePlayer#canInteractWith(Entity, double)
+     * Reach Distance represents the distance at which a player may interact with the world.  The default is 4.5 blocks.  Players in creative mode have an additional 0.5 blocks of block reach.
+     * @see IForgePlayer#getBlockReach()
+     * @see IForgePlayer#canReach(BlockPos, double)
      */
-    public static final RegistryObject<Attribute> REACH_DISTANCE = ATTRIBUTES.register("reach_distance", () -> new RangedAttribute("generic.reachDistance", 4.5D, 0.0D, 1024.0D).setSyncable(true));
+    public static final RegistryObject<Attribute> BLOCK_REACH = ATTRIBUTES.register("block_reach", () -> new RangedAttribute("forge.block_reach", 4.5D, 0.0D, 1024.0D).setSyncable(true));
 
     /**
-     * Attack Range represents the distance at which a player may attack an entity.  The default is 3 blocks.  Players in creative mode have an additional 3 blocks of attack reach.
-     * @see IForgePlayer#getAttackRange()
-     * @see IForgePlayer#canHit(Entity, double)
+     * TODO: Remove
+     * @deprecated - use {@link #BLOCK_REACH}
      */
-    public static final RegistryObject<Attribute> ATTACK_RANGE = ATTRIBUTES.register("attack_range", () -> new RangedAttribute("generic.attack_range", 3.0D, 0.0D, 1024.0D).setSyncable(true));
+    @Deprecated(forRemoval = true, since = "1.19.4")
+    public static final RegistryObject<Attribute> REACH_DISTANCE = BLOCK_REACH;
+
+    /**
+     * Attack Range represents the distance at which a player may attack an entity.  The default is 3 blocks.  Players in creative mode have an additional 3 blocks of entity reach.
+     * The default of 3.0 is technically considered a bug by Mojang - see MC-172289 and MC-92484. However, updating this value would allow for longer-range attacks on vanilla servers, which makes some people mad.
+     * @see IForgePlayer#getEntityReach()
+     * @see IForgePlayer#canReach(Entity, double)
+     * @see IForgePlayer#canReach(Vec3, double)
+     */
+    public static final RegistryObject<Attribute> ENTITY_REACH = ATTRIBUTES.register("entity_reach", () -> new RangedAttribute("forge.entity_reach", 3.0D, 0.0D, 1024.0D).setSyncable(true));
+
+    /**
+     * TODO: Remove
+     * @deprecated - use {@link #ENTITY_REACH}
+     */
+    @Deprecated(forRemoval = true, since = "1.19.4")
+    public static final RegistryObject<Attribute> ATTACK_RANGE = ENTITY_REACH;
 
     /**
      * Step Height Addition modifies the amount of blocks an entity may walk up without jumping.
      * @see IForgeEntity#getStepHeight()
      */
-    public static final RegistryObject<Attribute> STEP_HEIGHT_ADDITION = ATTRIBUTES.register("step_height_addition", () -> new RangedAttribute("forge.stepHeight", 0.0D, -512.0D, 512.0D).setSyncable(true));
+    public static final RegistryObject<Attribute> STEP_HEIGHT_ADDITION = ATTRIBUTES.register("step_height_addition", () -> new RangedAttribute("forge.step_height", 0.0D, -512.0D, 512.0D).setSyncable(true));
 
     /**
      * Noop biome modifier. Can be used in a biome modifier json with "type": "forge:none".
@@ -461,6 +476,10 @@ public class ForgeMod
         MinecraftForge.EVENT_BUS.addListener(this::registerPermissionNodes);
 
         ForgeRegistries.ITEMS.tags().addOptionalTagDefaults(Tags.Items.ENCHANTING_FUELS, Set.of(ForgeRegistries.ITEMS.getDelegateOrThrow(Items.LAPIS_LAZULI)));
+
+        // TODO: Remove when addAlias becomes proper API, as this should be done in the DR's above.
+        addAlias(ForgeRegistries.ATTRIBUTES, new ResourceLocation("forge", "reach_distance"), new ResourceLocation("forge", "block_reach"));
+        addAlias(ForgeRegistries.ATTRIBUTES, new ResourceLocation("forge", "attack_range"), new ResourceLocation("forge", "entity_reach"));
     }
 
     public void preInit(FMLCommonSetupEvent evt)
@@ -624,4 +643,14 @@ public class ForgeMod
     {
         event.addNodes(USE_SELECTORS_PERMISSION);
     }
+
+    /**
+     * TODO: Remove when {@link ForgeRegistry#addAlias(ResourceLocation, ResourceLocation)} is elevated to {@link IForgeRegistry}.
+     */
+    @Deprecated(forRemoval = true, since = "1.19.3")
+    private static <T> void addAlias(IForgeRegistry<T> registry, ResourceLocation from, ResourceLocation to)
+    {
+        ForgeRegistry<T> fReg = (ForgeRegistry<T>) registry;
+        fReg.addAlias(from, to);
+    }
 }

src/main/java/net/minecraftforge/common/extensions/IForgePlayer.java

@@ -5,11 +5,12 @@
 
 package net.minecraftforge.common.extensions;
 
-import java.util.Optional;
-
 import net.minecraft.core.BlockPos;
+import net.minecraft.util.Mth;
 import net.minecraft.world.entity.Entity;
 import net.minecraft.world.entity.player.Player;
+import net.minecraft.world.phys.AABB;
+import net.minecraft.world.phys.HitResult;
 import net.minecraft.world.phys.Vec3;
 import net.minecraftforge.common.ForgeMod;
 
@@ -22,74 +23,121 @@ public interface IForgePlayer
     }
 
     /**
-     * The attack range is increased by 3 for creative players, unless it is currently zero, which disables attacks.
-     * @return The attack range of this player.
+     * The entity reach is increased by 3 for creative players, unless it is currently zero, which disables attacks and entity interactions.
+     * @return The entity reach of this player.
      */
+    default double getEntityReach()
+    {
+        double range = self().getAttributeValue(ForgeMod.ENTITY_REACH.get());
+        return range == 0 ? 0 : range + (self().isCreative() ? 3 : 0);
+    }
+
+    /**
+     * @deprecated Use {@link #getEntityReach()}
+     */
+    @Deprecated(forRemoval = true, since = "1.19.4")
     default double getAttackRange()
     {
-        double range = self().getAttributeValue(ForgeMod.ATTACK_RANGE.get());
-        return range == 0 ? 0 : range + (self().isCreative() ? 3 : 0);
+        return this.getEntityReach();
     }
 
     /**
      * The reach distance is increased by 0.5 for creative players, unless it is currently zero, which disables interactions.
      * @return The reach distance of this player.
      */
-    default double getReachDistance()
+    default double getBlockReach()
     {
-        double reach = self().getAttributeValue(ForgeMod.REACH_DISTANCE.get());
+        double reach = self().getAttributeValue(ForgeMod.BLOCK_REACH.get());
         return reach == 0 ? 0 : reach + (self().isCreative() ? 0.5 : 0);
     }
 
     /**
-     * Checks if the player can attack the passed entity.<br>
-     * On the server, additional leniency is added to account for movement/lag.
+     * @deprecated use {@link #getBlockReach()}
+     */
+    @Deprecated(forRemoval = true, since = "1.19.4")
+    default double getReachDistance()
+    {
+        return this.getBlockReach();
+    }
+
+    /**
+     * Checks if the player can reach an entity by targeting the passed vector.<br>
+     * On the server, additional padding is added to account for movement/lag.
+     * @param vec The vector being range-checked.
+     * @param padding Extra validation distance.
+     * @return If the player can attack the entity.
+     * @apiNote Do not use for block checks, as this method uses {@link #getEntityReach()}
+     */
+    default boolean canReach(Vec3 entityHitVec, double padding)
+    {
+        return self().getEyePosition().closerThan(entityHitVec, getEntityReach() + padding);
+    }
+
+    /**
+     * Checks if the player can reach an entity.<br>
+     * On the server, additional padding is added to account for movement/lag.
      * @param entity The entity being range-checked.
      * @param padding Extra validation distance.
      * @return If the player can attack the passed entity.
+     * @apiNote Prefer using {@link #canReach(Vec3, double)} if you have a {@link HitResult} available.
      */
-    default boolean canHit(Entity entity, double padding) // Do not rename to canAttack - will conflict with LivingEntity#canAttack
+    default boolean canReach(Entity entity, double padding)
     {
-        return isCloseEnough(entity, getAttackRange() + padding);
+        return isCloseEnough(entity, getEntityReach() + padding);
     }
 
     /**
-     * Checks if the player can reach (right-click) the passed entity.<br>
-     * On the server, additional leniency is added to account for movement/lag.
-     * @param entity The entity being range-checked.
-     * @param padding Extra validation distance.
-     * @return If the player can interact with the passed entity.
+     * @deprecated use {@link #canReach(Entity, double)}h
      */
+    @Deprecated(forRemoval = true, since = "1.19.4")
+    default boolean canHit(Entity entity, double padding)
+    {
+    	return canReach(entity, padding);
+    }
+
+    /**
+     * @deprecated use {@link #canReach(Entity, double)}h
+     */
+    @Deprecated(forRemoval = true, since = "1.19.4")
     default boolean canInteractWith(Entity entity, double padding)
     {
-        return isCloseEnough(entity, getReachDistance() + padding);
+    	return canReach(entity, padding);
     }
 
     /**
-     * Utility check to see if the player is close enough to a target entity.
-     * @param entity The entity being checked against
-     * @param dist The max distance allowed
-     * @return If the eye-to-center distance between this player and the passed entity is less than dist.
-     */
-    default boolean isCloseEnough(Entity entity, double dist)
-    {
-        Vec3 eye = self().getEyePosition();
-        Vec3 targetCenter = entity.getPosition(1.0F).add(0, entity.getBbHeight() / 2, 0);
-        Optional<Vec3> hit = entity.getBoundingBox().clip(eye, targetCenter); //This hit should almost always be present, but we have a fallback just in case.  It likely indicates lag if it is not present.
-        return (hit.isPresent() ? eye.distanceToSqr(hit.get()) : self().distanceToSqr(entity)) < dist * dist;
-    }
-
-    /**
-     * Checks if the player can reach (right-click) a block.<br>
-     * On the server, additional leniency is added to account for movement/lag.
+     * Checks if the player can reach a block.<br>
+     * On the server, additional padding is added to account for movement/lag.
      * @param pos The position being range-checked.
      * @param padding Extra validation distance.
      * @return If the player can interact with this location.
      */
+    default boolean canReach(BlockPos pos, double padding)
+    {
+        double reach = this.getBlockReach() + padding;
+        return self().getEyePosition().distanceToSqr(Vec3.atCenterOf(pos)) < reach * reach;
+    }
+
+    /**
+     * @deprecated use {@link #canReach(BlockPos, double)}
+     */
+    @Deprecated(forRemoval = true, since = "1.19.4")
     default boolean canInteractWith(BlockPos pos, double padding)
     {
-        double reach = getReachDistance() + padding;
-        return self().getEyePosition().distanceToSqr(pos.getX() + 0.5D, pos.getY() + 0.5D, pos.getZ() + 0.5D) < reach * reach;
+    	return canReach(pos, padding);
+    }
+
+    /**
+     * Utility check to see if the player is close enough to a target entity. Uses "eye-to-closest-corner" checks.
+     * @param entity The entity being checked against
+     * @param dist The max distance allowed
+     * @return If the eye-to-center distance between this player and the passed entity is less than dist.
+     * @implNote This method inflates the bounding box by the pick radius, which differs from vanilla. But vanilla doesn't use the pick radius, the only entity with > 0 is AbstractHurtingProjectile.
+     */
+    default boolean isCloseEnough(Entity entity, double dist)
+    {
+        Vec3 eye = self().getEyePosition();
+        AABB aabb = entity.getBoundingBox().inflate(entity.getPickRadius());
+        return aabb.distanceToSqr(eye) < dist * dist;
     }
 
 }

src/main/java/net/minecraftforge/registries/ForgeRegistry.java

@@ -45,9 +45,11 @@ import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.Marker;
 import org.apache.logging.log4j.MarkerManager;
+import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
+@ApiStatus.Internal
 public class ForgeRegistry<V> implements IForgeRegistryInternal<V>, IForgeRegistryModifiable<V>
 {
     public static Marker REGISTRIES = MarkerManager.getMarker("REGISTRIES");
@@ -480,25 +482,33 @@ public class ForgeRegistry<V> implements IForgeRegistryInternal<V>, IForgeRegist
         return ret;
     }
 
-    void addAlias(ResourceLocation from, ResourceLocation to)
+    /**
+     * Adds an alias that maps from the name specified by <code>src</code> to the name specified by <code>dst</code>.<p>
+     * Any registry lookups that target the first name will resolve as the second name, if the first name is not present.
+     * @param src The source registry name to alias from.
+     * @param dst The target registry name to alias to.
+     * 
+     * TODO: Add as public API in IForgeRegistry and DeferredRegister.
+     */
+    public void addAlias(ResourceLocation src, ResourceLocation dst)
     {
         if (this.isLocked())
-            throw new IllegalStateException(String.format(Locale.ENGLISH, "Attempted to register the alias %s -> %s to late", from, to));
+            throw new IllegalStateException(String.format(Locale.ENGLISH, "Attempted to register the alias %s -> %s too late", src, dst));
 
-        if (from.equals(to))
+        if (src.equals(dst))
         {
-            LOGGER.warn(REGISTRIES, "Registry {} Ignoring invalid alias: {} -> {}", this.name, from, to);
+            LOGGER.warn(REGISTRIES, "Registry {} Ignoring invalid alias: {} -> {}", this.name, src, dst);
             return;
         }
 
-        this.aliases.put(from, to);
-        LOGGER.trace(REGISTRIES,"Registry {} alias: {} -> {}", this.name, from, to);
+        this.aliases.put(src, dst);
+        LOGGER.trace(REGISTRIES,"Registry {} alias: {} -> {}", this.name, src, dst);
     }
 
     void addDummy(ResourceLocation key)
     {
         if (this.isLocked())
-            throw new IllegalStateException(String.format(Locale.ENGLISH, "Attempted to register the dummy %s to late", key));
+            throw new IllegalStateException(String.format(Locale.ENGLISH, "Attempted to register the dummy %s too late", key));
         this.dummies.add(key);
         LOGGER.trace(REGISTRIES,"Registry {} dummy: {}", this.name, key);
     }
@@ -647,8 +657,14 @@ public class ForgeRegistry<V> implements IForgeRegistryInternal<V>, IForgeRegist
         this.max = from.max;
         this.min = from.min;
         */
-        this.aliases.clear();
-        from.aliases.forEach(this::addAlias);
+        // Aliases from the active registry take priority over those from the staged (loaded) registry.
+        for (var entry : from.aliases.entrySet())
+        {
+            if (!this.aliases.containsKey(entry.getKey()))
+            {
+                this.aliases.put(entry.getKey(), entry.getValue());
+            }
+        }
 
         this.ids.clear();
         this.names.clear();