mirror of
https://github.com/openfrontio/OpenFrontIO.git
synced 2026-06-21 06:10:42 +00:00
Resolves #4094 ## Description: In Free-For-All (FFA) mode where teams default to 0, player isOnSameTeam checks returned false for oneself, allowing players to attack themselves. Consequently, if a bot conquered the targeted tile between queueing a transport ship action and its actual initialization, the target became itself, causing the bot to execute a self-invasion. This fix adds a reflexive check in PlayerImpl.ts's isFriendly method to always treat oneself as friendly. It also adds a safety guard in TransportShipExecution.ts's init method to abort ship execution if the target has shifted to the attacker. ## Please complete the following: - [X] I have added screenshots for all UI updates - [X] I process any text displayed to the user through translateText() and I've added it to the en.json file - [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 ## Please put your Discord username so you can be contacted if a bug or regression is found: barfires
This commit is contained in:
@@ -91,6 +91,11 @@ export class TransportShipExecution implements Execution {
|
||||
}
|
||||
}
|
||||
|
||||
if (this.target === this.attacker) {
|
||||
this.active = false;
|
||||
return;
|
||||
}
|
||||
|
||||
if (this.target.isPlayer() && !this.attacker.canAttackPlayer(this.target)) {
|
||||
this.active = false;
|
||||
return;
|
||||
|
||||
@@ -776,6 +776,9 @@ export class PlayerImpl implements Player {
|
||||
}
|
||||
|
||||
canDonateGold(recipient: Player): boolean {
|
||||
if (recipient === this) {
|
||||
return false;
|
||||
}
|
||||
if (
|
||||
!this.isAlive() ||
|
||||
!recipient.isAlive() ||
|
||||
@@ -803,6 +806,9 @@ export class PlayerImpl implements Player {
|
||||
}
|
||||
|
||||
canDonateTroops(recipient: Player): boolean {
|
||||
if (recipient === this) {
|
||||
return false;
|
||||
}
|
||||
if (
|
||||
!this.isAlive() ||
|
||||
!recipient.isAlive() ||
|
||||
@@ -830,6 +836,9 @@ export class PlayerImpl implements Player {
|
||||
}
|
||||
|
||||
donateTroops(recipient: Player, troops: number): boolean {
|
||||
// Defense-in-depth: canDonateTroops already checks this, but guard here too
|
||||
// to prevent self-donation if the method is called directly.
|
||||
if (recipient === this) return false;
|
||||
if (troops <= 0) return false;
|
||||
const removed = this.removeTroops(troops);
|
||||
if (removed === 0) return false;
|
||||
@@ -847,6 +856,9 @@ export class PlayerImpl implements Player {
|
||||
}
|
||||
|
||||
donateGold(recipient: Player, gold: Gold): boolean {
|
||||
// Defense-in-depth: canDonateGold already checks this, but guard here too
|
||||
// to prevent self-donation if the method is called directly.
|
||||
if (recipient === this) return false;
|
||||
if (gold <= 0n) return false;
|
||||
const removed = this.removeGold(gold);
|
||||
if (removed === 0n) return false;
|
||||
@@ -969,6 +981,9 @@ export class PlayerImpl implements Player {
|
||||
}
|
||||
|
||||
isFriendly(other: Player, treatAFKFriendly: boolean = false): boolean {
|
||||
if (other === this) {
|
||||
return true;
|
||||
}
|
||||
if (other.isDisconnected() && !treatAFKFriendly) {
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -527,4 +527,19 @@ describe("Attack immunity", () => {
|
||||
constructionExecution(game, playerA, 0, 11, UnitType.AtomBomb, 3);
|
||||
expect(playerA.units(UnitType.AtomBomb)).toHaveLength(1);
|
||||
});
|
||||
|
||||
test("Should abort TransportShipExecution when target is the attacker itself", async () => {
|
||||
// Wait for spawn immunity to end to ensure it doesn't prematurely abort the execution
|
||||
waitForImmunityToEnd();
|
||||
|
||||
// playerA tries to send a transport ship targeting one of playerA's own tiles (spawn tile at 7, 0)
|
||||
const selfTarget = game.ref(7, 0);
|
||||
const exec = new TransportShipExecution(playerA, selfTarget, 10);
|
||||
game.addExecution(exec);
|
||||
game.executeNextTick();
|
||||
|
||||
// Verify it aborted immediately: active is false, and no transport ship unit spawned
|
||||
expect(exec.isActive()).toBe(false);
|
||||
expect(playerA.units(UnitType.TransportShip)).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -239,3 +239,51 @@ describe("Donate Gold to a non ally", () => {
|
||||
expect(recipient.gold() >= recipientGoldBefore).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("Self donation prevention", () => {
|
||||
it("Should evaluate isFriendly(this) to true but disallow donating to self", async () => {
|
||||
const game = await setup("ocean_and_land", {
|
||||
infiniteGold: false,
|
||||
infiniteTroops: false,
|
||||
donateGold: true,
|
||||
donateTroops: true,
|
||||
});
|
||||
const gameID: GameID = "game_id";
|
||||
|
||||
// Create a player with team=0/null (default/FFA)
|
||||
const playerInfo = new PlayerInfo(
|
||||
"player_self",
|
||||
PlayerType.Human,
|
||||
null,
|
||||
"self_id",
|
||||
);
|
||||
game.addPlayer(playerInfo);
|
||||
|
||||
const player = game.player(playerInfo.id);
|
||||
const spawnA = game.ref(0, 10);
|
||||
|
||||
game.addExecution(new SpawnExecution(gameID, playerInfo, spawnA));
|
||||
game.executeNextTick();
|
||||
|
||||
// Assert player.isFriendly(player) === true
|
||||
expect(player.isFriendly(player)).toBe(true);
|
||||
|
||||
// Assert canDonateGold and canDonateTroops return false for self
|
||||
expect(player.canDonateGold(player)).toBe(false);
|
||||
expect(player.canDonateTroops(player)).toBe(false);
|
||||
|
||||
// Try executing DonateGoldExecution and DonateTroopsExecution on self
|
||||
player.addGold(1000n);
|
||||
player.addTroops(1000);
|
||||
const goldBefore = player.gold();
|
||||
const troopsBefore = player.troops();
|
||||
|
||||
game.addExecution(new DonateGoldExecution(player, player.id(), 500));
|
||||
game.addExecution(new DonateTroopsExecution(player, player.id(), 500));
|
||||
game.executeNextTick();
|
||||
|
||||
// Verify no changes occurred to gold or troops (execution failed/aborted)
|
||||
expect(player.gold()).toBeGreaterThanOrEqual(goldBefore);
|
||||
expect(player.troops()).toBeGreaterThanOrEqual(troopsBefore);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user