Skip to content

Conversation

@FloPinguin
Copy link
Contributor

@FloPinguin FloPinguin commented Dec 25, 2025

Description:

A small number of people are complaining that bots no longer attack them and that "This has broken the factory farming strat".
In the old #2550 I somehow added that bots on easy difficulty don't attack humans and nations anymore.
And public games are on easy difficulty now.
But I think the difficulty should actually only change nation behavior, not bot behavior. Their attacks are harmless anyways.
So lets remove that little check.
Also let shouldAttack() return true for bots.

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
  • I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced

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

FloPinguin

@FloPinguin FloPinguin requested a review from a team as a code owner December 25, 2025 02:58
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 25, 2025

Walkthrough

Removed difficulty-based exemption for Easy in attackRandomTarget; skip decision now depends only on a random 1-in-2 chance. shouldAttack now treats Terra Nullius, non-humans, traitors, and Bots as unconditional attack targets.

Changes

Cohort / File(s) Summary
AI Attack Behavior
src/core/execution/utils/AiAttackBehavior.ts
Removed Difficulty.Easy check from attackRandomTarget; skip-of-neighbor now uses only a random 1-in-2 chance. Expanded shouldAttack to return true for Terra Nullius, non-humans, traitors, and when the current player is a Bot. Minor comment updates to reflect bot-based unconditional attack behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested labels

Feature - AI

Suggested reviewers

  • evanpelle

Poem

🤖 A random coin, no easy shield,
Bots decide when swords must yield.
Terra Nullius and traitors called,
No comfort for the human-all.
New rules sing — the battlefield. ⚔️

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: enabling bots to attack humans again, which is the primary objective of the changeset.
Description check ✅ Passed The description is directly related to the changeset, explaining the rationale for removing the difficulty-based skip logic and enabling bot attacks.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3b78549 and 9aeeea1.

📒 Files selected for processing (1)
  • src/core/execution/utils/AiAttackBehavior.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/core/execution/utils/AiAttackBehavior.ts

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

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

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

⚠️ Outside diff range comments (2)
src/core/execution/utils/AiAttackBehavior.ts (2)

434-453: Consider exempting Bot-type attackers from difficulty checks.

Based on the PR description stating "difficulty should only affect nation behavior, not bot behavior", this method should check the attacker type and bypass difficulty restrictions when the attacker is a Bot.

🔎 Suggested fix to exempt bots from difficulty checks
 shouldAttack(other: Player | TerraNullius): boolean {
   // Always attack Terra Nullius, non-humans and traitors
   if (
     other.isPlayer() === false ||
     other.type() !== PlayerType.Human ||
     other.isTraitor()
   ) {
     return true;
   }
 
+  // Bots always attack regardless of difficulty
+  if (this.player.type() === PlayerType.Bot) {
+    return true;
+  }
+
   // Prevent attacking of humans on lower difficulties
   const { difficulty } = this.game.config().gameConfig();
   if (difficulty === Difficulty.Easy && this.random.chance(2)) {
     return false;
   }
   if (difficulty === Difficulty.Medium && this.random.chance(4)) {
     return false;
   }
   return true;
 }

391-401: The fix is incomplete—bots still have difficulty-based attack restrictions applied.

The change removes the Easy difficulty check at line 395, but sendAttack() (line 399) calls shouldAttack() (line 434), which still applies difficulty-based restrictions to all attacks on PlayerType.Human targets, regardless of whether the attacker is a Bot or Nation.

Since shouldAttack() does not check the attacker's player type, both Bot and Nation players currently face the same difficulty penalties when attacking humans:

  • Easy: 50% chance to refuse (line 446)
  • Medium: 75% chance to refuse (line 449)

If the intent is for bots to attack regardless of difficulty (as implied by "difficulty should only affect nation behavior, not bot behavior"), then shouldAttack() should check this.player.type() === PlayerType.Bot and return true before applying difficulty restrictions.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 28e22c9 and 3b78549.

📒 Files selected for processing (1)
  • src/core/execution/utils/AiAttackBehavior.ts
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: FloPinguin
Repo: openfrontio/OpenFrontIO PR: 2161
File: src/core/execution/FakeHumanExecution.ts:670-678
Timestamp: 2025-11-01T00:24:33.860Z
Learning: In OpenFrontIO, PlayerType.Bot entities cannot be in teams and do not have friendliness relationships. Only PlayerType.Human and PlayerType.FakeHuman can participate in teams and alliances. Therefore, when targeting bot-owned tiles, friendliness checks like `owner.isFriendly(this.player)` are unnecessary and meaningless for bots.
Learnt from: Foorack
Repo: openfrontio/OpenFrontIO PR: 2141
File: src/client/ClientGameRunner.ts:228-234
Timestamp: 2025-10-08T17:14:49.369Z
Learning: For the window close confirmation feature in `ClientGameRunner.ts`, the troop count requirement (>10,000 troops) from issue #2137 was intentionally removed because it was arbitrary and troop count can be reported as low despite having significant land. The confirmation now triggers for any alive player regardless of troop count.
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:770-795
Timestamp: 2025-10-27T09:47:26.395Z
Learning: In src/core/execution/FakeHumanExecution.ts, the selectSteamrollStopTarget() method intentionally allows MIRV targeting when secondHighest city count is 0 (e.g., nuclear endgame scenarios where structures are destroyed). This is valid game design—"if you can afford it, you're good to go"—and should not be flagged as requiring a guard condition.
📚 Learning: 2025-10-27T09:47:26.395Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:770-795
Timestamp: 2025-10-27T09:47:26.395Z
Learning: In src/core/execution/FakeHumanExecution.ts, the selectSteamrollStopTarget() method intentionally allows MIRV targeting when secondHighest city count is 0 (e.g., nuclear endgame scenarios where structures are destroyed). This is valid game design—"if you can afford it, you're good to go"—and should not be flagged as requiring a guard condition.

Applied to files:

  • src/core/execution/utils/AiAttackBehavior.ts
📚 Learning: 2025-10-18T17:54:01.311Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:172-173
Timestamp: 2025-10-18T17:54:01.311Z
Learning: In src/core/execution/FakeHumanExecution.ts, MIRVs and ground attacks should not be mutually exclusive. The considerMIRV() method should not short-circuit maybeAttack() - both actions can occur in the same tick.

Applied to files:

  • src/core/execution/utils/AiAttackBehavior.ts
📚 Learning: 2025-10-20T20:15:28.858Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:51-51
Timestamp: 2025-10-20T20:15:28.858Z
Learning: In src/core/execution/FakeHumanExecution.ts, game balance constants like MIRV_COOLDOWN_TICKS, MIRV_HESITATION_ODDS, VICTORY_DENIAL_TEAM_THRESHOLD, VICTORY_DENIAL_INDIVIDUAL_THRESHOLD, and STEAMROLL_CITY_GAP_MULTIPLIER are experimental tuning parameters subject to frequent change during balance testing. Do not flag changes to these values as issues or compare them against previous values.

Applied to files:

  • src/core/execution/utils/AiAttackBehavior.ts
📚 Learning: 2025-10-20T11:02:16.969Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:57-57
Timestamp: 2025-10-20T11:02:16.969Z
Learning: In src/core/execution/FakeHumanExecution.ts, the correct MIRV victory denial thresholds are VICTORY_DENIAL_TEAM_THRESHOLD = 0.8 (80% for team games) and VICTORY_DENIAL_INDIVIDUAL_THRESHOLD = 0.65 (65% for individual players), not 0.85 and 0.7 as might be mentioned in some documentation.

Applied to files:

  • src/core/execution/utils/AiAttackBehavior.ts
📚 Learning: 2025-08-23T07:48:19.060Z
Learnt from: ElMelchizedek
Repo: openfrontio/OpenFrontIO PR: 1876
File: src/core/execution/FakeHumanExecution.ts:470-473
Timestamp: 2025-08-23T07:48:19.060Z
Learning: In FakeHumanExecution.ts DefensePost placement logic, returning -Infinity from structureSpawnTileValue when no sampled border tiles neighbor enemies is intentional. The logic samples up to 50 border tiles as a heuristic - if none are adjacent to enemies, it assumes DefensePost placement is unnecessary and aborts the entire placement attempt rather than continuing to evaluate individual tiles.

Applied to files:

  • src/core/execution/utils/AiAttackBehavior.ts

coderabbitai[bot]
coderabbitai bot previously approved these changes Dec 25, 2025
@iiamlewis iiamlewis added the Bug label Dec 25, 2025
@iiamlewis iiamlewis moved this from Triage to Final Review in OpenFront Release Management Dec 25, 2025
@iiamlewis iiamlewis added this to the v29 milestone Dec 25, 2025
@evanpelle evanpelle modified the milestones: v29, v28 Dec 26, 2025
Copy link
Collaborator

@evanpelle evanpelle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@evanpelle evanpelle merged commit 6d2ac30 into openfrontio:main Dec 26, 2025
13 of 15 checks passed
@github-project-automation github-project-automation bot moved this from Final Review to Complete in OpenFront Release Management Dec 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Complete

Development

Successfully merging this pull request may close these issues.

3 participants