Fix NeoForge incompatibility with projectile impact hooks

Reverts "Update projectile hit entity hook in light of forge's changes to the event" (commit e340dad)
Want to prevent adding a new API that will be removed once we switch to Neo in 1.21, so just restore the 1.19 version of the entity hit
For block hit, no way for me to cancel the event that is compatible with both loaders, so just ditch the return on the hook for now; we don't use it.

src/main/java/slimeknights/tconstruct/library/modifiers/hook/ranged/ProjectileHitModifierHook.java

@@ -4,7 +4,6 @@
 import net.minecraft.world.entity.projectile.Projectile;
 import net.minecraft.world.phys.BlockHitResult;
 import net.minecraft.world.phys.EntityHitResult;
-import net.minecraftforge.event.entity.ProjectileImpactEvent.ImpactResult;
 import slimeknights.tconstruct.library.modifiers.ModifierEntry;
 import slimeknights.tconstruct.library.tools.nbt.ModDataNBT;
 import slimeknights.tconstruct.library.tools.nbt.ModifierNBT;
@@ -23,58 +22,40 @@ public interface ProjectileHitModifierHook {
    * @param hit             Hit result
    * @param attacker        Living entity who fired the projectile, null if non-living or not fired
    * @param target          Living target, will be null if not living
-   * @return Impact result. Options:
-   * <ul>
-   *   <li>{@code DEFAULT} means process the hit as normal.</li>
-   *   <li>{@code STOP_AT_CURRENT} means process the hit as normal, but clear piercing level.</li>
-   *   <li>{@code STOP_AT_CURRENT_NO_DAMAGE} means the hit will not be processed and the entity will be discarded. Further modifiers will not run.</li>
-   *   <li>{@code SKIP_ENTITY} means the hit will not be processed for this entity, but the projectile may continue due to piercing. Further modifiers will not run.</li>
-   * </ul>
+   * @return true if the hit should be canceled, preventing vanilla logic
    */
-  default ImpactResult onProjectileHitEntity(ModifierNBT modifiers, ModDataNBT persistentData, ModifierEntry modifier, Projectile projectile, EntityHitResult hit, @Nullable LivingEntity attacker, @Nullable LivingEntity target) {
-    return ImpactResult.DEFAULT;
+  default boolean onProjectileHitEntity(ModifierNBT modifiers, ModDataNBT persistentData, ModifierEntry modifier, Projectile projectile, EntityHitResult hit, @Nullable LivingEntity attacker, @Nullable LivingEntity target) {
+    return false;
   }
 
   /**
-   * Called when a projectile hits a block
+   * Called when a projectile hits a block.
+   * TODO 1.21: bring back canceling behavior once we no longer have to deal with inconsistent APIs on Neo vs Forge?
    * @param modifiers       Modifiers from the tool firing this arrow
    * @param persistentData  Persistent data on the entity
    * @param modifier        Modifier triggering this hook
    * @param projectile      Projectile that hit the entity
    * @param hit             Hit result
    * @param attacker        Living entity who fired the projectile, null if non-living or not fired
-   * @return true if the hit should be canceled
    */
-  default boolean onProjectileHitBlock(ModifierNBT modifiers, ModDataNBT persistentData, ModifierEntry modifier, Projectile projectile, BlockHitResult hit, @Nullable LivingEntity attacker) {
-    return false;
-  }
+  default void onProjectileHitBlock(ModifierNBT modifiers, ModDataNBT persistentData, ModifierEntry modifier, Projectile projectile, BlockHitResult hit, @Nullable LivingEntity attacker) {}
 
   /** Merger that runs all hooks and returns true if any did */
   record AllMerger(Collection<ProjectileHitModifierHook> modules) implements ProjectileHitModifierHook {
     @Override
-    public ImpactResult onProjectileHitEntity(ModifierNBT modifiers, ModDataNBT persistentData, ModifierEntry modifier, Projectile projectile, EntityHitResult hit, @Nullable LivingEntity attacker, @Nullable LivingEntity target) {
-      ImpactResult ret = ImpactResult.DEFAULT;
+    public boolean onProjectileHitEntity(ModifierNBT modifiers, ModDataNBT persistentData, ModifierEntry modifier, Projectile projectile, EntityHitResult hit, @Nullable LivingEntity attacker, @Nullable LivingEntity target) {
+      boolean ret = false;
       for (ProjectileHitModifierHook module : modules) {
-        ImpactResult newResult = module.onProjectileHitEntity(modifiers, persistentData, modifier, projectile, hit, attacker, target);
-        // if a hook says stop at current, continue running
-        if (newResult != ImpactResult.DEFAULT) {
-          ret = newResult;
-        }
-        // if a hook says skip or stop, we are done. Former means this entity is ignored while latter means
-        if (newResult == ImpactResult.SKIP_ENTITY || newResult == ImpactResult.STOP_AT_CURRENT_NO_DAMAGE) {
-          break;
-        }
+        ret |= module.onProjectileHitEntity(modifiers, persistentData, modifier, projectile, hit, attacker, target);
       }
       return ret;
     }
 
     @Override
-    public boolean onProjectileHitBlock(ModifierNBT modifiers, ModDataNBT persistentData, ModifierEntry modifier, Projectile projectile, BlockHitResult hit, @Nullable LivingEntity attacker) {
-      boolean ret = false;
+    public void onProjectileHitBlock(ModifierNBT modifiers, ModDataNBT persistentData, ModifierEntry modifier, Projectile projectile, BlockHitResult hit, @Nullable LivingEntity attacker) {
       for (ProjectileHitModifierHook module : modules) {
-        ret |= module.onProjectileHitBlock(modifiers, persistentData, modifier, projectile, hit, attacker);
+        module.onProjectileHitBlock(modifiers, persistentData, modifier, projectile, hit, attacker);
       }
-      return ret;
     }
   }
 }

src/main/java/slimeknights/tconstruct/library/modifiers/modules/combat/MobEffectModule.java

@@ -12,7 +12,6 @@
 import net.minecraft.world.entity.LivingEntity;
 import net.minecraft.world.entity.projectile.Projectile;
 import net.minecraft.world.phys.EntityHitResult;
-import net.minecraftforge.event.entity.ProjectileImpactEvent.ImpactResult;
 import slimeknights.mantle.data.loadable.Loadables;
 import slimeknights.mantle.data.loadable.record.RecordLoadable;
 import slimeknights.mantle.data.predicate.IJsonPredicate;
@@ -93,9 +92,9 @@ public void afterMeleeHit(IToolStackView tool, ModifierEntry modifier, ToolAttac
   }
 
   @Override
-  public ImpactResult onProjectileHitEntity(ModifierNBT modifiers, ModDataNBT persistentData, ModifierEntry modifier, Projectile projectile, EntityHitResult hit, @Nullable LivingEntity attacker, @Nullable LivingEntity target) {
+  public boolean onProjectileHitEntity(ModifierNBT modifiers, ModDataNBT persistentData, ModifierEntry modifier, Projectile projectile, EntityHitResult hit, @Nullable LivingEntity attacker, @Nullable LivingEntity target) {
     applyEffect(target, modifier.getEffectiveLevel());
-    return ImpactResult.DEFAULT;
+    return false;
   }
 
   @Override

src/main/java/slimeknights/tconstruct/tools/logic/ToolEvents.java

@@ -31,7 +31,6 @@
 import net.minecraft.world.phys.EntityHitResult;
 import net.minecraft.world.phys.HitResult;
 import net.minecraftforge.event.entity.ProjectileImpactEvent;
-import net.minecraftforge.event.entity.ProjectileImpactEvent.ImpactResult;
 import net.minecraftforge.event.entity.living.LivingAttackEvent;
 import net.minecraftforge.event.entity.living.LivingDamageEvent;
 import net.minecraftforge.event.entity.living.LivingEvent.LivingTickEvent;
@@ -414,25 +413,15 @@ static void livingVisibility(LivingVisibilityEvent event) {
   }
 
   /** Implements projectile hit hook */
+  @SuppressWarnings("removal")  // can't update without losing Neo compat
   @SubscribeEvent
   static void projectileHit(ProjectileImpactEvent event) {
-    ImpactResult result = event.getImpactResult();
-    // if the event was "canceled" do nothing
-    if (result == ImpactResult.STOP_AT_CURRENT_NO_DAMAGE) {
-      return;
-    }
-    // if told to skip the entity also do nothing
-    HitResult hit = event.getRayTraceResult();
-    HitResult.Type type = hit.getType();
-    if (result == ImpactResult.SKIP_ENTITY && type == HitResult.Type.ENTITY) {
-      return;
-    }
-    // from this point on we are either DEFAULT or STOP_AT_CURRENT, either one we do stuff
-
     Projectile projectile = event.getProjectile();
     ModifierNBT modifiers = EntityModifierCapability.getOrEmpty(projectile);
     if (!modifiers.isEmpty()) {
       ModDataNBT nbt = PersistentDataCapability.getOrWarn(projectile);
+      HitResult hit = event.getRayTraceResult();
+      HitResult.Type type = hit.getType();
       // extract a firing entity as that is a common need
       LivingEntity attacker = projectile.getOwner() instanceof LivingEntity l ? l : null;
       switch(type) {
@@ -444,23 +433,22 @@ static void projectileHit(ProjectileImpactEvent event) {
             // extract a living target as that is the most common need
             LivingEntity target = ToolAttackUtil.getLivingEntity(entityHit.getEntity());
             for (ModifierEntry entry : modifiers.getModifiers()) {
-              ImpactResult newResult = entry.getHook(ModifierHooks.PROJECTILE_HIT).onProjectileHitEntity(modifiers, nbt, entry, projectile, entityHit, attacker, target);
-              if (newResult != ImpactResult.DEFAULT) {
-                result = newResult;
-              }
-              if (newResult == ImpactResult.SKIP_ENTITY || newResult == ImpactResult.STOP_AT_CURRENT_NO_DAMAGE) {
+              if (entry.getHook(ModifierHooks.PROJECTILE_HIT).onProjectileHitEntity(modifiers, nbt, entry, projectile, entityHit, attacker, target)) {
+                // on forge, this means the cancelled entity won't be hit again if its a piercing arrow
+                // on neo, they will get processed again next frame. Is this something we need to work around?
+                event.setCanceled(true);
                 break;
               }
             }
-            event.setImpactResult(result);
           }
         }
         case BLOCK -> {
           BlockHitResult blockHit = (BlockHitResult)hit;
           for (ModifierEntry entry : modifiers.getModifiers()) {
-            if (entry.getHook(ModifierHooks.PROJECTILE_HIT).onProjectileHitBlock(modifiers, nbt, entry, projectile, blockHit, attacker)) {
-              event.setImpactResult(ImpactResult.STOP_AT_CURRENT_NO_DAMAGE);
-            }
+            // TODO 1.21: consider bringing back canceling to this hook
+            // we can't cancel the event as on Forge that does nothing while Neo will prevent the block hit
+            // Forge wants us to set the impact result to "cancel" it, but that will cause Neo to crash.
+            entry.getHook(ModifierHooks.PROJECTILE_HIT).onProjectileHitBlock(modifiers, nbt, entry, projectile, blockHit, attacker);
           }
         }
       }

src/main/java/slimeknights/tconstruct/tools/modifiers/traits/general/EnderportingModifier.java

@@ -19,7 +19,6 @@
 import net.minecraft.world.phys.EntityHitResult;
 import net.minecraft.world.phys.Vec3;
 import net.minecraftforge.common.MinecraftForge;
-import net.minecraftforge.event.entity.ProjectileImpactEvent.ImpactResult;
 import slimeknights.tconstruct.TConstruct;
 import slimeknights.tconstruct.common.Sounds;
 import slimeknights.tconstruct.library.events.teleport.EnderportingTeleportEvent;
@@ -143,26 +142,25 @@ public void afterHarvest(IToolStackView tool, ModifierEntry modifier, UseOnConte
   }
 
   @Override
-  public ImpactResult onProjectileHitEntity(ModifierNBT modifiers, ModDataNBT persistentData, ModifierEntry modifier, Projectile projectile, EntityHitResult hit, @Nullable LivingEntity attacker, @Nullable LivingEntity target) {
+  public boolean onProjectileHitEntity(ModifierNBT modifiers, ModDataNBT persistentData, ModifierEntry modifier, Projectile projectile, EntityHitResult hit, @Nullable LivingEntity attacker, @Nullable LivingEntity target) {
     if (attacker != null && attacker != target && persistentData.getBoolean(PRIMARY_ARROW)) {
       Entity hitEntity = hit.getEntity();
       Vec3 oldPosition = attacker.position();
       if (attacker.level() == projectile.level() && tryTeleport(attacker, hitEntity.getX(), hitEntity.getY(), hitEntity.getZ()) && target != null) {
         tryTeleport(target, oldPosition.x, oldPosition.y, oldPosition.z);
       }
     }
-    return ImpactResult.DEFAULT;
+    return false;
   }
 
   @Override
-  public boolean onProjectileHitBlock(ModifierNBT modifiers, ModDataNBT persistentData, ModifierEntry modifier, Projectile projectile, BlockHitResult hit, @Nullable LivingEntity attacker) {
+  public void onProjectileHitBlock(ModifierNBT modifiers, ModDataNBT persistentData, ModifierEntry modifier, Projectile projectile, BlockHitResult hit, @Nullable LivingEntity attacker) {
     if (attacker != null && persistentData.getBoolean(PRIMARY_ARROW)) {
       BlockPos target = hit.getBlockPos().relative(hit.getDirection());
       if (attacker.level() == projectile.level() && tryTeleport(attacker, target.getX() + 0.5f, target.getY(), target.getZ() + 0.5f)) {
         projectile.discard();
       }
     }
-    return false;
   }
 
   @Override

src/main/java/slimeknights/tconstruct/tools/modifiers/traits/melee/DecayModifier.java

@@ -6,7 +6,6 @@
 import net.minecraft.world.entity.projectile.AbstractArrow;
 import net.minecraft.world.entity.projectile.Projectile;
 import net.minecraft.world.phys.EntityHitResult;
-import net.minecraftforge.event.entity.ProjectileImpactEvent.ImpactResult;
 import slimeknights.tconstruct.library.modifiers.Modifier;
 import slimeknights.tconstruct.library.modifiers.ModifierEntry;
 import slimeknights.tconstruct.library.modifiers.ModifierHooks;
@@ -16,8 +15,8 @@
 import slimeknights.tconstruct.library.module.ModuleHookMap.Builder;
 import slimeknights.tconstruct.library.tools.context.ToolAttackContext;
 import slimeknights.tconstruct.library.tools.nbt.IToolStackView;
-import slimeknights.tconstruct.library.tools.nbt.ModDataNBT;
 import slimeknights.tconstruct.library.tools.nbt.ModifierNBT;
+import slimeknights.tconstruct.library.tools.nbt.ModDataNBT;
 
 import javax.annotation.Nullable;
 
@@ -54,12 +53,12 @@ public void afterMeleeHit(IToolStackView tool, ModifierEntry modifier, ToolAttac
   }
 
   @Override
-  public ImpactResult onProjectileHitEntity(ModifierNBT modifiers, ModDataNBT persistentData, ModifierEntry modifier, Projectile projectile, EntityHitResult hit, @Nullable LivingEntity attacker, @Nullable LivingEntity target) {
+  public boolean onProjectileHitEntity(ModifierNBT modifiers, ModDataNBT persistentData, ModifierEntry modifier, Projectile projectile, EntityHitResult hit, @Nullable LivingEntity attacker, @Nullable LivingEntity target) {
     if (target != null && (!(projectile instanceof AbstractArrow arrow) || arrow.isCritArrow())) {
       // always poison the target, means it works twice as often as lacerating
       target.addEffect(makeDecayEffect(modifier.getLevel()));
     }
-    return ImpactResult.DEFAULT;
+    return false;
   }
 
   @Override

src/main/java/slimeknights/tconstruct/tools/modifiers/traits/melee/EnderferenceModifier.java

@@ -20,7 +20,6 @@
 import net.minecraft.world.phys.Vec3;
 import net.minecraftforge.common.MinecraftForge;
 import net.minecraftforge.event.entity.EntityTeleportEvent;
-import net.minecraftforge.event.entity.ProjectileImpactEvent.ImpactResult;
 import slimeknights.tconstruct.common.TinkerDamageTypes;
 import slimeknights.tconstruct.library.modifiers.Modifier;
 import slimeknights.tconstruct.library.modifiers.ModifierEntry;
@@ -32,8 +31,8 @@
 import slimeknights.tconstruct.library.tools.context.EquipmentContext;
 import slimeknights.tconstruct.library.tools.context.ToolAttackContext;
 import slimeknights.tconstruct.library.tools.nbt.IToolStackView;
-import slimeknights.tconstruct.library.tools.nbt.ModDataNBT;
 import slimeknights.tconstruct.library.tools.nbt.ModifierNBT;
+import slimeknights.tconstruct.library.tools.nbt.ModDataNBT;
 import slimeknights.tconstruct.tools.TinkerModifiers;
 
 import javax.annotation.Nullable;
@@ -103,7 +102,7 @@ public void onAttacked(IToolStackView tool, ModifierEntry modifier, EquipmentCon
   }
 
   @Override
-  public ImpactResult onProjectileHitEntity(ModifierNBT modifiers, ModDataNBT persistentData, ModifierEntry modifier, Projectile projectile, EntityHitResult hit, @Nullable LivingEntity attacker, @Nullable LivingEntity target) {
+  public boolean onProjectileHitEntity(ModifierNBT modifiers, ModDataNBT persistentData, ModifierEntry modifier, Projectile projectile, EntityHitResult hit, @Nullable LivingEntity attacker, @Nullable LivingEntity target) {
     if (target != null) {
       target.addEffect(new MobEffectInstance(TinkerModifiers.enderferenceEffect.get(), modifier.getLevel() * 100, 0, false, true, true));
 
@@ -118,7 +117,8 @@ public ImpactResult onProjectileHitEntity(ModifierNBT modifiers, ModDataNBT pers
             arrow.piercedAndKilledEntities = Lists.newArrayListWithCapacity(5);
           }
           if (arrow.piercingIgnoreEntityIds.size() >= arrow.getPierceLevel() + 1) {
-            return ImpactResult.STOP_AT_CURRENT_NO_DAMAGE;
+            arrow.discard();
+            return true;
           }
           arrow.piercingIgnoreEntityIds.add(target.getId());
         }
@@ -178,7 +178,7 @@ public ImpactResult onProjectileHitEntity(ModifierNBT modifiers, ModDataNBT pers
 
           arrow.playSound(arrow.soundEvent, 1.0F, 1.2F / (RANDOM.nextFloat() * 0.2F + 0.9F));
           if (arrow.getPierceLevel() <= 0) {
-            return ImpactResult.STOP_AT_CURRENT_NO_DAMAGE;
+            arrow.discard();
           }
         } else {
           // reset fire and drop the arrow
@@ -191,13 +191,13 @@ public ImpactResult onProjectileHitEntity(ModifierNBT modifiers, ModDataNBT pers
               arrow.spawnAtLocation(arrow.getPickupItem(), 0.1F);
             }
 
-            return ImpactResult.STOP_AT_CURRENT_NO_DAMAGE;
+            arrow.discard();
           }
         }
 
-        return ImpactResult.SKIP_ENTITY;
+        return true;
       }
     }
-    return ImpactResult.DEFAULT;
+    return false;
   }
 }

src/main/java/slimeknights/tconstruct/tools/modifiers/traits/melee/InsatiableModifier.java

@@ -6,7 +6,6 @@
 import net.minecraft.world.entity.projectile.Projectile;
 import net.minecraft.world.item.TooltipFlag;
 import net.minecraft.world.phys.EntityHitResult;
-import net.minecraftforge.event.entity.ProjectileImpactEvent.ImpactResult;
 import slimeknights.mantle.client.TooltipKey;
 import slimeknights.tconstruct.common.TinkerEffect;
 import slimeknights.tconstruct.library.modifiers.Modifier;
@@ -74,11 +73,11 @@ public float modifyStat(IToolStackView tool, ModifierEntry modifier, LivingEntit
   }
 
   @Override
-  public ImpactResult onProjectileHitEntity(ModifierNBT modifiers, ModDataNBT persistentData, ModifierEntry modifier, Projectile projectile, EntityHitResult hit, @Nullable LivingEntity attacker, @Nullable LivingEntity target) {
+  public boolean onProjectileHitEntity(ModifierNBT modifiers, ModDataNBT persistentData, ModifierEntry modifier, Projectile projectile, EntityHitResult hit, @Nullable LivingEntity attacker, @Nullable LivingEntity target) {
     if (attacker != null) {
       applyEffect(attacker, ToolType.RANGED, 10*20, 1, 7);
     }
-    return ImpactResult.DEFAULT;
+    return false;
   }
 
   @Override

src/main/java/slimeknights/tconstruct/tools/modifiers/traits/melee/LaceratingModifier.java

@@ -8,7 +8,6 @@
 import net.minecraft.world.entity.projectile.AbstractArrow;
 import net.minecraft.world.entity.projectile.Projectile;
 import net.minecraft.world.phys.EntityHitResult;
-import net.minecraftforge.event.entity.ProjectileImpactEvent.ImpactResult;
 import slimeknights.tconstruct.library.modifiers.Modifier;
 import slimeknights.tconstruct.library.modifiers.ModifierEntry;
 import slimeknights.tconstruct.library.modifiers.ModifierHooks;
@@ -19,8 +18,8 @@
 import slimeknights.tconstruct.library.tools.context.EquipmentContext;
 import slimeknights.tconstruct.library.tools.context.ToolAttackContext;
 import slimeknights.tconstruct.library.tools.nbt.IToolStackView;
-import slimeknights.tconstruct.library.tools.nbt.ModDataNBT;
 import slimeknights.tconstruct.library.tools.nbt.ModifierNBT;
+import slimeknights.tconstruct.library.tools.nbt.ModDataNBT;
 import slimeknights.tconstruct.tools.TinkerModifiers;
 
 import javax.annotation.Nullable;
@@ -50,15 +49,15 @@ public void afterMeleeHit(IToolStackView tool, ModifierEntry modifier, ToolAttac
   }
 
   @Override
-  public ImpactResult onProjectileHitEntity(ModifierNBT modifiers, ModDataNBT persistentData, ModifierEntry modifier, Projectile projectile, EntityHitResult hit, @Nullable LivingEntity attacker, @Nullable LivingEntity target) {
+  public boolean onProjectileHitEntity(ModifierNBT modifiers, ModDataNBT persistentData, ModifierEntry modifier, Projectile projectile, EntityHitResult hit, @Nullable LivingEntity attacker, @Nullable LivingEntity target) {
     if (target != null && (!(projectile instanceof AbstractArrow arrow) || arrow.isCritArrow()) && target.isAlive() && RANDOM.nextFloat() < 0.50f) {
       Entity owner = projectile.getOwner();
       if (owner != null) {
         target.setLastHurtMob(owner);
       }
       applyEffect(target, modifier.getLevel());
     }
-    return ImpactResult.DEFAULT;
+    return false;
   }
 
   @Override