From 024285389a54e8c9b563ff19671390f2f8c8815a Mon Sep 17 00:00:00 2001 From: Loacky <117084283+LoackyBit@users.noreply.github.com> Date: Fri, 21 Nov 2025 23:25:41 +0100 Subject: [PATCH] Implement donation troops/gold between human players after forming an alliance (#2450) Resolves #2448 Hi team, I've implemented and locally tested the alliance-related changes (including unit tests and some manual simulation with multiple browser profiles). Unfortunately I wasn't able to perform full end-to-end testing on the live game server with two separate machines/accounts. If someone on the team (or another contributor) can verify the alliance flow with two real players, that would be greatly appreciated before merging. Happy to hop on a call or provide any clarification needed. Thanks! ## Description: Fixed a race condition bug where donations (troops/gold) between human players failed after forming an alliance. The issue was caused by a one-tick delay in `AllianceRequestReplyExecution`: the alliance acceptance logic ran in `tick()` instead of `init()`, meaning the alliance wasn't created until the tick after the execution was added. If a donation execution was added in the same turn as the alliance acceptance, it would fail the `isFriendly()` check because the alliance didn't exist yet. **Root cause:** When human players formed alliances via reply, the execution model delayed alliance creation by one tick, while bots called `accept()` directly without this delay. **Solution:** Moved alliance acceptance logic from `tick()` to `init()` in `AllianceRequestReplyExecution.ts`, ensuring immediate alliance creation and eliminating race conditions with donations. **Changes:** - Modified `src/core/execution/alliance/AllianceRequestReplyExecution.ts` to process alliance replies in `init()` instead of `tick()` - Added comprehensive test suite `tests/AllianceDonation.test.ts` with 5 test cases covering donation scenarios after alliance formation (reply and mutual request flows) - All existing tests pass (323 total) ## Please complete the following: - [x] I have added screenshots for all UI updates (N/A - backend logic fix only) - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file (N/A - no user-facing text changes) - [x] I have added relevant tests to the test directory - [x] I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced Discord: loacky GitHub: @LoackyBit --------- Co-authored-by: Evan --- .../alliance/AllianceRequestReplyExecution.ts | 7 +- tests/AllianceDonation.test.ts | 139 ++++++++++++++++++ 2 files changed, 141 insertions(+), 5 deletions(-) create mode 100644 tests/AllianceDonation.test.ts diff --git a/src/core/execution/alliance/AllianceRequestReplyExecution.ts b/src/core/execution/alliance/AllianceRequestReplyExecution.ts index bd3d90a58..b1a79fc44 100644 --- a/src/core/execution/alliance/AllianceRequestReplyExecution.ts +++ b/src/core/execution/alliance/AllianceRequestReplyExecution.ts @@ -19,12 +19,7 @@ export class AllianceRequestReplyExecution implements Execution { return; } this.requestor = mg.player(this.requestorID); - } - tick(ticks: number): void { - if (this.requestor === null) { - throw new Error("Not initialized"); - } if (this.requestor.isFriendly(this.recipient)) { console.warn("already allied"); } else { @@ -46,6 +41,8 @@ export class AllianceRequestReplyExecution implements Execution { this.active = false; } + tick(ticks: number): void {} + isActive(): boolean { return this.active; } diff --git a/tests/AllianceDonation.test.ts b/tests/AllianceDonation.test.ts new file mode 100644 index 000000000..22cab40df --- /dev/null +++ b/tests/AllianceDonation.test.ts @@ -0,0 +1,139 @@ +import { AllianceRequestExecution } from "../src/core/execution/alliance/AllianceRequestExecution"; +import { AllianceRequestReplyExecution } from "../src/core/execution/alliance/AllianceRequestReplyExecution"; +import { DonateGoldExecution } from "../src/core/execution/DonateGoldExecution"; +import { Game, Player, PlayerType } from "../src/core/game/Game"; +import { playerInfo, setup } from "./util/Setup"; + +let game: Game; +let player1: Player; +let player2: Player; + +describe("Alliance Donation", () => { + beforeEach(async () => { + game = await setup( + "plains", + { + infiniteGold: false, + instantBuild: true, + infiniteTroops: false, + donateGold: true, + donateTroops: true, + }, + [ + playerInfo("player1", PlayerType.Human), + playerInfo("player2", PlayerType.Human), + ], + ); + + player1 = game.player("player1"); + player1.conquer(game.ref(0, 0)); + player1.addGold(1000n); + player1.addTroops(1000); + + player2 = game.player("player2"); + player2.conquer(game.ref(0, 1)); + player2.addGold(100n); + player2.addTroops(100); + + while (game.inSpawnPhase()) { + game.executeNextTick(); + } + }); + + test("Can donate gold after alliance formed by reply", () => { + game.addExecution(new AllianceRequestExecution(player1, player2.id())); + game.executeNextTick(); + + game.addExecution( + new AllianceRequestReplyExecution(player1.id(), player2, true), + ); + game.executeNextTick(); + + expect(player1.isAlliedWith(player2)).toBeTruthy(); + expect(player2.isAlliedWith(player1)).toBeTruthy(); + expect(player1.isFriendly(player2)).toBeTruthy(); + expect(player2.isFriendly(player1)).toBeTruthy(); + + expect(player1.canDonateGold(player2)).toBeTruthy(); + const goldBefore = player2.gold(); + const success = player1.donateGold(player2, 100n); + expect(success).toBeTruthy(); + expect(player2.gold()).toBe(goldBefore + 100n); + }); + + test("Can donate troops after alliance formed by reply", () => { + game.addExecution(new AllianceRequestExecution(player1, player2.id())); + game.executeNextTick(); + + game.addExecution( + new AllianceRequestReplyExecution(player1.id(), player2, true), + ); + game.executeNextTick(); + + expect(player1.isAlliedWith(player2)).toBeTruthy(); + expect(player2.isAlliedWith(player1)).toBeTruthy(); + + expect(player1.canDonateTroops(player2)).toBeTruthy(); + const troopsBefore = player2.troops(); + const success = player1.donateTroops(player2, 100); + expect(success).toBeTruthy(); + expect(player2.troops()).toBe(troopsBefore + 100); + }); + + test("Can donate gold after alliance formed by mutual request", () => { + game.addExecution(new AllianceRequestExecution(player1, player2.id())); + game.executeNextTick(); + + game.addExecution(new AllianceRequestExecution(player2, player1.id())); + game.executeNextTick(); + + expect(player1.isAlliedWith(player2)).toBeTruthy(); + expect(player2.isAlliedWith(player1)).toBeTruthy(); + expect(player1.isFriendly(player2)).toBeTruthy(); + expect(player2.isFriendly(player1)).toBeTruthy(); + + expect(player1.canDonateGold(player2)).toBeTruthy(); + const goldBefore = player2.gold(); + const success = player1.donateGold(player2, 100n); + expect(success).toBeTruthy(); + expect(player2.gold()).toBe(goldBefore + 100n); + }); + + test("Can donate troops after alliance formed by mutual request", () => { + game.addExecution(new AllianceRequestExecution(player1, player2.id())); + game.executeNextTick(); + + game.addExecution(new AllianceRequestExecution(player2, player1.id())); + game.executeNextTick(); + + expect(player1.isAlliedWith(player2)).toBeTruthy(); + expect(player2.isAlliedWith(player1)).toBeTruthy(); + + expect(player1.canDonateTroops(player2)).toBeTruthy(); + const troopsBefore = player2.troops(); + const success = player1.donateTroops(player2, 100); + expect(success).toBeTruthy(); + expect(player2.troops()).toBe(troopsBefore + 100); + }); + + test("Can donate immediately after accepting alliance (race condition)", () => { + game.addExecution(new AllianceRequestExecution(player1, player2.id())); + game.executeNextTick(); + + const goldBefore = player2.gold(); + game.addExecution( + new AllianceRequestReplyExecution(player1.id(), player2, true), + ); + game.addExecution(new DonateGoldExecution(player1, player2.id(), 100)); + + game.executeNextTick(); + + expect(player1.isAlliedWith(player2)).toBeTruthy(); + expect(player2.isAlliedWith(player1)).toBeTruthy(); + + game.executeNextTick(); + + // Donation should have succeeded + expect(player2.gold()).toBe(goldBefore + 100n); + }); +});