Skip to content
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions src/client/ClientGameRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -943,7 +943,8 @@ export class ClientGameRunner {
this.eventBus.emit(
new SendAttackIntentEvent(
this.gameView.owner(tile).id(),
this.myPlayer!.troops() * this.renderer.uiState.attackRatio,
this.renderer.uiState.attackRatio,
this.myPlayer!.troops(),
),
);
} else if (this.canAutoBoat(actions.buildableUnits, tile)) {
Expand Down Expand Up @@ -1102,7 +1103,8 @@ export class ClientGameRunner {
this.eventBus.emit(
new SendAttackIntentEvent(
this.gameView.owner(tile).id(),
this.myPlayer!.troops() * this.renderer.uiState.attackRatio,
this.renderer.uiState.attackRatio,
this.myPlayer!.troops(),
),
);
}
Expand Down Expand Up @@ -1136,12 +1138,14 @@ export class ClientGameRunner {
mostRecentAttack.attackerID,
) as PlayerView;
if (!attacker) return;

const counterTroops = Math.min(
mostRecentAttack.troops,
this.renderer.uiState.attackRatio * this.myPlayer.troops(),
this.eventBus.emit(
new SendAttackIntentEvent(
attacker.id(),
this.renderer.uiState.attackRatio,
this.myPlayer.troops(),
mostRecentAttack.troops,
),
);
this.eventBus.emit(new SendAttackIntentEvent(attacker.id(), counterTroops));
}

private doRequestAllianceUnderCursor(): void {
Expand Down Expand Up @@ -1226,7 +1230,8 @@ export class ClientGameRunner {
this.eventBus.emit(
new SendBoatAttackIntentEvent(
tile,
this.myPlayer.troops() * this.renderer.uiState.attackRatio,
this.renderer.uiState.attackRatio,
this.myPlayer.troops(),
),
);
}
Expand Down
14 changes: 10 additions & 4 deletions src/client/Transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,17 @@ export class SendSpawnIntentEvent implements GameEvent {
export class SendAttackIntentEvent implements GameEvent {
constructor(
public readonly targetID: PlayerID | null,
public readonly troops: number,
public readonly troopRatio: number,
public readonly troopCount: number,
public readonly maxTroopCount: number | null = null,
) {}
}

export class SendBoatAttackIntentEvent implements GameEvent {
constructor(
public readonly dst: TileRef,
public readonly troops: number,
public readonly troopRatio: number,
public readonly troopCount: number,
) {}
}

Expand Down Expand Up @@ -484,14 +487,17 @@ export class Transport {
this.sendIntent({
type: "attack",
targetID: event.targetID,
troops: event.troops,
troopRatio: event.troopRatio,
troopCount: event.troopCount,
maxTroopSent: event.maxTroopCount ?? undefined,
});
}

private onSendBoatAttackIntent(event: SendBoatAttackIntentEvent) {
this.sendIntent({
type: "boat",
troops: event.troops,
troopRatio: event.troopRatio,
troopCount: event.troopCount,
dst: event.dst,
});
}
Expand Down
12 changes: 7 additions & 5 deletions src/client/hud/layers/AttacksDisplay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,12 +200,14 @@ export class AttacksDisplay extends LitElement implements Controller {

const myPlayer = this.game.myPlayer();
if (!myPlayer) return;

const counterTroops = Math.min(
attack.troops,
this.uiState.attackRatio * myPlayer.troops(),
this.eventBus.emit(
new SendAttackIntentEvent(
attacker.id(),
this.uiState.attackRatio,
myPlayer.troops(),
attack.troops,
),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
);
this.eventBus.emit(new SendAttackIntentEvent(attacker.id(), counterTroops));
}

private renderIncomingAttacks() {
Expand Down
6 changes: 4 additions & 2 deletions src/client/hud/layers/PlayerActionHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ export class PlayerActionHandler {
this.eventBus.emit(
new SendAttackIntentEvent(
targetId,
this.uiState.attackRatio * player.troops(),
this.uiState.attackRatio,
player.troops(),
),
);
}
Expand All @@ -36,7 +37,8 @@ export class PlayerActionHandler {
this.eventBus.emit(
new SendBoatAttackIntentEvent(
targetTile,
this.uiState.attackRatio * player.troops(),
this.uiState.attackRatio,
player.troops(),
),
);
}
Expand Down
8 changes: 6 additions & 2 deletions src/core/Schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,10 @@ export const AllianceExtensionIntentSchema = z.object({
export const AttackIntentSchema = z.object({
type: z.literal("attack"),
targetID: ID.nullable(),
troops: z.number().nonnegative().nullable(),
troopCount: z.number().nonnegative(),
troopRatio: z.number().gt(0).max(1),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
// maxTroopSent present only for retaliation.
maxTroopSent: z.number().nonnegative().optional(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's read the file and understand the schema structure
head -n 380 src/core/Schemas.ts | tail -n 30

Repository: openfrontio/OpenFrontIO

Length of output: 910


🏁 Script executed:

sed -n '370,380p' src/core/Schemas.ts

Repository: openfrontio/OpenFrontIO

Length of output: 419


🏁 Script executed:

# Search for all troopCount and similar troop-related integer fields in the file
rg "troopCount|maxTroopSent|troop" src/core/Schemas.ts -n

Repository: openfrontio/OpenFrontIO

Length of output: 535


🏁 Script executed:

# Let's see the context around line 426 to understand the donate_troops schema
sed -n '422,430p' src/core/Schemas.ts

Repository: openfrontio/OpenFrontIO

Length of output: 398


🏁 Script executed:

# Check if .int() is used elsewhere in Zod schemas in this file
rg "\.int\(" src/core/Schemas.ts

Repository: openfrontio/OpenFrontIO

Length of output: 668


🏁 Script executed:

# Let's verify the broader context - check the actual structure of these schemas and ensure the .int() placement is correct
sed -n '360,380p' src/core/Schemas.ts

Repository: openfrontio/OpenFrontIO

Length of output: 732


🏁 Script executed:

# Search for tests related to these schemas to see if any tests validate decimal troop counts
find . -name "*.test.ts" -o -name "*.spec.ts" | head -20

Repository: openfrontio/OpenFrontIO

Length of output: 1168


🏁 Script executed:

# Search for tests of schemas, especially AttackIntentSchema or BoatAttackIntentSchema
find . -path "*/tests/*" -name "*[Ss]chema*.test.ts" -exec grep -l "AttackIntent\|BoatAttackIntent\|troopCount" {} \;

Repository: openfrontio/OpenFrontIO

Length of output: 161


🏁 Script executed:

# Search for tests that validate intent schemas
rg "AttackIntentSchema|BoatAttackIntentSchema" tests/ --type ts

Repository: openfrontio/OpenFrontIO

Length of output: 161


🏁 Script executed:

# Let's check if there are any usages of AttackIntentSchema or BoatAttackIntentSchema that might show how troopCount is used
rg "AttackIntentSchema|BoatAttackIntentSchema|donate_troops" src/ --type ts -A 2 -B 2 | head -50

Repository: openfrontio/OpenFrontIO

Length of output: 2568


🏁 Script executed:

# Check where these intents are created to see if they're ever given decimal troopCount values
rg "troopCount|donate_troops|troops:" src/core/ --type ts -B 2 -A 2 | head -80

Repository: openfrontio/OpenFrontIO

Length of output: 3916


🏁 Script executed:

# Let's look at the broader context in ExecutionManager to see how troopCount is used
sed -n '1,50p' src/core/execution/ExecutionManager.ts | head -30

Repository: openfrontio/OpenFrontIO

Length of output: 1976


🏁 Script executed:

# Look at the exact calculation with Math.floor to understand the intent
rg "Math.floor.*troopRatio.*troopCount" src/core/execution/ExecutionManager.ts -B 5 -A 5

Repository: openfrontio/OpenFrontIO

Length of output: 631


🏁 Script executed:

# Let's check the DonateTroopIntentSchema and see if troops should also be an integer
sed -n '422,428p' src/core/Schemas.ts

Repository: openfrontio/OpenFrontIO

Length of output: 317


Validate troop counts as integers.

troopCount and maxTroopSent represent troop counts but the schema accepts decimals like 12.5. Use .int() to tighten the wire contract.

Suggested schema changes
-  troopCount: z.number().nonnegative(),
+  troopCount: z.number().int().nonnegative(),
   troopRatio: z.number().gt(0).max(1),
   // maxTroopSent present only for retaliation.
-  maxTroopSent: z.number().nonnegative().optional(),
+  maxTroopSent: z.number().int().nonnegative().optional(),
-  troopCount: z.number().nonnegative(),
+  troopCount: z.number().int().nonnegative(),
   troopRatio: z.number().gt(0).max(1),

Consider also applying the same fix to troops in DonateTroopIntentSchema (line 426).

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
troopCount: z.number().nonnegative(),
troopRatio: z.number().gt(0).max(1),
// maxTroopSent present only for retaliation.
maxTroopSent: z.number().nonnegative().optional(),
troopCount: z.number().int().nonnegative(),
troopRatio: z.number().gt(0).max(1),
// maxTroopSent present only for retaliation.
maxTroopSent: z.number().int().nonnegative().optional(),
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/Schemas.ts` around lines 362 - 365, The troopCount and maxTroopSent
fields in this schema currently accept decimal numbers (like 12.5), but troop
counts must be integers. Add the .int() constraint to the troopCount field
(which has .nonnegative()) and to the maxTroopSent field (which has
.nonnegative().optional()) to tighten the wire contract. Additionally, apply the
same .int() constraint to the troops field in DonateTroopIntentSchema at line
426 for consistency.

Comment thread
Katokoda marked this conversation as resolved.
Outdated
});

export const SpawnIntentSchema = z.object({
Expand All @@ -369,7 +372,8 @@ export const SpawnIntentSchema = z.object({

export const BoatAttackIntentSchema = z.object({
type: z.literal("boat"),
troops: z.number().nonnegative(),
troopCount: z.number().nonnegative(),
troopRatio: z.number().gt(0).max(1),
dst: z.number(),
});

Expand Down
63 changes: 59 additions & 4 deletions src/core/execution/ExecutionManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,57 @@ export class Executor {
this.random = new PseudoRandom(simpleHash(gameID) + 1);
}

private computeRatio(
remainingTroopRatio: number,
totalRatioUsage: number,
): number {
return (1 - remainingTroopRatio) / totalRatioUsage;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

createExecs(turn: Turn): Execution[] {
return turn.intents.map((i) => this.createExec(i));
// In the rare case a client sends multiple troopRatio-orders,
// we need to "merge" their orders instead of executing them in parallel.
// (two 60% attacks should be one 84% attack, not one 120% attack)
// But, they may be of different types/on different targets
// (hence we do two (84/120)*60% = 42% attacks).
const remainingTroopRatio_perClientID = new Map<ClientID, number>();
const totalRatioUsage_perClientID = new Map<ClientID, number>();
for (const intent of turn.intents) {
switch (intent.type) {
case "boat":
case "attack": {
remainingTroopRatio_perClientID.set(
intent.clientID,
(remainingTroopRatio_perClientID.get(intent.clientID) ?? 1) *
(1 - intent.troopRatio),
);
totalRatioUsage_perClientID.set(
intent.clientID,
(totalRatioUsage_perClientID.get(intent.clientID) ?? 0) +
intent.troopRatio,
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
}
}

return turn.intents.map((intent) => {
switch (intent.type) {
case "boat":
case "attack":
return this.createExec(
intent,
this.computeRatio(
remainingTroopRatio_perClientID.get(intent.clientID)!,
totalRatioUsage_perClientID.get(intent.clientID)!,
),
);
default:
return this.createExec(intent, undefined);
}
});
}

createExec(intent: StampedIntent): Execution {
createExec(intent: StampedIntent, troopRatioFactor?: number): Execution {
const player = this.mg.playerByClientID(intent.clientID);
if (!player) {
console.warn(`player with clientID ${intent.clientID} not found`);
Expand All @@ -57,7 +103,12 @@ export class Executor {
switch (intent.type) {
case "attack": {
return new AttackExecution(
intent.troops,
Math.floor(
Math.min(
troopRatioFactor! * intent.troopRatio * intent.troopCount,
intent.maxTroopSent ?? intent.troopCount,
),
),
player,
intent.targetID,
null,
Expand All @@ -72,7 +123,11 @@ export class Executor {
case "spawn":
return new SpawnExecution(this.gameID, player.info(), intent.tile);
case "boat":
return new TransportShipExecution(player, intent.dst, intent.troops);
return new TransportShipExecution(
player,
intent.dst,
Math.floor(troopRatioFactor! * intent.troopRatio * intent.troopCount),
);
case "allianceRequest":
return new AllianceRequestExecution(player, intent.recipient);
case "allianceReject":
Expand Down
12 changes: 6 additions & 6 deletions tests/Attack.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@ let defender: Player;
let defenderSpawn: TileRef;
let attackerSpawn: TileRef;

function sendBoat(target: TileRef, troops: number) {
game.addExecution(new TransportShipExecution(defender, target, troops));
}

const immunityPhaseTicks = 10;
function waitForImmunityToEnd() {
for (let i = 0; i < immunityPhaseTicks + 1; i++) {
Expand Down Expand Up @@ -110,7 +106,9 @@ describe("Attack", () => {
constructionExecution(game, defender, 1, 1, UnitType.MissileSilo);
expect(defender.units(UnitType.MissileSilo)).toHaveLength(1);

sendBoat(game.ref(15, 8), 100);
game.addExecution(
new TransportShipExecution(defender, game.ref(15, 8), 100),
);

constructionExecution(game, defender, 0, 15, UnitType.AtomBomb, 3);
const nuke = defender.units(UnitType.AtomBomb)[0];
Expand All @@ -129,7 +127,9 @@ describe("Attack", () => {
const player_start_troops = defender.troops();
const boat_troops = player_start_troops * 0.5;

sendBoat(game.ref(15, 8), boat_troops);
game.addExecution(
new TransportShipExecution(defender, game.ref(15, 8), boat_troops),
);

game.executeNextTick();

Expand Down
Loading
Loading