[pull] master from mcMMO-Dev:master

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

@@ -434,13 +434,7 @@ public void onPlayerFishMonitor(PlayerFishEvent event) {
         Entity caught = event.getCaught();
         FishingManager fishingManager = UserManager.getPlayer(player).getFishingManager();
 
-        //Track the hook
         if (ExperienceConfig.getInstance().isFishingExploitingPrevented()) {
-            if (event.getHook().getMetadata(MetadataConstants.METADATA_KEY_FISH_HOOK_REF).size()
-                    == 0) {
-                fishingManager.setFishHookReference(event.getHook());
-            }
-
             //Spam Fishing
             if (event.getState() == PlayerFishEvent.State.CAUGHT_FISH
                     && fishingManager.isFishingTooOften()) {

src/main/java/com/gmail/nossr50/skills/fishing/FishingManager.java

@@ -24,7 +24,6 @@
 import com.gmail.nossr50.util.BlockUtils;
 import com.gmail.nossr50.util.EventUtils;
 import com.gmail.nossr50.util.ItemUtils;
-import com.gmail.nossr50.util.MetadataConstants;
 import com.gmail.nossr50.util.Misc;
 import com.gmail.nossr50.util.Permissions;
 import com.gmail.nossr50.util.adapter.BiomeAdapter;
@@ -53,15 +52,14 @@
 import org.bukkit.inventory.ItemStack;
 import org.bukkit.inventory.PlayerInventory;
 import org.bukkit.inventory.meta.SkullMeta;
-import org.bukkit.metadata.Metadatable;
 import org.bukkit.util.BoundingBox;
 import org.bukkit.util.Vector;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
 public class FishingManager extends SkillManager {
-    private long fishHookSpawnTimestamp = 0L;
-    private long lastWarned = 0L;
+    protected long lastFishCaughtTimestamp = 0L;
+    protected long lastWarned = 0L;
     private BoundingBox lastFishingBoundingBox;
     private boolean sameTarget;
     private int fishCaughtCounter = 1;
@@ -90,27 +88,21 @@ public boolean canMasterAngler() {
                 && Permissions.isSubSkillEnabled(getPlayer(), SubSkillType.FISHING_MASTER_ANGLER);
     }
 
-    public void setFishHookReference(Metadatable fishHook) {
-        if (!fishHook.getMetadata(MetadataConstants.METADATA_KEY_FISH_HOOK_REF).isEmpty()) {
-            return;
-        }
-
-        fishHook.setMetadata(MetadataConstants.METADATA_KEY_FISH_HOOK_REF,
-                MetadataConstants.MCMMO_METADATA_VALUE);
-        fishHookSpawnTimestamp = System.currentTimeMillis();
-    }
-
+    /**
+     * {@return whether the player has had a previous catch within the last second}
+     */
     public boolean isFishingTooOften() {
         long currentTime = System.currentTimeMillis();
-        long fishHookSpawnCD = fishHookSpawnTimestamp + 1000;
-        boolean hasFished = (currentTime < fishHookSpawnCD);
+        boolean hasFishedRecently = lastFishCaughtTimestamp + 1000 > currentTime;
 
-        if (hasFished && (lastWarned + (1000) < currentTime)) {
+        if (hasFishedRecently && currentTime > lastWarned + 1000) {
             getPlayer().sendMessage(LocaleLoader.getString("Fishing.Scared"));
-            lastWarned = System.currentTimeMillis();
+            lastWarned = currentTime;
         }
 
-        return hasFished;
+        lastFishCaughtTimestamp = currentTime;
+
+        return hasFishedRecently;
     }
 
     public void processExploiting(Vector centerOfCastVector) {

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

@@ -40,7 +40,6 @@ public class MetadataConstants {
     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";
     public static final @NotNull String METADATA_KEY_EXPLOSION_FROM_RUPTURE = "mcMMO: Rupture Explosion";
-    public static final @NotNull String METADATA_KEY_FISH_HOOK_REF = "mcMMO: Fish Hook Tracker";
     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";

src/test/java/com/gmail/nossr50/skills/fishing/FishingTest.java

@@ -0,0 +1,57 @@
+package com.gmail.nossr50.skills.fishing;
+
+import com.gmail.nossr50.MMOTestEnvironment;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
+
+import java.util.logging.Logger;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+
+public class FishingTest extends MMOTestEnvironment {
+    private static final Logger LOGGER = Logger.getLogger(FishingTest.class.getName());
+
+    private FishingManager fishingManager;
+
+    @BeforeEach
+    void setUp() {
+        mockBaseEnvironment(LOGGER);
+
+        fishingManager = Mockito.spy(new FishingManager(mmoPlayer));
+    }
+
+    @AfterEach
+    void tearDown() {
+        cleanUpStaticMocks();
+    }
+
+    @Test
+    void testExploitFishingTooOften() {
+        assertFalse(fishingManager.isFishingTooOften());
+        assertFalse(fishingManager.lastWarned > 0);
+
+        // Since we called the method again within a second, this will now consider the player to be fishing too often.
+        assertTrue(fishingManager.isFishingTooOften());
+
+        // Ensure that the player was warned about the exploit fishing.
+        verify(player, times(1)).sendMessage(anyString());
+        final long lastWarningTime = fishingManager.lastWarned;
+        assertTrue(lastWarningTime > 0);
+
+        // Still fishing too often, but make sure another warning doesn't get sent
+        assertTrue(fishingManager.isFishingTooOften());
+        assertEquals(lastWarningTime, fishingManager.lastWarned);
+        verify(player, times(1)).sendMessage(anyString()); // still only called from the previous invocation
+
+        // Manually decrement the last catch timestamp to simulate time passing
+        fishingManager.lastFishCaughtTimestamp -= 1000;
+        assertFalse(fishingManager.isFishingTooOften());
+    }
+}