diff --git a/docs/ALLIED_VICTORY_BATTLE_RESOLUTION.md b/docs/ALLIED_VICTORY_BATTLE_RESOLUTION.md index 38d1ea6942..fc459a5e0e 100644 --- a/docs/ALLIED_VICTORY_BATTLE_RESOLUTION.md +++ b/docs/ALLIED_VICTORY_BATTLE_RESOLUTION.md @@ -8,12 +8,12 @@ processing to player-facing aftermath decisions. When Eagle sends a battle to Shardok via `RequestBattlesAction`, each player gets specific victory conditions: -| Player | Victory Conditions | -|-------------|-------------------------------------------------------| -| Attacker(s) | `HoldsCriticalTiles`, `LastPlayerStanding` | -| Defender | `LastPlayerStanding`, `WinAfterMaxRounds` | +| Player | Victory Conditions | +|-------------|-----------------------------------------------------------------------| +| Attacker(s) | `HoldsCriticalTiles`, `LastPlayerStanding`, `LastAllianceStanding` | +| Defender | `LastPlayerStanding`, `WinAfterMaxRounds` | -Attackers do **not** receive `LastAllianceStanding`. This is set in `RequestBattlesAction.shardokPlayers`. +This is set in `RequestBattlesAction.shardokPlayers`. Alliance information is sent separately in `PlayerSetupInfo.allies`. Eagle reads each player's `factionRelationships` and includes all non-HOSTILE factions as allies @@ -36,8 +36,9 @@ If **multiple** players survive, check if they form a winning alliance: - ALL survivors must be **mutually** allied (every pair checks both directions) - If yes: all survivors are winners -**In practice, this never triggers for AssaultProvince battles** because attackers don't receive -`LastAllianceStanding`. It exists for other battle types. +This triggers for AssaultProvince battles when allied attackers eliminate all defenders. Since the +defender does **not** have `LastAllianceStanding`, the check correctly requires that only the +allied attackers remain alive. ### Check 3: WinAfterMaxRounds (end-of-round only) @@ -49,7 +50,11 @@ If the round counter exceeds `max_rounds`: ### Check 4: HoldsCriticalTiles (end-of-round only) **Single-player castle control**: If one player with `HoldsCriticalTiles` occupies ALL critical -tiles with hero-bearing units, that player wins alone. +tiles with hero-bearing units, that player wins. Additionally, any other player who: +- Has `HoldsCriticalTiles` +- Is **mutually** allied with the castle holder + +is also added to `winning_shardok_ids` as a co-winner. **Allied castle control**: If multiple players collectively occupy all critical tiles with hero-bearing units, AND: @@ -64,6 +69,14 @@ Shardok assigns each player a `Victory` or `Loss`. All winners receive `Victory` `AllyVictory` distinction. The `EndGameCondition` proto reserves fields 2 (`ally_victory`) and 3 (`draw`) for backwards compatibility but Eagle rejects both. +`GameOverResponsePopulator` validates that any player allied to a winner is also in +`winning_shardok_ids`. If an ally is missing from the winners list, it throws an internal error. + +**Known issue**: This validation is too strict for `LastPlayerStanding`. When one allied attacker +is the sole survivor, their dead co-attacker is allied to the winner but legitimately not in +`winning_shardok_ids` (they're dead). The throw should be changed to `Loss` for dead allies, or +the check should only apply when the ally has surviving units. + | Condition | Criterion | |-----------|-----------| | `Victory(type)` | Player is in `winning_shardok_ids` | @@ -77,11 +90,15 @@ In standard AssaultProvince battles: 1. **Allied attackers jointly hold all castles** -> both get `Victory(HoldsCriticalTiles)`. -2. **One attacker holds all castles alone** -> only that attacker gets `Victory(HoldsCriticalTiles)`; - their ally gets `Loss(HoldsCriticalTiles)`. +2. **One attacker holds all castles alone, ally has `HoldsCriticalTiles` and is mutually allied** + -> both get `Victory(HoldsCriticalTiles)`. The ally is added to `winning_shardok_ids` by + `UpdateGameStatusAction`. -When allied attackers jointly eliminate the defender but don't hold all castles, the game -continues until max rounds (defender wins via `WinAfterMaxRounds`). See "Potential Issues" below. +3. **Allied attackers eliminate all defenders** -> both get `Victory(LastAllianceStanding)`. + This triggers because both have `LastAllianceStanding` and are mutually allied. + +4. **One attacker holds all castles, co-attacker is NOT allied** -> only the castle holder + gets `Victory(HoldsCriticalTiles)`; the non-allied co-attacker gets `Loss`. ## 4. Eagle Processes Battle Results @@ -194,49 +211,64 @@ Once all claimants have decided (`FinalizeAftermathAction`): - Players: Largest army picks first: Keep or Withdraw. Last player auto-keeps if no one chose Keep. ### One attacker holds all castles, allied attacker doesn't -- Shardok: Castle-holder gets `Victory(HoldsCriticalTiles)`, ally gets - `Loss(HoldsCriticalTiles)` (only the holder is in `winning_shardok_ids`) -- Eagle: Single attacker won -> ProvinceConqueredAction. Allied attacker's units are captured - along with the defender's. +- Shardok: Castle-holder gets `Victory(HoldsCriticalTiles)`. Mutually-allied co-attacker with + `HoldsCriticalTiles` is added to `winning_shardok_ids` -> also gets `Victory(HoldsCriticalTiles)`. +- Eagle: Both are attacking winners -> MultiVictorBattleSetupAction -> Battle Aftermath. +- Players: Largest army picks first: Keep or Withdraw. -### Allied attackers eliminate all defenders (LastPlayerStanding impossible with 2 survivors) -- Shardok: LastAllianceStanding check fails (attackers don't have this condition). - Game continues until max rounds -> defender wins via `WinAfterMaxRounds` (even if dead). -- Eagle: Defender won -> ProvinceHeldAction. Province stays with defender. -- Players: The attack effectively fails. Attacker units are captured. +### Allied attackers eliminate all defenders +- Shardok: Both attackers have `LastAllianceStanding` and are mutually allied -> both get + `Victory(LastAllianceStanding)`. +- Eagle: Both are attacking winners -> MultiVictorBattleSetupAction -> Battle Aftermath. +- Players: Largest army picks first: Keep or Withdraw. -### Single attacker wins (any condition) +### Non-allied co-attackers: one holds all castles +- Shardok: Castle-holder gets `Victory(HoldsCriticalTiles)`, non-allied co-attacker gets + `Loss(HoldsCriticalTiles)`. +- Eagle: Single attacker won -> ProvinceConqueredAction. Non-allied co-attacker's units are + captured along with the defender's. + +### One allied attacker is sole survivor (LastPlayerStanding) +- Shardok: Sole survivor gets `Victory(LastPlayerStanding)`. Dead ally should get `Loss`. +- **BUG**: `GameOverResponsePopulator` currently throws because dead ally is allied to winner + but not in `winning_shardok_ids`. Needs fix — see known issue in Section 3. +- Eagle (once fixed): Single attacker won -> ProvinceConqueredAction. Dead ally's units were + already destroyed in battle. + +### Single attacker wins (any condition, no co-attackers) - No alliance considerations. ProvinceConqueredAction immediately. ### Defender wins (WinAfterMaxRounds or LastPlayerStanding) - ProvinceHeldAction. All non-returned attacker units are captured. -## 7. Potential Issues +## 7. Edge Cases and Known Issues -### Allied attackers can't win by elimination alone +### BUG: Dead ally causes crash via LastPlayerStanding -Since attackers receive `LastPlayerStanding` but not `LastAllianceStanding`, two allied attackers -who destroy the entire defending army **cannot win**. The game continues until max rounds, at which -point the (dead) defender wins via `WinAfterMaxRounds`. The attackers must capture all critical -tiles to actually win. +When one allied attacker is the sole survivor (both the co-attacker and defender are dead), +`LastPlayerStanding` fires and only the survivor is in `winning_shardok_ids`. The +`GameOverResponsePopulator` then sees the dead co-attacker is allied to the winner but not in +`winning_shardok_ids`, and throws `ShardokInternalErrorException`. Fix: dead allies should +receive `Loss`, not trigger a validation error. -This is probably intentional -- it forces allied attackers to achieve a concrete tactical objective -(castle control) rather than simply killing troops. But it means that on maps with castles, even -total elimination of the enemy doesn't count as victory for allied attackers. +### Non-allied co-attackers eliminate all defenders -### One allied attacker wins via castles, the other loses +Two non-allied attackers who both survive after eliminating the defender cannot win via +`LastAllianceStanding` (they aren't mutually allied). The game continues until max rounds, +at which point the defender wins via `WinAfterMaxRounds`. They must capture all critical tiles. -When one attacker holds all castles alone, only that attacker gets `Victory`. Their allied -co-attacker gets `Loss` and their units are captured -- the same as the defender's units. This -is correct behavior (the ally didn't achieve a victory condition), but may feel surprising to -players. The aftermath decision only triggers when both attackers are winners. +### Non-allied co-attackers: one holds all castles + +Only the castle holder wins. The non-allied co-attacker gets `Loss` and their units are captured +along with the defender's. The mutual-alliance check in `UpdateGameStatusAction` prevents +non-allied players from being added to `winning_shardok_ids`. ## Key File References | Component | File | |-----------|------| | Victory condition checking | `src/main/cpp/net/eagle0/shardok/library/actions/UpdateGameStatusAction.cpp` | -| Per-player EndGameCondition | `src/main/cpp/net/eagle0/shardok/server/EagleInterfaceGrpcServer.cpp` | +| Per-player EndGameCondition | `src/main/cpp/net/eagle0/shardok/server/GameOverResponsePopulator.cpp` | | Battle setup (victory conditions) | `src/main/scala/.../library/actions/impl/action/RequestBattlesAction.scala` | | Alliance communication | `src/main/scala/.../shardok_interface/ShardokInterfaceGrpcClient.scala:75-80` | | Battle result processing | `src/main/scala/.../library/actions/impl/action/ResolveBattleAction.scala` | diff --git a/src/main/scala/net/eagle0/eagle/library/actions/impl/action/ResolveBattleAction.scala b/src/main/scala/net/eagle0/eagle/library/actions/impl/action/ResolveBattleAction.scala index 086b4db1bd..4ade109726 100644 --- a/src/main/scala/net/eagle0/eagle/library/actions/impl/action/ResolveBattleAction.scala +++ b/src/main/scala/net/eagle0/eagle/library/actions/impl/action/ResolveBattleAction.scala @@ -492,6 +492,8 @@ case class ResolveBattleAction( ) } else { // Multiple allied attackers won -- defer province assignment + val destroyed = destroyedBattalionIds(battleResolution).toSet + val notFledDefenders = (for { sp <- battle.players.filterNot(p => winningShardokPlayers.exists(_.eagleFid == p.eagleFid)) rsp = battleResolution.resolvedPlayers @@ -508,7 +510,7 @@ case class ResolveBattleAction( .filterNot(_.status == UnitStatus.Outlawed) } yield AftermathClaimantUnit( heroId = u.hero.id, - battalionId = u.optionalBattalionId + battalionId = u.optionalBattalionId.filterNot(destroyed.contains) )).toVector val claimants = attackingWinners.map { wsp => @@ -520,6 +522,7 @@ case class ResolveBattleAction( val armySize = notFledUnits .flatMap(_.battalion) + .filter(b => !destroyed.contains(b.id)) .map(_.size) .sum @@ -528,8 +531,12 @@ case class ResolveBattleAction( AftermathClaimant( factionId = wsp.eagleFid, - units = - notFledUnits.map(u => AftermathClaimantUnit(heroId = u.hero.id, battalionId = u.optionalBattalionId)), + units = notFledUnits.map(u => + AftermathClaimantUnit( + heroId = u.hero.id, + battalionId = u.optionalBattalionId.filterNot(destroyed.contains) + ) + ), armySize = armySize, decision = None, broughtGold = broughtGold,