minor refactoring of ProjectileUtils and MetadataConstants

Fixes #5153 Fixes #5144

Changelog.txt

@@ -1,3 +1,6 @@
+Version 2.2.034
+    Fixed bug where arrow would award archery xp after a crossbow trickshot bounce
+
 Version 2.2.033
     Added Breeze_Rod entries to potions.yml for Awkward potion (see notes)
     Added missing TURTLE_HELMET entry to potions.yml for Tier 1 ingredients

pom.xml

@@ -2,7 +2,7 @@
     <modelVersion>4.0.0</modelVersion>
     <groupId>com.gmail.nossr50.mcMMO</groupId>
     <artifactId>mcMMO</artifactId>
-    <version>2.2.033-SNAPSHOT</version>
+    <version>2.2.034-SNAPSHOT</version>
     <name>mcMMO</name>
     <url>https://github.com/mcMMO-Dev/mcMMO</url>
     <scm>

src/main/java/com/gmail/nossr50/listeners/EntityListener.java

@@ -21,6 +21,7 @@ import com.gmail.nossr50.util.player.NotificationManager;
 import com.gmail.nossr50.util.player.UserManager;
 import com.gmail.nossr50.util.random.ProbabilityUtil;
 import com.gmail.nossr50.util.skills.CombatUtils;
+import com.gmail.nossr50.util.skills.ProjectileUtils;
 import com.gmail.nossr50.worldguard.WorldGuardManager;
 import com.gmail.nossr50.worldguard.WorldGuardUtils;
 import org.bukkit.ChatColor;
@@ -1090,7 +1091,7 @@ public class EntityListener implements Listener {
             return;
 
         if (event.getEntity() instanceof Arrow arrow) {
-            if (arrow.isShotFromCrossbow()) {
+            if (ProjectileUtils.isCrossbowProjectile(arrow)) {
                 Crossbows.processCrossbows(event, pluginRef, arrow);
             }
         }

src/main/java/com/gmail/nossr50/skills/crossbows/CrossbowsManager.java

@@ -20,7 +20,10 @@ import org.bukkit.projectiles.ProjectileSource;
 import org.bukkit.util.Vector;
 import org.jetbrains.annotations.NotNull;
 
+import static com.gmail.nossr50.util.MetadataConstants.MCMMO_METADATA_VALUE;
+import static com.gmail.nossr50.util.MetadataConstants.METADATA_KEY_CROSSBOW_PROJECTILE;
 import static com.gmail.nossr50.util.skills.CombatUtils.delayArrowMetaCleanup;
+import static com.gmail.nossr50.util.skills.ProjectileUtils.isCrossbowProjectile;
 
 public class CrossbowsManager extends SkillManager {
     public CrossbowsManager(McMMOPlayer mmoPlayer) {
@@ -28,7 +31,7 @@ public class CrossbowsManager extends SkillManager {
     }
 
     public void handleRicochet(@NotNull Plugin pluginRef, @NotNull Arrow arrow, @NotNull Vector hitBlockNormal) {
-        if (!arrow.isShotFromCrossbow())
+        if (!isCrossbowProjectile(arrow))
             return;
 
         // Check player permission
@@ -79,6 +82,12 @@ public class CrossbowsManager extends SkillManager {
                 new FixedMetadataValue(pluginRef, bounceCount + 1));
         spawnedArrow.setMetadata(MetadataConstants.METADATA_KEY_SPAWNED_ARROW,
                 new FixedMetadataValue(pluginRef, originalArrowShooter));
+        // Easy fix to recognize the arrow as a crossbow projectile
+        // TODO: Replace the hack with the new API for setting weapon on projectiles
+        if (!spawnedArrow.hasMetadata(METADATA_KEY_CROSSBOW_PROJECTILE)) {
+            spawnedArrow.setMetadata(MetadataConstants.METADATA_KEY_CROSSBOW_PROJECTILE, MCMMO_METADATA_VALUE);
+        }
+        // There are reasons to keep this despite using the metadata values above
         spawnedArrow.setShotFromCrossbow(true);
 
         // Don't allow multi-shot or infinite arrows to be picked up

src/main/java/com/gmail/nossr50/util/MetadataConstants.java

@@ -1,19 +1,42 @@
 package com.gmail.nossr50.util;
 
-import com.google.common.collect.ImmutableSet;
 import org.bukkit.metadata.FixedMetadataValue;
 import org.jetbrains.annotations.NotNull;
 
-import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
 
 /**
  * Stores our constants related to metadata
  */
 public class MetadataConstants {
-    /* Metadata Values
-     * Take great care if you ever modify the value of these keys
-     */
+    public static final @NotNull Set<String> MOB_METADATA_KEYS = Set.of(
+        MetadataConstants.METADATA_KEY_MOB_SPAWNER_MOB,
+        MetadataConstants.METADATA_KEY_EGG_MOB,
+        MetadataConstants.METADATA_KEY_NETHER_PORTAL_MOB,
+        MetadataConstants.METADATA_KEY_COTW_SUMMONED_MOB,
+        MetadataConstants.METADATA_KEY_PLAYER_BRED_MOB,
+        MetadataConstants.METADATA_KEY_PLAYER_TAMED_MOB,
+        MetadataConstants.METADATA_KEY_EXPLOITED_ENDERMEN,
+        MetadataConstants.METADATA_KEY_CUSTOM_NAME,
+        MetadataConstants.METADATA_KEY_RUPTURE,
+        MetadataConstants.METADATA_KEY_EXPLOSION_FROM_RUPTURE,
+        MetadataConstants.METADATA_KEY_OLD_NAME_KEY,
+        MetadataConstants.METADATA_KEY_DODGE_TRACKER
+    );
+
+    public static final @NotNull List<String> ARROW_METADATA_KEYS = List.of(
+            MetadataConstants.METADATA_KEY_INF_ARROW,
+            MetadataConstants.METADATA_KEY_BOW_FORCE,
+            MetadataConstants.METADATA_KEY_ARROW_DISTANCE,
+            MetadataConstants.METADATA_KEY_SPAWNED_ARROW,
+            MetadataConstants.METADATA_KEY_MULTI_SHOT_ARROW,
+            MetadataConstants.METADATA_KEY_BOUNCE_COUNT,
+            MetadataConstants.METADATA_KEY_CROSSBOW_PROJECTILE
+    );
+
     public static final @NotNull String METADATA_KEY_REPLANT = "mcMMO: Recently Replanted";
+    public static final @NotNull String METADATA_KEY_CROSSBOW_PROJECTILE = "mcMMO: Crossbow Projectile";
     public static final @NotNull String METADATA_KEY_SPAWNED_ARROW = "mcMMO: Spawned Arrow";
     public static final @NotNull String METADATA_KEY_MULTI_SHOT_ARROW = "mcMMO: Multi-shot Arrow";
     public static final @NotNull String METADATA_KEY_BOUNCE_COUNT = "mcMMO: Arrow Bounce Count";
@@ -22,10 +45,8 @@ public class MetadataConstants {
     public static final @NotNull String METADATA_KEY_DODGE_TRACKER = "mcMMO: Dodge Tracker";
     public static final @NotNull String METADATA_KEY_CUSTOM_DAMAGE = "mcMMO: Custom Damage";
     public static final @NotNull String METADATA_KEY_TRAVELING_BLOCK = "mcMMO: Traveling Block";
-    public static final @NotNull String METADATA_KEY_PISTON_TRACKING = "mcMMO: Piston Tracking";
     public static final @NotNull String METADATA_KEY_TRACKED_TNT = "mcMMO: Tracked TNT";
     public static final @NotNull String METADATA_KEY_NAME_VISIBILITY = "mcMMO: Name Visibility";
-    public static final @NotNull String METADATA_KEY_TRACKED_ITEM = "mcMMO: Tracked Item";
     public static final @NotNull String METADATA_KEY_INF_ARROW = "mcMMO: Infinite Arrow";
     public static final @NotNull String METADATA_KEY_TRACKED_ARROW = "mcMMO: Tracked Arrow";
     public static final @NotNull String METADATA_KEY_BOW_FORCE = "mcMMO: Bow Force";
@@ -49,28 +70,6 @@ public class MetadataConstants {
     public static final @NotNull String METADATA_KEY_CUSTOM_NAME = "mcmmo_custom_name";
     public static final @NotNull String METADATA_KEY_OLD_NAME_KEY = "mcmmo_old_name";
     public static final @NotNull String METADATA_KEY_RUPTURE = "mcmmo_rupture";
-
     public static final byte SIMPLE_FLAG_VALUE = (byte) 0x1;
-
-    public static final @NotNull ImmutableSet<String> MOB_METADATA_KEYS;
-
     public static FixedMetadataValue MCMMO_METADATA_VALUE;
-
-    static {
-        HashSet<String> temp = new HashSet<>();
-        temp.add(MetadataConstants.METADATA_KEY_MOB_SPAWNER_MOB);
-        temp.add(MetadataConstants.METADATA_KEY_EGG_MOB);
-        temp.add(MetadataConstants.METADATA_KEY_NETHER_PORTAL_MOB);
-        temp.add(MetadataConstants.METADATA_KEY_COTW_SUMMONED_MOB);
-        temp.add(MetadataConstants.METADATA_KEY_PLAYER_BRED_MOB);
-        temp.add(MetadataConstants.METADATA_KEY_PLAYER_TAMED_MOB);
-        temp.add(MetadataConstants.METADATA_KEY_EXPLOITED_ENDERMEN);
-        temp.add(MetadataConstants.METADATA_KEY_CUSTOM_NAME);
-        temp.add(MetadataConstants.METADATA_KEY_RUPTURE);
-        temp.add(MetadataConstants.METADATA_KEY_EXPLOSION_FROM_RUPTURE);
-        temp.add(MetadataConstants.METADATA_KEY_OLD_NAME_KEY);
-        temp.add(MetadataConstants.METADATA_KEY_DODGE_TRACKER);
-
-        MOB_METADATA_KEYS = ImmutableSet.copyOf(temp);
-    }
 }

src/main/java/com/gmail/nossr50/util/skills/CombatUtils.java

@@ -39,6 +39,7 @@ import java.util.List;
 import static com.gmail.nossr50.datatypes.experience.XPGainReason.PVP;
 import static com.gmail.nossr50.util.AttributeMapper.MAPPED_MOVEMENT_SPEED;
 import static com.gmail.nossr50.util.MobMetadataUtils.hasMobFlag;
+import static com.gmail.nossr50.util.skills.ProjectileUtils.isCrossbowProjectile;
 
 public final class CombatUtils {
 
@@ -567,9 +568,8 @@ public final class CombatUtils {
             }
         } else if (painSource instanceof Arrow arrow) {
             ProjectileSource projectileSource = arrow.getShooter();
-            boolean isCrossbow = arrow.isShotFromCrossbow();
+            boolean isCrossbow = isCrossbowProjectile(arrow);
             if (projectileSource instanceof Player player) {
-
                 if (!Misc.isNPCEntityExcludingVillagers(player)) {
                     if (!isCrossbow && mcMMO.p.getSkillTools().canCombatSkillsTrigger(PrimarySkillType.ARCHERY, target)) {
                         processArcheryCombat(target, player, event, arrow);

src/main/java/com/gmail/nossr50/util/skills/ProjectileUtils.java

@@ -5,11 +5,16 @@ import com.gmail.nossr50.util.MetadataConstants;
 import org.bukkit.block.BlockFace;
 import org.bukkit.entity.Arrow;
 import org.bukkit.metadata.FixedMetadataValue;
+import org.bukkit.metadata.MetadataValue;
 import org.bukkit.plugin.Plugin;
 import org.bukkit.util.Vector;
 import org.jetbrains.annotations.NotNull;
 
+import static com.gmail.nossr50.util.MetadataConstants.ARROW_METADATA_KEYS;
+import static com.gmail.nossr50.util.MetadataConstants.MCMMO_METADATA_VALUE;
+
 public class ProjectileUtils {
+
     public static Vector getNormal(BlockFace blockFace) {
         return switch (blockFace) {
             case UP -> new Vector(0, 1, 0);
@@ -27,58 +32,37 @@ public class ProjectileUtils {
      *
      * @param arrow projectile
      */
-    // TODO: Add test
     public static void cleanupProjectileMetadata(@NotNull Arrow arrow) {
-        if (arrow.hasMetadata(MetadataConstants.METADATA_KEY_INF_ARROW)) {
-            arrow.removeMetadata(MetadataConstants.METADATA_KEY_INF_ARROW, mcMMO.p);
-        }
-
-        if (arrow.hasMetadata(MetadataConstants.METADATA_KEY_BOW_FORCE)) {
-            arrow.removeMetadata(MetadataConstants.METADATA_KEY_BOW_FORCE, mcMMO.p);
-        }
-
-        if (arrow.hasMetadata(MetadataConstants.METADATA_KEY_ARROW_DISTANCE)) {
-            arrow.removeMetadata(MetadataConstants.METADATA_KEY_ARROW_DISTANCE, mcMMO.p);
-        }
-
-        if (arrow.hasMetadata(MetadataConstants.METADATA_KEY_SPAWNED_ARROW)) {
-            arrow.removeMetadata(MetadataConstants.METADATA_KEY_SPAWNED_ARROW, mcMMO.p);
-        }
-
-        if (arrow.hasMetadata(MetadataConstants.METADATA_KEY_MULTI_SHOT_ARROW)) {
-            arrow.removeMetadata(MetadataConstants.METADATA_KEY_MULTI_SHOT_ARROW, mcMMO.p);
-        }
-
-        if (arrow.hasMetadata(MetadataConstants.METADATA_KEY_BOUNCE_COUNT)) {
-            arrow.removeMetadata(MetadataConstants.METADATA_KEY_BOUNCE_COUNT, mcMMO.p);
-        }
+        ARROW_METADATA_KEYS.stream()
+                .filter(arrow::hasMetadata)
+                .forEach(key -> arrow.removeMetadata(key, mcMMO.p));
     }
 
-    public static void copyArrowMetadata(@NotNull Plugin pluginRef, @NotNull Arrow arrowToCopy, @NotNull Arrow newArrow) {
-        if (arrowToCopy.hasMetadata(MetadataConstants.METADATA_KEY_INF_ARROW)) {
-            newArrow.setMetadata(MetadataConstants.METADATA_KEY_INF_ARROW,
-                    arrowToCopy.getMetadata(MetadataConstants.METADATA_KEY_INF_ARROW).get(0));
-        }
+    /**
+     * Copies metadata from one arrow to another.
+     *
+     * @param pluginRef mcMMO plugin reference.
+     * @param sourceArrow The arrow from which metadata is copied.
+     * @param targetArrow The arrow to which metadata is copied.
+     */
+    public static void copyArrowMetadata(@NotNull Plugin pluginRef, @NotNull Arrow sourceArrow,
+                                         @NotNull Arrow targetArrow) {
+        ARROW_METADATA_KEYS.stream()
+                .filter(sourceArrow::hasMetadata)
+                .forEach(key -> {
+                    final MetadataValue metadataValue = sourceArrow.getMetadata(key).get(0);
+                    if (key.equals(MetadataConstants.METADATA_KEY_BOW_FORCE)) {
+                        targetArrow.setMetadata(key, new FixedMetadataValue(pluginRef, metadataValue.asDouble()));
+                    } else if (key.equals(MetadataConstants.METADATA_KEY_CROSSBOW_PROJECTILE)) {
+                        targetArrow.setMetadata(key, MCMMO_METADATA_VALUE);
+                    } else {
+                        targetArrow.setMetadata(key, metadataValue);
+                    }
+                });
+    }
 
-        if (arrowToCopy.hasMetadata(MetadataConstants.METADATA_KEY_BOW_FORCE)) {
-            newArrow.setMetadata(MetadataConstants.METADATA_KEY_BOW_FORCE,
-                    new FixedMetadataValue(pluginRef,
-                            arrowToCopy.getMetadata(MetadataConstants.METADATA_KEY_BOW_FORCE).get(0).asDouble()));
-        }
-
-        if (arrowToCopy.hasMetadata(MetadataConstants.METADATA_KEY_ARROW_DISTANCE)) {
-            newArrow.setMetadata(MetadataConstants.METADATA_KEY_ARROW_DISTANCE,
-                    arrowToCopy.getMetadata(MetadataConstants.METADATA_KEY_ARROW_DISTANCE).get(0));
-        }
-
-        if (arrowToCopy.hasMetadata(MetadataConstants.METADATA_KEY_SPAWNED_ARROW)) {
-            newArrow.setMetadata(MetadataConstants.METADATA_KEY_SPAWNED_ARROW,
-                    arrowToCopy.getMetadata(MetadataConstants.METADATA_KEY_SPAWNED_ARROW).get(0));
-        }
-
-        if (arrowToCopy.hasMetadata(MetadataConstants.METADATA_KEY_MULTI_SHOT_ARROW)) {
-            newArrow.setMetadata(MetadataConstants.METADATA_KEY_MULTI_SHOT_ARROW,
-                    arrowToCopy.getMetadata(MetadataConstants.METADATA_KEY_MULTI_SHOT_ARROW).get(0));
-        }
+    public static boolean isCrossbowProjectile(@NotNull Arrow arrow) {
+        return arrow.isShotFromCrossbow()
+                || arrow.hasMetadata(MetadataConstants.METADATA_KEY_CROSSBOW_PROJECTILE);
     }
 }