From 4b129a2f7f66c67841f5f16c8ec02ae498bedf83 Mon Sep 17 00:00:00 2001 From: Kipstz <140314732+Kipstz@users.noreply.github.com> Date: Tue, 5 Aug 2025 07:22:07 +0200 Subject: [PATCH] Add button for remove building (#1609) ## Description: Added a red delete button with trash can icon to the right-click radial menu that allows players to voluntarily delete their own units. ## 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 - [x] I have read and accepted the CLA agreement (only required once). ## Please put your Discord username so you can be contacted if a bug or regression is found: Kipstz image --------- Co-authored-by: Drills Kibo <59177241+drillskibo@users.noreply.github.com> --- resources/lang/en.json | 7 +- src/client/Transport.ts | 17 +++ src/client/graphics/layers/MainRadialMenu.ts | 4 - .../graphics/layers/PlayerActionHandler.ts | 5 + src/client/graphics/layers/RadialMenu.ts | 14 +- .../graphics/layers/RadialMenuElements.ts | 81 ++++++++++++ src/core/Schemas.ts | 8 ++ src/core/configuration/Config.ts | 1 + src/core/configuration/DefaultConfig.ts | 3 + src/core/execution/DeleteUnitExecution.ts | 81 ++++++++++++ src/core/execution/ExecutionManager.ts | 3 + src/core/game/Game.ts | 2 + src/core/game/GameView.ts | 4 + src/core/game/PlayerImpl.ts | 13 ++ tests/DeleteUnitExecution.test.ts | 123 ++++++++++++++++++ 15 files changed, 359 insertions(+), 7 deletions(-) create mode 100644 src/core/execution/DeleteUnitExecution.ts create mode 100644 tests/DeleteUnitExecution.test.ts diff --git a/resources/lang/en.json b/resources/lang/en.json index ec87195a0..178783d1d 100644 --- a/resources/lang/en.json +++ b/resources/lang/en.json @@ -502,7 +502,8 @@ "accept_alliance": "Accept", "reject_alliance": "Reject", "alliance_renewed": "Your alliance with {name} has been renewed", - "ignore": "Ignore" + "ignore": "Ignore", + "unit_voluntarily_deleted": "Unit voluntarily deleted" }, "unit_info_modal": { "structure_info": "Structure Info", @@ -608,5 +609,9 @@ "redirecting": "You are being redirected...", "not_authorized": "You are not authorized to access this website.", "contact_admin": "If you believe you are seeing this message in error, please contact the website administrator." + }, + "radial_menu": { + "delete_unit_title": "Delete Unit", + "delete_unit_description": "Click to delete the nearest unit" } } diff --git a/src/client/Transport.ts b/src/client/Transport.ts index abd0410ff..76357fd42 100644 --- a/src/client/Transport.ts +++ b/src/client/Transport.ts @@ -132,6 +132,10 @@ export class SendEmbargoIntentEvent implements GameEvent { ) {} } +export class SendDeleteUnitIntentEvent implements GameEvent { + constructor(public readonly unitId: number) {} +} + export class CancelAttackIntentEvent implements GameEvent { constructor(public readonly attackID: string) {} } @@ -237,6 +241,11 @@ export class Transport { this.eventBus.on(MoveWarshipIntentEvent, (e) => { this.onMoveWarshipEvent(e); }); + + this.eventBus.on(SendDeleteUnitIntentEvent, (e) => + this.onSendDeleteUnitIntent(e), + ); + this.eventBus.on(SendKickPlayerIntentEvent, (e) => this.onSendKickPlayerIntent(e), ); @@ -598,6 +607,14 @@ export class Transport { }); } + private onSendDeleteUnitIntent(event: SendDeleteUnitIntentEvent) { + this.sendIntent({ + type: "delete_unit", + clientID: this.lobbyConfig.clientID, + unitId: event.unitId, + }); + } + private onSendKickPlayerIntent(event: SendKickPlayerIntentEvent) { this.sendIntent({ type: "kick_player", diff --git a/src/client/graphics/layers/MainRadialMenu.ts b/src/client/graphics/layers/MainRadialMenu.ts index e3822de4b..002b488b0 100644 --- a/src/client/graphics/layers/MainRadialMenu.ts +++ b/src/client/graphics/layers/MainRadialMenu.ts @@ -163,10 +163,6 @@ export class MainRadialMenu extends LitElement implements Layer { return this.radialMenu.shouldTransform(); } - redraw() { - // No redraw implementation needed - } - closeMenu() { if (this.radialMenu.isMenuVisible()) { this.radialMenu.hideRadialMenu(); diff --git a/src/client/graphics/layers/PlayerActionHandler.ts b/src/client/graphics/layers/PlayerActionHandler.ts index e40a40d3d..1bba3300e 100644 --- a/src/client/graphics/layers/PlayerActionHandler.ts +++ b/src/client/graphics/layers/PlayerActionHandler.ts @@ -7,6 +7,7 @@ import { SendAttackIntentEvent, SendBoatAttackIntentEvent, SendBreakAllianceIntentEvent, + SendDeleteUnitIntentEvent, SendDonateGoldIntentEvent, SendDonateTroopsIntentEvent, SendEmbargoIntentEvent, @@ -99,4 +100,8 @@ export class PlayerActionHandler { handleQuickChat(recipient: PlayerView, chatKey: string, params: any = {}) { this.eventBus.emit(new SendQuickChatEvent(recipient, chatKey, params)); } + + handleDeleteUnit(unitId: number) { + this.eventBus.emit(new SendDeleteUnitIntentEvent(unitId)); + } } diff --git a/src/client/graphics/layers/RadialMenu.ts b/src/client/graphics/layers/RadialMenu.ts index 652bff522..9e81cc7cc 100644 --- a/src/client/graphics/layers/RadialMenu.ts +++ b/src/client/graphics/layers/RadialMenu.ts @@ -2,11 +2,13 @@ import * as d3 from "d3"; import backIcon from "../../../../resources/images/BackIconWhite.svg"; import { EventBus, GameEvent } from "../../../core/EventBus"; import { CloseViewEvent } from "../../InputHandler"; +import { translateText } from "../../Utils"; import { Layer } from "./Layer"; import { CenterButtonElement, MenuElement, MenuElementParams, + TooltipKey, } from "./RadialMenuElements"; export class CloseRadialMenuEvent implements GameEvent { @@ -386,6 +388,8 @@ export class RadialMenu implements Layer { const disabled = this.params === null || d.data.disabled(this.params); if (d.data.tooltipItems && d.data.tooltipItems.length > 0) { this.showTooltip(d.data.tooltipItems); + } else if (d.data.tooltipKeys && d.data.tooltipKeys.length > 0) { + this.showTooltip(d.data.tooltipKeys); } if ( disabled || @@ -1008,7 +1012,7 @@ export class RadialMenu implements Layer { return timeSinceHide >= this.reopenCooldownMs; } - private showTooltip(items: TooltipItem[]) { + private showTooltip(items: TooltipItem[] | TooltipKey[]) { if (!this.tooltipElement) return; this.tooltipElement.innerHTML = ""; @@ -1016,7 +1020,13 @@ export class RadialMenu implements Layer { for (const item of items) { const div = document.createElement("div"); div.className = item.className; - div.textContent = item.text; + + if ("key" in item) { + div.textContent = translateText(item.key, item.params); + } else { + div.textContent = item.text; + } + this.tooltipElement.appendChild(div); } diff --git a/src/client/graphics/layers/RadialMenuElements.ts b/src/client/graphics/layers/RadialMenuElements.ts index 5fd53fe00..f749d9356 100644 --- a/src/client/graphics/layers/RadialMenuElements.ts +++ b/src/client/graphics/layers/RadialMenuElements.ts @@ -22,6 +22,7 @@ import infoIcon from "../../../../resources/images/InfoIcon.svg"; import swordIcon from "../../../../resources/images/SwordIconWhite.svg"; import targetIcon from "../../../../resources/images/TargetIconWhite.svg"; import traitorIcon from "../../../../resources/images/TraitorIconWhite.svg"; +import xIcon from "../../../../resources/images/XIcon.svg"; import { EventBus } from "../../../core/EventBus"; export interface MenuElementParams { @@ -48,12 +49,19 @@ export interface MenuElement { text?: string; fontSize?: string; tooltipItems?: TooltipItem[]; + tooltipKeys?: TooltipKey[]; disabled: (params: MenuElementParams) => boolean; action?: (params: MenuElementParams) => void; // For leaf items that perform actions subMenu?: (params: MenuElementParams) => MenuElement[]; // For non-leaf items that open submenus } +export interface TooltipKey { + key: string; + className: string; + params?: Record; +} + export interface CenterButtonElement { disabled: (params: MenuElementParams) => boolean; action: (params: MenuElementParams) => void; @@ -65,6 +73,7 @@ export const COLORS = { boat: "#3f6ab1", ally: "#53ac75", breakAlly: "#c74848", + delete: "#ff0000", info: "#64748B", target: "#ff0000", attack: "#ff0000", @@ -94,6 +103,7 @@ export enum Slot { Attack = "attack", Ally = "ally", Back = "back", + Delete = "delete", } const infoChatElement: MenuElement = { @@ -404,6 +414,76 @@ export const attackMenuElement: MenuElement = { }, }; +export const deleteUnitElement: MenuElement = { + id: Slot.Delete, + name: "delete", + disabled: (params: MenuElementParams) => { + const tileOwner = params.game.owner(params.tile); + const isLand = params.game.isLand(params.tile); + + if (!tileOwner.isPlayer() || tileOwner.id() !== params.myPlayer.id()) { + return true; + } + + if (!isLand) { + return true; + } + + if (params.game.inSpawnPhase()) { + return true; + } + + if (!params.myPlayer.canDeleteUnit()) { + return true; + } + + const DELETE_SELECTION_RADIUS = 5; + const myUnits = params.myPlayer + .units() + .filter( + (unit) => + params.game.manhattanDist(unit.tile(), params.tile) <= + DELETE_SELECTION_RADIUS, + ); + + return myUnits.length === 0; + }, + icon: xIcon, + color: COLORS.delete, + tooltipKeys: [ + { + key: "radial_menu.delete_unit_title", + className: "title", + }, + { + key: "radial_menu.delete_unit_description", + className: "description", + }, + ], + action: (params: MenuElementParams) => { + const DELETE_SELECTION_RADIUS = 5; + const myUnits = params.myPlayer + .units() + .filter( + (unit) => + params.game.manhattanDist(unit.tile(), params.tile) <= + DELETE_SELECTION_RADIUS, + ); + + if (myUnits.length > 0) { + myUnits.sort( + (a, b) => + params.game.manhattanDist(a.tile(), params.tile) - + params.game.manhattanDist(b.tile(), params.tile), + ); + + params.playerActionHandler.handleDeleteUnit(myUnits[0].id()); + } + + params.closeMenu(); + }, +}; + export const buildMenuElement: MenuElement = { id: Slot.Build, name: "build", @@ -497,6 +577,7 @@ export const rootMenuElement: MenuElement = { if (isOwnTerritory) { menuItems.push(buildMenuElement); + menuItems.push(deleteUnitElement); } else { menuItems.push(attackMenuElement); } diff --git a/src/core/Schemas.ts b/src/core/Schemas.ts index ee06566ed..72cb496c5 100644 --- a/src/core/Schemas.ts +++ b/src/core/Schemas.ts @@ -40,6 +40,7 @@ export type Intent = | MoveWarshipIntent | MarkDisconnectedIntent | UpgradeStructureIntent + | DeleteUnitIntent | KickPlayerIntent; export type AttackIntent = z.infer; @@ -69,6 +70,7 @@ export type MarkDisconnectedIntent = z.infer< export type AllianceExtensionIntent = z.infer< typeof AllianceExtensionIntentSchema >; +export type DeleteUnitIntent = z.infer; export type KickPlayerIntent = z.infer; export type Turn = z.infer; @@ -314,6 +316,11 @@ export const MoveWarshipIntentSchema = BaseIntentSchema.extend({ tile: z.number(), }); +export const DeleteUnitIntentSchema = BaseIntentSchema.extend({ + type: z.literal("delete_unit"), + unitId: z.number(), +}); + export const QuickChatIntentSchema = BaseIntentSchema.extend({ type: z.literal("quick_chat"), recipient: ID, @@ -351,6 +358,7 @@ const IntentSchema = z.discriminatedUnion("type", [ MoveWarshipIntentSchema, QuickChatIntentSchema, AllianceExtensionIntentSchema, + DeleteUnitIntentSchema, KickPlayerIntentSchema, ]); diff --git a/src/core/configuration/Config.ts b/src/core/configuration/Config.ts index a97dc2811..06c2249ea 100644 --- a/src/core/configuration/Config.ts +++ b/src/core/configuration/Config.ts @@ -127,6 +127,7 @@ export interface Config { emojiMessageCooldown(): Tick; emojiMessageDuration(): Tick; donateCooldown(): Tick; + deleteUnitCooldown(): Tick; defaultDonationAmount(sender: Player): number; unitInfo(type: UnitType): UnitInfo; tradeShipGold(dist: number, numPorts: number): Gold; diff --git a/src/core/configuration/DefaultConfig.ts b/src/core/configuration/DefaultConfig.ts index 07aa45a0f..08d2733eb 100644 --- a/src/core/configuration/DefaultConfig.ts +++ b/src/core/configuration/DefaultConfig.ts @@ -556,6 +556,9 @@ export class DefaultConfig implements Config { donateCooldown(): Tick { return 10 * 10; } + deleteUnitCooldown(): Tick { + return 5 * 10; + } emojiMessageDuration(): Tick { return 5 * 10; } diff --git a/src/core/execution/DeleteUnitExecution.ts b/src/core/execution/DeleteUnitExecution.ts new file mode 100644 index 000000000..424130eea --- /dev/null +++ b/src/core/execution/DeleteUnitExecution.ts @@ -0,0 +1,81 @@ +import { Execution, Game, MessageType, Player } from "../game/Game"; + +export class DeleteUnitExecution implements Execution { + private active: boolean = true; + private mg: Game; + + constructor( + private player: Player, + private unitId: number, + ) {} + + activeDuringSpawnPhase(): boolean { + return false; + } + + init(mg: Game, ticks: number) { + if (!this.active) { + return; + } + this.mg = mg; + + const unit = this.player.units().find((u) => u.id() === this.unitId); + if (!unit) { + console.warn( + `SECURITY: unit ${this.unitId} not found or not owned by player ${this.player.displayName()}`, + ); + this.active = false; + return; + } + + if (!unit.isActive()) { + console.warn(`SECURITY: unit ${this.unitId} is not active`); + this.active = false; + return; + } + + const tileOwner = mg.owner(unit.tile()); + if (!tileOwner.isPlayer() || tileOwner.id() !== this.player.id()) { + console.warn( + `SECURITY: unit ${this.unitId} is not on player's territory`, + ); + this.active = false; + return; + } + + if (!mg.isLand(unit.tile())) { + console.warn(`SECURITY: unit ${this.unitId} is not on land`); + this.active = false; + return; + } + + if (mg.inSpawnPhase()) { + console.warn(`SECURITY: cannot delete units during spawn phase`); + this.active = false; + return; + } + + if (!this.player.canDeleteUnit()) { + console.warn(`SECURITY: delete unit cooldown not expired`); + this.active = false; + return; + } + + unit.delete(false); + this.player.recordDeleteUnit(); + + this.mg.displayMessage( + `events_display.unit_voluntarily_deleted`, + MessageType.UNIT_DESTROYED, + this.player.id(), + ); + + this.active = false; + } + + tick(ticks: number) {} + + isActive(): boolean { + return this.active; + } +} diff --git a/src/core/execution/ExecutionManager.ts b/src/core/execution/ExecutionManager.ts index f841249db..67bd7c92d 100644 --- a/src/core/execution/ExecutionManager.ts +++ b/src/core/execution/ExecutionManager.ts @@ -10,6 +10,7 @@ import { AttackExecution } from "./AttackExecution"; import { BoatRetreatExecution } from "./BoatRetreatExecution"; import { BotSpawner } from "./BotSpawner"; import { ConstructionExecution } from "./ConstructionExecution"; +import { DeleteUnitExecution } from "./DeleteUnitExecution"; import { DonateGoldExecution } from "./DonateGoldExecution"; import { DonateTroopsExecution } from "./DonateTroopExecution"; import { EmbargoExecution } from "./EmbargoExecution"; @@ -107,6 +108,8 @@ export class Executor { case "upgrade_structure": return new UpgradeStructureExecution(player, intent.unitId); + case "delete_unit": + return new DeleteUnitExecution(player, intent.unitId); case "quick_chat": return new QuickChatExecution( player, diff --git a/src/core/game/Game.ts b/src/core/game/Game.ts index ccabf233d..9b5422acb 100644 --- a/src/core/game/Game.ts +++ b/src/core/game/Game.ts @@ -592,6 +592,8 @@ export interface Player { canDonate(recipient: Player): boolean; donateTroops(recipient: Player, troops: number): boolean; donateGold(recipient: Player, gold: Gold): boolean; + canDeleteUnit(): boolean; + recordDeleteUnit(): void; // Embargo hasEmbargoAgainst(other: Player): boolean; diff --git a/src/core/game/GameView.ts b/src/core/game/GameView.ts index 0956542fa..a73efceb7 100644 --- a/src/core/game/GameView.ts +++ b/src/core/game/GameView.ts @@ -355,6 +355,10 @@ export class PlayerView { isDisconnected(): boolean { return this.data.isDisconnected; } + + canDeleteUnit(): boolean { + return true; + } } export class GameView implements GameMap { diff --git a/src/core/game/PlayerImpl.ts b/src/core/game/PlayerImpl.ts index 92b849e13..758d34255 100644 --- a/src/core/game/PlayerImpl.ts +++ b/src/core/game/PlayerImpl.ts @@ -95,6 +95,8 @@ export class PlayerImpl implements Player { private relations = new Map(); + private lastDeleteUnitTick: Tick = -1; + public _incomingAttacks: Attack[] = []; public _outgoingAttacks: Attack[] = []; public _outgoingLandAttacks: Attack[] = []; @@ -635,6 +637,17 @@ export class PlayerImpl implements Player { return true; } + canDeleteUnit(): boolean { + return ( + this.mg.ticks() - this.lastDeleteUnitTick >= + this.mg.config().deleteUnitCooldown() + ); + } + + recordDeleteUnit(): void { + this.lastDeleteUnitTick = this.mg.ticks(); + } + hasEmbargoAgainst(other: Player): boolean { return this.embargoes.has(other.id()); } diff --git a/tests/DeleteUnitExecution.test.ts b/tests/DeleteUnitExecution.test.ts new file mode 100644 index 000000000..c8486a1fe --- /dev/null +++ b/tests/DeleteUnitExecution.test.ts @@ -0,0 +1,123 @@ +import { DeleteUnitExecution } from "../src/core/execution/DeleteUnitExecution"; +import { SpawnExecution } from "../src/core/execution/SpawnExecution"; +import { + Game, + Player, + PlayerInfo, + PlayerType, + Unit, + UnitType, +} from "../src/core/game/Game"; +import { TileRef } from "../src/core/game/GameMap"; +import { setup } from "./util/Setup"; + +describe("DeleteUnitExecution Security Tests", () => { + let game: Game; + let player: Player; + let enemyPlayer: Player; + let unit: Unit; + + beforeEach(async () => { + game = await setup("plains", { + infiniteGold: true, + instantBuild: true, + infiniteTroops: true, + }); + + const player1Info = new PlayerInfo( + "TestPlayer", + PlayerType.Human, + null, + "TestPlayer", + ); + const player2Info = new PlayerInfo( + "EnemyPlayer", + PlayerType.Human, + null, + "EnemyPlayer", + ); + + game.addPlayer(player1Info); + game.addPlayer(player2Info); + + const playerSpawn: TileRef = game.ref(0, 10); + const enemySpawn: TileRef = game.ref(0, 15); + + game.addExecution( + new SpawnExecution(game.player(player1Info.id).info(), playerSpawn), + new SpawnExecution(game.player(player2Info.id).info(), enemySpawn), + ); + + while (game.inSpawnPhase()) { + game.executeNextTick(); + } + + player = game.player(player1Info.id); + enemyPlayer = game.player(player2Info.id); + + const playerTiles = Array.from(player.tiles()); + if (playerTiles.length === 0) { + throw new Error("Player has no tiles"); + } + const spawnTile = playerTiles[0]; + unit = player.buildUnit(UnitType.City, spawnTile, {}); + + const tileOwner = game.owner(unit.tile()); + if (!tileOwner.isPlayer() || tileOwner.id() !== player.id()) { + throw new Error("Unit is not on player's territory"); + } + }); + + describe("Security Validations", () => { + it("should prevent deleting units not owned by player", () => { + const enemyUnit = enemyPlayer.buildUnit( + UnitType.City, + Array.from(enemyPlayer.tiles())[0], + {}, + ); + const execution = new DeleteUnitExecution(player, enemyUnit.id()); + execution.init(game, 0); + + expect(execution.isActive()).toBe(false); + }); + + it("should prevent deleting units on enemy territory", () => { + const enemyTiles = Array.from(enemyPlayer.tiles()); + if (enemyTiles.length > 0) { + unit.move(enemyTiles[0]); + + const execution = new DeleteUnitExecution(player, unit.id()); + execution.init(game, 0); + + expect(execution.isActive()).toBe(false); + } + }); + + it("should prevent deleting units during spawn phase", () => { + jest.spyOn(game, "inSpawnPhase").mockReturnValue(true); + + const execution = new DeleteUnitExecution(player, unit.id()); + execution.init(game, 0); + + expect(execution.isActive()).toBe(false); + }); + + it("should allow deleting the last city (suicide)", () => { + jest.spyOn(game, "inSpawnPhase").mockReturnValue(false); + + const execution = new DeleteUnitExecution(player, unit.id()); + execution.init(game, 0); + + expect(unit.isActive()).toBe(false); + }); + + it("should allow deleting units when all conditions are met", () => { + jest.spyOn(game, "inSpawnPhase").mockReturnValue(false); + + const execution = new DeleteUnitExecution(player, unit.id()); + execution.init(game, 0); + + expect(unit.isActive()).toBe(false); + }); + }); +});