Fix nations being blocked by PVP immunity 🛡️

[FloPinguin]

Description:

Problem

PVP immunity (the extended spawn immunity setting) was incorrectly preventing AI nations from attacking human players. The intent of PVP immunity is to protect human-vs-human combat only, but nations were subject to the same restriction.

Root Cause

In canAttackPlayer(), only PlayerType.Bot was exempt from checking target immunity. Nations fell through to the same path as humans, so when a nation tried to attack an immune human, player.isImmune() returned true and the attack was blocked.

Fix

Changed the immunity bypass condition from this.type() === PlayerType.Bot to this.type() !== PlayerType.Human. Now only human attackers check target immunity. Both bots and nations bypass it (they only check alliance status).

This does not affect nation spawn immunity (nationSpawnImmunityDuration), which is a separate mechanism that protects newly spawned nations from all attackers and continues to work independently.

Please complete the following:

• I have added screenshots for all UI updates
• I process any text displayed to the user through translateText() and I've added it to the en.json file
• I have added relevant tests to the test directory

Please put your Discord username so you can be contacted if a bug or regression is found:

FloPinguin

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 53e23e12-1d62-4101-94b5-c251f72c1cf5

📥 Commits

Reviewing files that changed from the base of the PR and between 0c0a2e1 and ac72c5e.

📒 Files selected for processing (1)

• tests/Attack.test.ts

---

Walkthrough

In PlayerImpl.canAttackPlayer, the attacker-type guard is changed from checking PlayerType.Bot to checking PlayerType.Human. Non-human attackers (bots, nations, and any other non-human type) now skip the isImmune() check entirely and return based only on friendliness. New test cases confirm this behavior across different attacker types and alliance scenarios.

Changes

PVP immunity guard fix

Layer / File(s)	Summary
canAttackPlayer non-Human bypass src/core/game/PlayerImpl.ts	The immunity check (player.isImmune()) is now gated behind this.type() === PlayerType.Human. Any non-human attacker (bot, nation, etc.) skips immunity and returns !isFriendly(...) directly.
PVP immunity test coverage tests/Attack.test.ts	Four test cases verify that during PVP immunity: nations and bots can attack human players, nations can attack other nations, and nations cannot attack allied humans.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

• Players Can Attack Nations in PVP Immunity Without the Nation Attacking Back #4201 — The widened guard directly fixes the case where nation attackers were blocked by the immunity check, allowing nations to attack immune players.

Suggested labels

Bug, small-fix

Suggested reviewers

• evanpelle

Poem

A bot, a nation, under immunity's guard,
One bool flips—and the typecheck becomes less hard.
Non-humans bypass what humans must respect,
Tests affirm it—alliance logic done correct. 🏳️⚔️

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main fix: preventing nations from being incorrectly blocked by PVP immunity.
Description check	✅ Passed	The description provides clear context about the problem, root cause, and solution, all directly related to the changeset.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed due to a network error.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/core/game/PlayerImpl.ts (1)

1607-1616: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add core tests for the new immunity gate behavior.

This src/core/ behavior changed, but no accompanying test change is present in the provided diff. Please add/attach tests covering: (1) Human attacker blocked by immune human target, and (2) Nation/Bot attacker bypassing target PVP immunity while still respecting friendliness.
As per coding guidelines, "All changes to src/core/ must include tests".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/game/PlayerImpl.ts` around lines 1607 - 1616, The canAttackPlayer
method behavior in PlayerImpl was modified to add new immunity gate logic, but
the diff lacks accompanying test coverage. Add tests for the canAttackPlayer
method that verify both scenarios: (1) a Human type attacker cannot attack an
immune Human target (blocked by immunity check), and (2) a Nation or Bot type
attacker can bypass the target's PVP immunity check but still respects
friendliness rules. These tests are required per the coding guideline that all
changes to src/core/ must include tests.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/core/game/PlayerImpl.ts`:
- Around line 1607-1616: The canAttackPlayer method behavior in PlayerImpl was
modified to add new immunity gate logic, but the diff lacks accompanying test
coverage. Add tests for the canAttackPlayer method that verify both scenarios:
(1) a Human type attacker cannot attack an immune Human target (blocked by
immunity check), and (2) a Nation or Bot type attacker can bypass the target's
PVP immunity check but still respects friendliness rules. These tests are
required per the coding guideline that all changes to src/core/ must include
tests.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0f51d5ee-8402-4c32-9813-917ef0e4146a

📥 Commits

Reviewing files that changed from the base of the PR and between 6a8900f and 0c0a2e1.

📒 Files selected for processing (1)

• src/core/game/PlayerImpl.ts