Fix untag/death inconsistencies

Author: ChanceSD

pvpmanager/src/main/java/me/chancesd/pvpmanager/command/Tag.java

@@ -49,6 +49,11 @@ public void execute(final CommandSender sender, final String label, final List<C
 		final CommandArgument targetArg = getArgument(args, ARG_TARGET);
 		final CommandArgument timeArg = getArgument(args, ARG_TIME);
 		final long timeInMs = timeArg.getAsInt() * 1000L;
+		
+		if (timeInMs <= 0) {
+			ChatUtils.send(sender, Lang.PREFIX + " &#FF5555Combat time must be positive (greater than 0)");
+			return;
+		}
 
 		if (targetArg.isWildcard()) {
 			tagAll(sender, timeInMs);

pvpmanager/src/main/java/me/chancesd/pvpmanager/player/CombatPlayer.java

@@ -45,8 +45,8 @@ public class CombatPlayer extends EcoPlayer {
 	private boolean lastDeathWasPvP;
 	private long toggleTime;
 	private long respawnTime;
-	private long taggedTime;
-	private long totalTagTime;
+	private volatile long taggedTime;
+	private volatile long totalTagTime;
 	private long lastKillCommandTime;
 	private NewbieTask newbieTask;
 	private CombatPlayer enemy;
@@ -100,6 +100,7 @@ public boolean isEnemyOf(final CombatPlayer enemyPlayer) {
 	public void addEnemy(final CombatPlayer enemyPlayer) {
 		if (enemyPlayer == this)
 			return;
+		this.enemy = enemyPlayer;
 		this.lastHitters.add(enemyPlayer);
 	}
 
@@ -157,11 +158,11 @@ public final void tag(final boolean isAttacker, final CombatPlayer other, final
 		}
 
 		this.taggedTime = System.currentTimeMillis();
-		this.enemy = other;
 		addEnemy(other);
 
-		if (tagged)
+		if (tagged) {
 			return;
+		}
 
 		this.totalTagTime = timeMiliseconds;
 
@@ -207,6 +208,10 @@ public final void tag(final boolean isAttacker, final CombatPlayer other) {
 	 * Takes the player out of combat
 	 */
 	public final void untag(final UntagReason reason) {
+		if (!isInCombat()) {
+			Log.debug("Not untagging " + getName() + " because player is not tagged.");
+			return;
+		}
 		final PlayerUntagEvent event = new PlayerUntagEvent(getPlayer(), this, reason);
 		ScheduleUtils.ensureMainThread(() -> {
 			Bukkit.getPluginManager().callEvent(event);

pvpmanager/src/main/java/me/chancesd/pvpmanager/setting/LogFile.java

@@ -63,7 +63,7 @@ public final void logCombatLog(final CombatPlayer player) {
 			locationInfo = " | Location:Unknown";
 		}
 
-		final String data = player.getName() + " tried to escape combat! (" + player.getTagTimeLeft() / 1000 + "s left)"
+		final String data = player.getName() + " tried to escape combat! (" + String.format("%.3f", player.getTagTimeLeft() / 1000.0) + "s left)"
 				+ " | In combat with: " + player.getEnemies().stream().map(CombatPlayer::getName).toList() + locationInfo;
 		write(format.format(new Date()) + data);
 	}

pvpmanager/src/main/java/me/chancesd/pvpmanager/tasks/NewbieTask.java

@@ -76,12 +76,12 @@ private void startBossBarDisplay() {
 		final TimeProgressSource timeProgressSource = new TimeProgressSource() {
 			@Override
 			public long getGoal() {
-				return getDuration() / 1000;
+				return getDuration();
 			}
 
 			@Override
-			public double getProgress() {
-				return (getDuration() - getTimeleft()) / 1000.0;
+			public long getProgress() {
+				return getDuration() - getTimeleft();
 			}
 		};
 

pvpmanager/src/main/java/me/chancesd/pvpmanager/tasks/TagTask.java

@@ -63,12 +63,12 @@ private final void startTracking(final CombatPlayer combatPlayer) {
 		final TimeProgressSource timeProgressSource = new TimeProgressSource() {
 			@Override
 			public long getGoal() {
-				return combatPlayer.getTotalTagTime() / 1000;
+				return combatPlayer.getTotalTagTime();
 			}
 
 			@Override
-			public double getProgress() {
-				return (System.currentTimeMillis() - combatPlayer.getTaggedTime()) / 1000D;
+			public long getProgress() {
+				return System.currentTimeMillis() - combatPlayer.getTaggedTime();
 			}
 		};
 
@@ -82,13 +82,16 @@ public double getProgress() {
 		}
 		if (Conf.BOSS_BAR_ENABLED.asBool()) {
 			builder.withBossBar(bossBar.build(), timeSource -> {
+				final double secondsRemaining = (timeSource.getGoal() - timeSource.getProgress()) / 1000.0;
 				final String message = Conf.BOSS_BAR_MESSAGE.asString().replace("<time>",
-						Double.toString(Utils.roundTo1Decimal(timeSource.getGoal() - timeSource.getProgress())));
+						Double.toString(Utils.roundTo1Decimal(secondsRemaining)));
 				return CombatUtils.processPlaceholders(combatPlayer.getPlayer(), message);
 			});
 		}
 		final CountdownData countdownData = builder.withTimeSource(timeProgressSource)
-				.onFinish(() -> combatPlayer.untag(UntagReason.TIME_EXPIRED))
+				.onFinish(() -> {
+					combatPlayer.untag(UntagReason.TIME_EXPIRED);
+				})
 				.build(combatPlayer.getPlayer());
 
 		display.createCountdown(combatPlayer.getPlayer(), countdownData);

pvpmanager/src/test/java/me/chancesd/pvpmanager/listener/PlayerListenerTest.java

@@ -120,19 +120,22 @@ final void regularDeath() {
 
 	@Test
 	final void inCombatDeath() {
+		// Test 1: Default config (UNTAG_ON_KILL=false, SELF_TAG=false)
 		tagPlayer(combatDefender, combatAttacker);
 		tagPlayer(combatAttacker, combatDefender);
 		listener.onPlayerDeath(createDeathEvent(defender));
 		assertFalse(combatDefender.isInCombat());
 		assertTrue(combatAttacker.isInCombat());
 
+		// Test 2: UNTAG_ON_KILL=true
 		Conf.UNTAG_ON_KILL.set(true);
 		tagPlayer(combatDefender, combatAttacker);
 		tagPlayer(combatAttacker, combatDefender);
 		listener.onPlayerDeath(createDeathEvent(defender));
 		assertFalse(combatDefender.isInCombat());
 		assertFalse(combatAttacker.isInCombat());
 
+		// Test 3: SELF_TAG=true
 		Conf.SELF_TAG.set(true);
 		tagPlayer(combatAttacker, combatAttacker);
 		tagPlayer(combatDefender, combatAttacker);
@@ -142,4 +145,25 @@ final void inCombatDeath() {
 		assertFalse(combatAttacker.isInCombat());
 	}
 
+	@Test
+	final void deathEdgeCases() {
+		Conf.SELF_TAG.set(false);
+		tagPlayer(combatDefender, combatAttacker);
+		PlayerDeathEvent event = mock(PlayerDeathEvent.class);
+		when(event.getEntity()).thenReturn(defender);
+		when(defender.getKiller()).thenReturn(defender);
+		
+		listener.onPlayerDeath(event);
+		assertFalse(combatDefender.isInCombat(), "Player should be untagged after self-kill");
+		
+		// Test 2: Environmental death (killer = null)
+		tagPlayer(combatDefender, combatAttacker);
+		event = mock(PlayerDeathEvent.class);
+		when(event.getEntity()).thenReturn(defender);
+		when(defender.getKiller()).thenReturn(null);
+		
+		listener.onPlayerDeath(event);
+		assertFalse(combatDefender.isInCombat(), "Player should be untagged after environmental death");
+	}
+
 }