mirror of
https://github.com/nolen777/eagle0.git
synced 2026-07-28 20:55:44 +00:00
Filter destroyed battalion IDs from aftermath claimants and update doc (#6437)
Captured units' battalions are destroyed during BattleEnded processing, but the multi-victor branch was storing those destroyed IDs in AftermathClaimantUnit. This caused invalid battalion references when claimants withdrew. Filter them out when building claimants and notFledDefenders, and exclude them from armySize calculation. Also update ALLIED_VICTORY_BATTLE_RESOLUTION.md to reflect PR 6429 changes (LastAllianceStanding for attackers, allied co-attacker added to winners for solo castle holder) and document the dead-ally GameOverResponsePopulator bug fixed in PR 6435. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -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` |
|
||||
|
||||
+10
-3
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user