From ac14eaff107b32ecee30b1dce15ad63f7c3a092b Mon Sep 17 00:00:00 2001 From: Ilia Denisov Date: Sun, 10 May 2026 17:20:48 +0200 Subject: [PATCH] ui/phase-20: pick-first Send + lock after Modernize/Dismantle/Transfer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Send no longer carries a destination control inside the form: a click on the action drops the inspector straight into map-pick mode, and the form (ship count + confirm) only mounts after the player chooses a destination. Cancelling the picker leaves no form behind. A queued Modernize / Dismantle / Transfer for a given group locks every action button on its inspector and surfaces a banner that points the player at the order list. Cancelling the queued entry from the order tab releases the lock on the next render — the derivation watches draft.commands directly. Send / Load / Unload / Split / Join Fleet do not lock; Send is naturally followed by an out-of-orbit state at turn cutoff, the rest can stack legitimately. Co-Authored-By: Claude Opus 4.7 --- ui/PLAN.md | 37 +++- ui/docs/ship-group-actions.md | 33 ++- ui/frontend/src/lib/i18n/locales/en.ts | 6 +- ui/frontend/src/lib/i18n/locales/ru.ts | 6 +- .../lib/inspectors/ship-group/actions.svelte | 193 +++++++++++++----- ui/frontend/tests/e2e/ship-group-send.spec.ts | 29 ++- .../inspector-ship-group-actions.test.ts | 105 ++++++++++ 7 files changed, 332 insertions(+), 77 deletions(-) diff --git a/ui/PLAN.md b/ui/PLAN.md index 6c114ce..1dc2fd8 100644 --- a/ui/PLAN.md +++ b/ui/PLAN.md @@ -2160,12 +2160,14 @@ Artifacts: payload carries the *target* group UUID (the source group, or the freshly-minted `newGroupId` when an implicit split precedes the action) -- `Send` action picks destination through a planet picker filtered - by the group's reach (`localPlayerDrive * 40`, computed inline - via the existing `torusShortestDelta` from - `cargo-routes.svelte`); the player's tech levels are already on - `GameReport.localPlayer*` from Phase 18, no extra plumbing - needed +- `Send` action drops the inspector straight into map-pick mode + on click and only mounts the form (ship count + confirm) after + the player chooses a destination — there is no destination + control inside the form. The picker is filtered by the group's + reach (`localPlayerDrive * 40`, computed inline via the existing + `torusShortestDelta` from `cargo-routes.svelte`); the player's + tech levels are already on `GameReport.localPlayer*` from + Phase 18, no extra plumbing needed - `Modernize` cost preview through `core.blockUpgradeCost` (Phase 20 bridge), summed over the four ship-class blocks for the targeted ship count; preview hides when `Core` is not yet @@ -2175,6 +2177,14 @@ Artifacts: planet with colonists onboard (engine reference `controller/ship_group.go:177-179` — `UnloadColonists` is not called over a foreign planet, so the cargo is lost) +- destructive-command lock: a `Modernize` / `Dismantle` / + `Transfer` order in the draft for a given group disables every + action button on that group's inspector and surfaces a banner + pointing to the order list. Cancelling the queued command in + the order tab releases the lock. Other commands (Send / Load / + Unload / Split / JoinFleet) do not lock — Send is naturally + followed by an out-of-orbit state at turn cutoff and the + remaining four can stack legitimately - `pkg/calc/ship.go.BlockUpgradeCost` (migrated from `game/internal/controller/ship_group_upgrade.go`) — the bridge rule says `ui/core/calc/` only wraps `pkg/calc/` formulas, so @@ -2262,6 +2272,21 @@ Decisions during stage: at `newId`. JoinFleet and Split do not get a counter (JoinFleet is whole-group atomically per the engine; Split *is* the break command). +6. **Send is pick-first, form-second**. Click → enter map-pick + mode immediately. The form (ship count + confirm) only appears + after a destination is chosen; cancelling the picker leaves no + form behind. Removing the destination control from the form + keeps the surface to one editable field at any time. +7. **Destructive-command lock**. Any `upgradeShipGroup`, + `dismantleShipGroup`, or `transferShipGroup` in the draft for a + given group id disables every action button on that group's + inspector with a "command pending" tooltip and renders a + banner pointing the player at the order list. Cancellation + from the order tab releases the lock. The three commands all + change the group's engine-side state at turn cutoff + (`StateUpgrade` / removal / `StateTransfer`), so any second + action would race the engine's pre-condition check anyway — + the lock surfaces that commitment up-front. ## Phase 21. Sciences — CRUD List + Designer diff --git a/ui/docs/ship-group-actions.md b/ui/docs/ship-group-actions.md index eb5cf98..53b6217 100644 --- a/ui/docs/ship-group-actions.md +++ b/ui/docs/ship-group-actions.md @@ -61,7 +61,11 @@ every action with `ships are busy ({state})`. Per-action gates: pre-filters destinations by reach (`localPlayerDrive * 40`), so a valid pick is always within range. With no reachable planet, the action is disabled with - the "no planets in drive range" tooltip. + the "no planets in drive range" tooltip. Click drops the + inspector straight into map-pick mode; the form (ship count + + confirm) appears only after the player chooses a destination — + there is no destination control inside the form, so cancelling + the picker leaves the inspector untouched. - **Load**: requires the orbit planet to be owned by the player or unowned (`controller/ship_group.go:215`) and the ship class to have a cargo block (`shipGroupLoad:220`). The dropdown is @@ -88,6 +92,33 @@ every action with `ships are busy ({state})`. Per-action gates: in the same orbit (`fleet.go:135-137`); creating a new fleet always works. +## Destructive-command lock + +`Modernize`, `Dismantle`, and `Transfer` are *state-changing* at +turn cutoff: the engine moves the group into `StateUpgrade`, +removes it, or marks it `StateTransfer` respectively. Issuing a +follow-up action against the same group during the same draft +window would race the engine's pre-condition check, so the +inspector locks the group as soon as one of the three commands +lands in the draft for that `groupId`: + +- every action button on the group's inspector becomes disabled + with the "an order is already queued" tooltip; +- a banner above the buttons row names the queued command + (modernize / dismantle / transfer) and tells the player to + cancel it in the order list to issue something else; +- removing the queued entry from the order tab releases the lock + on the next render — the derivation watches `draft.commands` + directly. + +Send, Load, Unload, Split, and Join Fleet do not lock the group: +Send is naturally followed by an out-of-orbit state at turn +cutoff (the engine's busy check fires next turn anyway), and the +other four can stack legitimately during the same window. The +group continues to appear in the planet inspector's stationed- +ship list while locked — the player can still navigate to the +inspector to read the state and find the order to cancel. + ## Modernize cost preview The form's preview line calls diff --git a/ui/frontend/src/lib/i18n/locales/en.ts b/ui/frontend/src/lib/i18n/locales/en.ts index df699b8..246528a 100644 --- a/ui/frontend/src/lib/i18n/locales/en.ts +++ b/ui/frontend/src/lib/i18n/locales/en.ts @@ -308,6 +308,11 @@ const en = { "game.inspector.ship_group.action.disabled.full_load": "the group is fully loaded", "game.inspector.ship_group.action.disabled.no_other_races": "no other non-extinct races to transfer to", "game.inspector.ship_group.action.disabled.unknown_class": "the ship class is missing from the report", + "game.inspector.ship_group.action.disabled.locked": "an order is already queued for this group; cancel it in the order list to issue a new one", + "game.inspector.ship_group.action.locked.banner": "an order is already queued for this group: {command}. Cancel it in the order list to issue another action.", + "game.inspector.ship_group.action.locked.kind.modernize": "modernize", + "game.inspector.ship_group.action.locked.kind.dismantle": "dismantle", + "game.inspector.ship_group.action.locked.kind.transfer": "transfer", "game.inspector.ship_group.action.field.ships": "ships ({max} total)", "game.inspector.ship_group.action.field.cargo": "cargo type", "game.inspector.ship_group.action.field.quantity": "quantity", @@ -322,7 +327,6 @@ const en = { "game.inspector.ship_group.action.tech.shields": "shields", "game.inspector.ship_group.action.tech.cargo": "cargo", "game.inspector.ship_group.action.send.pick_prompt": "click a planet on the map (Esc to cancel)", - "game.inspector.ship_group.action.send.no_destination": "no destination chosen", "game.inspector.ship_group.action.modernize.cost": "estimated cost: {cost}", "game.inspector.ship_group.action.modernize.cost_unavailable": "cost preview unavailable", "game.inspector.ship_group.action.dismantle.warning": "the group is over a foreign planet with colonists aboard — they will die", diff --git a/ui/frontend/src/lib/i18n/locales/ru.ts b/ui/frontend/src/lib/i18n/locales/ru.ts index 4f91cb8..7ed7b7b 100644 --- a/ui/frontend/src/lib/i18n/locales/ru.ts +++ b/ui/frontend/src/lib/i18n/locales/ru.ts @@ -309,6 +309,11 @@ const ru: Record = { "game.inspector.ship_group.action.disabled.full_load": "трюм полностью заполнен", "game.inspector.ship_group.action.disabled.no_other_races": "нет других нерасправленных рас для передачи", "game.inspector.ship_group.action.disabled.unknown_class": "класс корабля не найден в отчёте", + "game.inspector.ship_group.action.disabled.locked": "по группе уже отдан приказ; отмените его в списке приказов, чтобы дать новое действие", + "game.inspector.ship_group.action.locked.banner": "по группе уже отдан приказ: {command}. Отмените его в списке приказов, чтобы дать другое действие.", + "game.inspector.ship_group.action.locked.kind.modernize": "модернизация", + "game.inspector.ship_group.action.locked.kind.dismantle": "разборка", + "game.inspector.ship_group.action.locked.kind.transfer": "передача", "game.inspector.ship_group.action.field.ships": "кораблей (всего {max})", "game.inspector.ship_group.action.field.cargo": "тип груза", "game.inspector.ship_group.action.field.quantity": "количество", @@ -323,7 +328,6 @@ const ru: Record = { "game.inspector.ship_group.action.tech.shields": "защита", "game.inspector.ship_group.action.tech.cargo": "груз", "game.inspector.ship_group.action.send.pick_prompt": "выберите планету на карте (Esc — отмена)", - "game.inspector.ship_group.action.send.no_destination": "планета не выбрана", "game.inspector.ship_group.action.modernize.cost": "ожидаемая стоимость: {cost}", "game.inspector.ship_group.action.modernize.cost_unavailable": "предпросмотр недоступен", "game.inspector.ship_group.action.dismantle.warning": "группа над чужой планетой везёт колонистов — они погибнут", diff --git a/ui/frontend/src/lib/inspectors/ship-group/actions.svelte b/ui/frontend/src/lib/inspectors/ship-group/actions.svelte index 1025fad..9b562a5 100644 --- a/ui/frontend/src/lib/inspectors/ship-group/actions.svelte +++ b/ui/frontend/src/lib/inspectors/ship-group/actions.svelte @@ -118,6 +118,22 @@ modernize cost preview backed by `core.blockUpgradeCost`. openForm = null; }); + // Close any open form the moment the group becomes locked + // (a destructive command landed in the draft from elsewhere — + // e.g. another browser tab editing the same draft, or a + // concurrent action on this inspector). Without this guard a + // pending form would still allow Confirm despite the locked + // banner above it. + $effect(() => { + if (pendingDestructiveCommand !== null) { + if (sendPicking) { + pick?.cancel(); + sendPicking = false; + } + openForm = null; + } + }); + const inOrbit = $derived(group.state === "In_Orbit"); const orbitPlanet = $derived( inOrbit ? (planets.find((p) => p.number === group.destination) ?? null) : null, @@ -229,14 +245,58 @@ modernize cost preview backed by `core.blockUpgradeCost`. return reason === null ? null : i18n.t(reason); } - const splitDisabledReason = $derived( - !inOrbit - ? "game.inspector.ship_group.action.disabled.not_in_orbit" - : group.count < 2 - ? "game.inspector.ship_group.action.invalid.ship_count" - : null, + // pendingDestructiveCommand watches the order draft for any + // modernize / dismantle / transfer command targeting this group. + // Once the player queues one of those three, every action on the + // group is disabled until the draft entry is removed: each is + // state-changing at turn cutoff (Modernize → state Upgrade, + // Transfer → state Transfer, Dismantle → group removed), so a + // follow-up action would race the engine's pre-condition check + // and noisy-fail server-side. The lock surfaces the commitment + // up-front and points the player at the order list as the way + // to release it. + const pendingDestructiveCommand = $derived.by(() => { + if (draft === undefined) return null; + for (const cmd of draft.commands) { + if ( + cmd.kind !== "upgradeShipGroup" && + cmd.kind !== "dismantleShipGroup" && + cmd.kind !== "transferShipGroup" + ) + continue; + if (cmd.groupId !== group.id) continue; + const status = draft.statuses[cmd.id]; + if (status === "rejected" || status === "invalid") continue; + return cmd; + } + return null; + }); + const lockedReason: TranslationKey | null = $derived( + pendingDestructiveCommand === null + ? null + : "game.inspector.ship_group.action.disabled.locked", ); + const lockedKindLabel = $derived.by(() => { + const cmd = pendingDestructiveCommand; + if (cmd === null) return ""; + switch (cmd.kind) { + case "upgradeShipGroup": + return i18n.t("game.inspector.ship_group.action.locked.kind.modernize"); + case "dismantleShipGroup": + return i18n.t("game.inspector.ship_group.action.locked.kind.dismantle"); + case "transferShipGroup": + return i18n.t("game.inspector.ship_group.action.locked.kind.transfer"); + } + }); + + const splitDisabledReason = $derived.by((): TranslationKey | null => { + if (lockedReason !== null) return lockedReason; + if (!inOrbit) return "game.inspector.ship_group.action.disabled.not_in_orbit"; + if (group.count < 2) return "game.inspector.ship_group.action.invalid.ship_count"; + return null; + }); const sendDisabledReason = $derived.by((): TranslationKey | null => { + if (lockedReason !== null) return lockedReason; if (!inOrbit) return "game.inspector.ship_group.action.disabled.not_in_orbit"; if (!hasDriveBlock) return "game.inspector.ship_group.action.disabled.no_drive"; if (orbitPlanet === null) return "game.inspector.ship_group.action.disabled.no_planet"; @@ -244,6 +304,7 @@ modernize cost preview backed by `core.blockUpgradeCost`. return null; }); const loadDisabledReason = $derived.by((): TranslationKey | null => { + if (lockedReason !== null) return lockedReason; if (!inOrbit) return "game.inspector.ship_group.action.disabled.not_in_orbit"; if (!hasCargoBlock) return "game.inspector.ship_group.action.disabled.no_cargo_block"; if (orbitPlanet === null) return "game.inspector.ship_group.action.disabled.no_planet"; @@ -256,6 +317,7 @@ modernize cost preview backed by `core.blockUpgradeCost`. return null; }); const unloadDisabledReason = $derived.by((): TranslationKey | null => { + if (lockedReason !== null) return lockedReason; if (!inOrbit) return "game.inspector.ship_group.action.disabled.not_in_orbit"; if (!hasCargoBlock) return "game.inspector.ship_group.action.disabled.no_cargo_block"; if (!cargoLoaded) return "game.inspector.ship_group.action.disabled.empty_cargo"; @@ -264,6 +326,7 @@ modernize cost preview backed by `core.blockUpgradeCost`. return null; }); const modernizeDisabledReason = $derived.by((): TranslationKey | null => { + if (lockedReason !== null) return lockedReason; if (!inOrbit) return "game.inspector.ship_group.action.disabled.not_in_orbit"; if (orbitPlanet === null) return "game.inspector.ship_group.action.disabled.no_planet"; if (!friendlyPlanet) return "game.inspector.ship_group.action.disabled.foreign_planet"; @@ -272,16 +335,19 @@ modernize cost preview backed by `core.blockUpgradeCost`. return null; }); const dismantleDisabledReason = $derived.by((): TranslationKey | null => { + if (lockedReason !== null) return lockedReason; if (!inOrbit) return "game.inspector.ship_group.action.disabled.not_in_orbit"; return null; }); const transferDisabledReason = $derived.by((): TranslationKey | null => { + if (lockedReason !== null) return lockedReason; if (!inOrbit) return "game.inspector.ship_group.action.disabled.not_in_orbit"; if (otherRaces.length === 0) return "game.inspector.ship_group.action.disabled.no_other_races"; return null; }); const joinFleetDisabledReason = $derived.by((): TranslationKey | null => { + if (lockedReason !== null) return lockedReason; if (!inOrbit) return "game.inspector.ship_group.action.disabled.not_in_orbit"; return null; }); @@ -302,10 +368,42 @@ modernize cost preview backed by `core.blockUpgradeCost`. closeOthers("split"); splitShips = Math.max(1, Math.min(Math.floor(group.count / 2), group.count - 1)); } - function openSend(): void { - closeOthers("send"); + async function openSend(): Promise { + // "Send" is a two-stage flow: the click drops the inspector + // straight into map-pick mode, and the form (ship count + + // confirm) only appears after the player chooses a destination + // or cancels. The destination is therefore never an editable + // control inside the form — picking is the entry point, not a + // sub-step. Re-clicking the action while picking cancels the + // session; re-clicking while the form is open closes it. + if (sendPicking) { + pick?.cancel(); + sendPicking = false; + openForm = null; + return; + } + if (openForm === "send") { + openForm = null; + return; + } + openForm = null; + if (pick === undefined || draft === undefined) return; + if (orbitPlanet === null || reachableSet.size === 0) return; sendShips = group.count; sendDestination = null; + sendPicking = true; + let picked: number | null = null; + try { + picked = await pick.pick({ + sourcePlanetNumber: orbitPlanet.number, + reachableIds: reachableSet, + }); + } finally { + sendPicking = false; + } + if (picked === null) return; + sendDestination = picked; + openForm = "send"; } function openLoad(): void { closeOthers("load"); @@ -409,21 +507,6 @@ modernize cost preview backed by `core.blockUpgradeCost`. openForm = null; } - async function startSendPick(): Promise { - if (pick === undefined || sendPicking) return; - if (reachableSet.size === 0 || orbitPlanet === null) return; - sendPicking = true; - try { - const picked = await pick.pick({ - sourcePlanetNumber: orbitPlanet.number, - reachableIds: reachableSet, - }); - if (picked !== null) sendDestination = picked; - } finally { - sendPicking = false; - } - } - async function confirmSend(): Promise { if (sendDestination === null || draft === undefined) return; const ships = clampShips(sendShips); @@ -592,13 +675,20 @@ modernize cost preview backed by `core.blockUpgradeCost`.
+ {#if pendingDestructiveCommand !== null} +

+ {i18n.t("game.inspector.ship_group.action.locked.banner", { + command: lockedKindLabel, + })} +

+ {/if}
@@ -704,8 +794,14 @@ modernize cost preview backed by `core.blockUpgradeCost`. {/if} - {#if openForm === "send"} + {#if openForm === "send" && sendDestination !== null}
{ e.preventDefault(); void confirmSend(); }}> +

+ {i18n.t("game.inspector.ship_group.action.field.destination")} + + {planets.find((p) => p.number === sendDestination)?.name ?? `#${sendDestination}`} + +

-
- {i18n.t("game.inspector.ship_group.action.field.destination")} - - {#if sendDestination !== null} - {planets.find((p) => p.number === sendDestination)?.name ?? `#${sendDestination}`} - {:else} - {i18n.t("game.inspector.ship_group.action.send.no_destination")} - {/if} - - -
@@ -753,6 +828,12 @@ modernize cost preview backed by `core.blockUpgradeCost`. {/if} + {#if sendPicking} +

+ {i18n.t("game.inspector.ship_group.action.send.pick_prompt")} +

+ {/if} + {#if openForm === "load"}
{ e.preventDefault(); void confirmLoad(); }}>