Keep war battles lethal for nonlethal attackers

[XxFran10xX]

Problem

If a player in nonlethal mode lands the killing blow in a SimpleFactions war battle, the victim is knocked out instead of dying. SimpleFactions only acts on PlayerDeathEvent, so it never sees the death:

• no battle life is spent (applyDeathAndNeedsJailRespawn, casualty ledger)
• the jail/spawn respawn routing doesn't run

That gives a battle loophole: nonlethal attackers can down enemies without costing them lives.

Fix

PvpKnockoutManager now skips the knockout for players in a started SimpleFactions battle. It uses PermadeathBattleExemption.isInStartedBattle, the roster check that already exempts battle deaths from permadeath and strikes. The killing blow goes through, and SimpleFactions handles the death as usual.

This also covers a player who was downed before the battle started. Once they're in a started battle, falls and other damage with no attacker can kill them again.

The lethal-health check now runs before the skip checks, so the battle lookup only happens on lethal hits, not on every hit.

Tests

• nonlethalBlowStillKillsInStartedBattle: in battle, the blow isn't cancelled, health isn't set and no strike is recorded. Outside battle, the same event still knocks the player out (control).
• playerDownedBeforeBattleStartsCanStillDie
• Both fail on main and pass here. PvpKnockoutManagerTest, PvpStrikeServiceTest and PermadeathBridgesTest all pass locally. The full suite runs in CI; the remaining local failures are Windows-only (POSIX permissions, decimal-comma locale) and also fail on main.

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 02d9c433-b1f2-49af-aec3-75e862b0de9e

📥 Commits

Reviewing files that changed from the base of the PR and between 2df7266 and 62917c0.

📒 Files selected for processing (3)

• README.md
• src/main/java/net/tfminecraft/rpcharacters/pvp/PvpKnockoutManager.java
• src/test/java/net/tfminecraft/rpcharacters/pvp/PvpKnockoutManagerTest.java

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.

---

📝 Summary

Summary by CodeRabbit

• Bug Fixes
  • Players in a started permadeath battle are no longer intercepted by the nonlethal knockout system when taking lethal damage. The battle can record the death and handle the player’s respawn.
  • The exemption applies only while the battle is started and does not prevent lethal damage from affecting a player who was already knocked down.

Walkthrough

Knockout handling now skips players in a started permadeath battle. The lethal-damage handler checks for lethal damage before checking skip conditions. Tests cover exempt and non-exempt victims, including a victim already downed by fall damage. The README describes the battle exception.

Changes

Started Battle Knockout Exemption

Layer / File(s)	Summary
Lethal damage exemption handling src/main/java/net/tfminecraft/rpcharacters/pvp/PvpKnockoutManager.java, src/test/java/net/tfminecraft/rpcharacters/pvp/PvpKnockoutManagerTest.java, README.md	The handler checks whether damage is lethal before checking knockout skip conditions. Started-battle players skip knockout handling. Tests cover lethal hits against exempt victims, and the README describes the exception.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: ryanbarlow97

Merge Risk: ⚪ Minimal · up to 62917

No actionable issue remains in the supplied evidence; the PR is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 62917

The change intentionally keeps started battles lethal and uses the existing battle roster to limit its scope. No introduced security defect was established, but the companion plugin’s death accounting, roster cleanup, and respawn behavior remain unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The locally demonstrated exposure covers lethal damage against any UUID currently reported as a started-battle member, including damage without an attacker. Outside that roster condition, the new exemption is inactive. Potential downstream consequences involve player death, battle lives, respawn routing, and character consequences—not demonstrated infrastructure or credential authority.

Trust Boundaries and Controls

• observed — Battle eligibility is supplied by the server-side battle manager rather than the attacker’s nonlethal setting. Existing creation and pending-permakill exclusions remain ahead of the new gate. Failure to obtain battle eligibility disables the exemption rather than treating an unknown player as a battle member.

Resilience and Maintainability Implications

• inferred — Damage interception and local death consequences use separate live roster lookups, not a shared event-scoped eligibility snapshot. Correct ownership therefore depends on roster state and lookup availability remaining compatible through death processing. The death-time lookup predates this PR; the PR adds knockout-originated deaths to that existing lifecycle. No actual ordering failure was established.

Hardening Proposals

• proposed — Validate the integration with the actual battle death consumer, especially last-life roster removal before local MONITOR handling, exactly-once casualty accounting, respawn routing, lookup failure, and already-downed cleanup. These checks would resolve the ownership uncertainty without assuming a vulnerability exists.

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}