-
Notifications
You must be signed in to change notification settings - Fork 761
Fix bots no longer attacking humans 🤖 #2690
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughRemoved 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
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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. Comment |
There was a problem hiding this 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) callsshouldAttack()(line 434), which still applies difficulty-based restrictions to all attacks onPlayerType.Humantargets, 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 checkthis.player.type() === PlayerType.Botand returntruebefore applying difficulty restrictions.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 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
evanpelle
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
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:
Please put your Discord username so you can be contacted if a bug or regression is found:
FloPinguin