Memory leak fix for arrow metadata

Changelog.txt

@@ -1,3 +1,7 @@
+Version 2.1.169
+    Fixed a few memory leaks involving arrows
+    Fixed mcMMO inappropriately assigning metadata to projectiles not fired from players
+
 Version 2.1.168
     Fixed an IndexOutOfBoundsException error when trying to access UserBlockTracker from an invalid range (thanks t00thpick1)
     (API) UserBlockTracker is now the interface by which our block-tracker will be known (thanks t00thpick1)

pom.xml

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

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

@@ -123,24 +123,26 @@ public class EntityListener implements Listener {
                 if(!WorldGuardManager.getInstance().hasMainFlag(player))
                     return;
             }
-        }
 
-        Entity projectile = event.getProjectile();
+            Entity projectile = event.getProjectile();
 
-        //Should be noted that there are API changes regarding Arrow from 1.13.2 to current versions of the game
-        if (!(projectile instanceof Arrow)) {
-            return;
-        }
+            //Should be noted that there are API changes regarding Arrow from 1.13.2 to current versions of the game
+            if (!(projectile instanceof Arrow)) {
+                return;
+            }
 
-        ItemStack bow = event.getBow();
+            ItemStack bow = event.getBow();
 
-        if (bow != null
-                && bow.containsEnchantment(Enchantment.ARROW_INFINITE)) {
-            projectile.setMetadata(mcMMO.infiniteArrowKey, mcMMO.metadataValue);
-        }
+            if (bow != null
+                    && bow.containsEnchantment(Enchantment.ARROW_INFINITE)) {
+                projectile.setMetadata(mcMMO.infiniteArrowKey, mcMMO.metadataValue);
+            }
 
-        projectile.setMetadata(mcMMO.bowForceKey, new FixedMetadataValue(pluginRef, Math.min(event.getForce() * AdvancedConfig.getInstance().getForceMultiplier(), 1.0)));
-        projectile.setMetadata(mcMMO.arrowDistanceKey, new FixedMetadataValue(pluginRef, projectile.getLocation()));
+            projectile.setMetadata(mcMMO.bowForceKey, new FixedMetadataValue(pluginRef, Math.min(event.getForce() * AdvancedConfig.getInstance().getForceMultiplier(), 1.0)));
+            projectile.setMetadata(mcMMO.arrowDistanceKey, new FixedMetadataValue(pluginRef, projectile.getLocation()));
+            //Cleanup metadata in 1 minute in case normal collection falls through
+            CombatUtils.cleanupArrowMetadata((Projectile) projectile);
+        }
     }
 
     @EventHandler(priority = EventPriority.MONITOR, ignoreCancelled = true)
@@ -164,6 +166,8 @@ public class EntityListener implements Listener {
             EntityType entityType = projectile.getType();
 
             if(entityType == EntityType.ARROW || entityType == EntityType.SPECTRAL_ARROW) {
+                CombatUtils.delayArrowMetaCleanup(projectile); //Cleans up metadata 1 minute from now in case other collection methods fall through
+
                 if(!projectile.hasMetadata(mcMMO.bowForceKey))
                     projectile.setMetadata(mcMMO.bowForceKey, new FixedMetadataValue(pluginRef, 1.0));
 
@@ -199,7 +203,6 @@ public class EntityListener implements Listener {
         Entity entity = event.getEntity();
         Material notYetReplacedType = block.getState().getType(); //because its from getState() this is the block that hasn't been changed yet, which is likely air/lava/water etc
 
-
         // When the event is fired for the falling block that changes back to a
         // normal block
         // event.getBlock().getType() returns AIR
@@ -418,13 +421,14 @@ public class EntityListener implements Listener {
             LivingEntity livingEntity = (LivingEntity) entityDamageEvent.getEntity();
 
             if(entityDamageEvent.getFinalDamage() >= livingEntity.getHealth()) {
-
-                /*
-                 * This sets entity names back to whatever they are supposed to be
-                 */
+                //This sets entity names back to whatever they are supposed to be
                 CombatUtils.fixNames(livingEntity);
-                }
             }
+        }
+
+        if(entityDamageEvent.getDamager() instanceof Projectile) {
+            CombatUtils.cleanupArrowMetadata((Projectile) entityDamageEvent.getDamager());
+        }
     }
 
     public boolean checkParties(Cancellable event, Player defendingPlayer, Player attackingPlayer) {

src/main/java/com/gmail/nossr50/skills/archery/ArcheryManager.java

@@ -55,8 +55,7 @@ public class ArcheryManager extends SkillManager {
     public double distanceXpBonusMultiplier(LivingEntity target, Entity arrow) {
         //Hacky Fix - some plugins spawn arrows and assign them to players after the ProjectileLaunchEvent fires
         if(!arrow.hasMetadata(mcMMO.arrowDistanceKey))
-            return arrow.getLocation().distance(target.getLocation());
-
+            return 1;
 
         Location firedLocation = (Location) arrow.getMetadata(mcMMO.arrowDistanceKey).get(0).value();
         Location targetLocation = target.getLocation();

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

@@ -25,16 +25,19 @@ import com.gmail.nossr50.util.compat.layers.persistentdata.MobMetaFlagType;
 import com.gmail.nossr50.util.player.NotificationManager;
 import com.gmail.nossr50.util.player.UserManager;
 import com.google.common.collect.ImmutableMap;
+import org.bukkit.Bukkit;
 import org.bukkit.GameMode;
 import org.bukkit.Material;
 import org.bukkit.attribute.Attribute;
 import org.bukkit.attribute.AttributeInstance;
+import org.bukkit.enchantments.Enchantment;
 import org.bukkit.entity.*;
 import org.bukkit.event.entity.EntityDamageByEntityEvent;
 import org.bukkit.event.entity.EntityDamageEvent;
 import org.bukkit.event.entity.EntityDamageEvent.DamageCause;
 import org.bukkit.event.entity.EntityDamageEvent.DamageModifier;
 import org.bukkit.inventory.ItemStack;
+import org.bukkit.metadata.FixedMetadataValue;
 import org.bukkit.metadata.MetadataValue;
 import org.bukkit.potion.PotionEffectType;
 import org.bukkit.projectiles.ProjectileSource;
@@ -269,6 +272,7 @@ public final class CombatUtils {
 
         //Make sure the profiles been loaded
         if(mcMMOPlayer == null) {
+            cleanupArrowMetadata(arrow);
             return;
         }
 
@@ -309,6 +313,8 @@ public final class CombatUtils {
                 "Force Multiplier: "+forceMultiplier,
                 "Initial Damage: "+initialDamage,
                 "Final Damage: "+finalDamage);
+        //Clean data
+        cleanupArrowMetadata(arrow);
     }
 
     /**
@@ -428,6 +434,9 @@ public final class CombatUtils {
 
                 if (!Misc.isNPCEntityExcludingVillagers(player) && PrimarySkillType.ARCHERY.getPermissions(player)) {
                     processArcheryCombat(target, player, event, arrow);
+                } else {
+                    //Cleanup Arrow
+                    cleanupArrowMetadata(arrow);
                 }
 
                 if (target.getType() != EntityType.CREEPER && !Misc.isNPCEntityExcludingVillagers(player) && PrimarySkillType.TAMING.getPermissions(player)) {
@@ -1059,4 +1068,32 @@ public final class CombatUtils {
             attributeInstance.setBaseValue(normalSpeed * multiplier);
         }
     }
+
+    /**
+     * Clean up metadata from a projectile
+     *
+     * @param entity projectile
+     */
+    public static void cleanupArrowMetadata(@NotNull Projectile entity) {
+        if(entity.hasMetadata(mcMMO.infiniteArrowKey)) {
+            entity.removeMetadata(mcMMO.infiniteArrowKey, mcMMO.p);
+        }
+
+        if(entity.hasMetadata(mcMMO.bowForceKey)) {
+            entity.removeMetadata(mcMMO.bowForceKey, mcMMO.p);
+        }
+
+        if(entity.hasMetadata(mcMMO.arrowDistanceKey)) {
+            entity.removeMetadata(mcMMO.arrowDistanceKey, mcMMO.p);
+        }
+    }
+
+    /**
+     * Clean up metadata from a projectile after a minute has passed
+     *
+     * @param entity the projectile
+     */
+    public static void delayArrowMetaCleanup(@NotNull Projectile entity) {
+        Bukkit.getServer().getScheduler().runTaskLater(mcMMO.p, () -> { cleanupArrowMetadata(entity);}, 20*60);
+    }
 }