From 723885e74ec51f9b4e6661d8322bfe78e50c5956 Mon Sep 17 00:00:00 2001 From: Ilia Denisov Date: Fri, 29 May 2026 11:42:27 +0200 Subject: [PATCH] fix(order): surface rejection reason, keep sync green, hydrate verdicts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three issues surfaced once the per-command rejection from the previous commit actually reached the UI: 1. Sync banner falsely red. `OrderDraftStore.runSync` flipped `syncStatus = "error"` whenever any command was rejected and advertised a Retry button. A per-command rejection is a player-correctable state — the round trip succeeded, the engine just refused that command — so the retry can't help. Keep `syncStatus = "synced"` on `success`; the red row highlight is the visible cue. 2. Rejection reason missing. Add `cmd_error_message: string` to `CommandItem` in `pkg/schema/fbs/order.fbs` (appended last to preserve existing slot offsets) and regenerate the Go + TS stubs for that one type. Plumb the message through `CommandMeta`, `Controller.applyCommand`'s `m.Result(code, message)` call, the Go transcoder, the UI decoders in `submit.ts` / `order-load.ts`, and the `OrderDraftStore.errorMessages` map. `order-tab.svelte` renders it as an italic danger-coloured line under rejected commands, with new CSS for `.error-reason`. 3. Verdict lost on navigation. `order-load.ts.decodeCommand` never read `cmdApplied`/`cmdErrorCode`, so `hydrateFromServer` fell back to a blanket "applied" status — a previously-rejected command came back green after a lobby → game round trip. Extend the fetch decoder to populate `statuses`/`errorCodes`/ `errorMessages` maps and have `hydrateFromServer` use them. Engine-side persistence already records the verdict on disk — verified against the live `0000/order/.json`. `flatbuffers@25` elides default-int8/int64 fields on write; the Go transcoder force-slots `cmd_applied=false` / `cmd_error_code=0` already, the new test fixtures flip `builder.forceDefaults(true)` to mirror that behaviour so the round trip survives. Co-Authored-By: Claude Opus 4.7 (1M context) --- game/internal/controller/order.go | 4 +- game/internal/repo/repo_test.go | 8 +- game/openapi.yaml | 10 ++ pkg/model/order/order.go | 12 +- pkg/schema/fbs/order.fbs | 6 + pkg/schema/fbs/order/CommandItem.go | 13 +- pkg/transcoder/order.go | 23 ++++ pkg/transcoder/order_test.go | 8 +- ui/frontend/src/lib/sidebar/order-tab.svelte | 23 ++++ .../proto/galaxy/fbs/order/command-item.ts | 31 ++++- ui/frontend/src/sync/order-draft.svelte.ts | 71 +++++++--- ui/frontend/src/sync/order-load.ts | 33 ++++- ui/frontend/src/sync/submit.ts | 13 +- .../tests/helpers/fake-order-client.ts | 41 +++++- ui/frontend/tests/order-draft.test.ts | 127 +++++++++++++++++- ui/frontend/tests/order-queue.test.ts | 1 + ui/frontend/tests/submit.test.ts | 20 ++- 17 files changed, 404 insertions(+), 40 deletions(-) diff --git a/game/internal/controller/order.go b/game/internal/controller/order.go index e695708..6a30d30 100644 --- a/game/internal/controller/order.go +++ b/game/internal/controller/order.go @@ -109,11 +109,11 @@ func (c *Controller) applyCommand(actor string, cmd order.DecodableCommand) (err } if ge, ok := errors.AsType[*e.GenericError](err); ok { - m.Result(ge.Code) + m.Result(ge.Code, ge.Error()) } else if err != nil { panic(fmt.Errorf("error applying command has unknown origin: %w", err)) } else { - m.Result(0) + m.Result(0, "") } return diff --git a/game/internal/repo/repo_test.go b/game/internal/repo/repo_test.go index 7bca20f..b504df8 100644 --- a/game/internal/repo/repo_test.go +++ b/game/internal/repo/repo_test.go @@ -75,14 +75,14 @@ func TestSaveOrder(t *testing.T) { for i := range o.Commands { if v, ok := order.AsCommand[*order.CommandRaceVote](o.Commands[i]); ok { m := &v.CommandMeta - m.Result(0) + m.Result(0, "") } else if v, ok := order.AsCommand[*order.CommandRaceQuit](o.Commands[i]); ok { - v.Result(10) + v.Result(10, "race quit failed") } else if v, ok := order.AsCommand[*order.CommandShipClassCreate](o.Commands[i]); ok { m := &v.CommandMeta - m.Result(33) + m.Result(33, "ship class create failed") } else if v, ok := order.AsCommand[*order.CommandShipGroupMerge](o.Commands[i]); ok { - v.Result(0) + v.Result(0, "") } } diff --git a/game/openapi.yaml b/game/openapi.yaml index b875b5b..42bfd75 100644 --- a/game/openapi.yaml +++ b/game/openapi.yaml @@ -520,6 +520,16 @@ components: `0` when the command was applied, a non-zero `GenericError` code (shelves `2xxx`/`3xxx` in `pkg/error/generic.go`) when the command was rejected. Omitted on requests. + cmdErrorMessage: + type: string + description: | + Per-command rejection reason, formatted by the engine's + `GenericError.Error()` (e.g. + `Entity does not exists: ship type "Drone"`). Set alongside + `cmdApplied=false`/`cmdErrorCode!=0`, omitted when the + command was applied. Provided so clients can surface the + specific reason without keeping their own code → text + catalog in sync with the engine. CommandType: type: string description: Discriminator identifying the game command variant carried in a `cmd` element. diff --git a/pkg/model/order/order.go b/pkg/model/order/order.go index 2488431..5eef406 100644 --- a/pkg/model/order/order.go +++ b/pkg/model/order/order.go @@ -124,6 +124,7 @@ type CommandMeta struct { CmdID string `json:"cmdId" binding:"required,uuid_rfc4122"` CmdApplied *bool `json:"cmdApplied,omitempty"` CmdErrCode *int `json:"cmdErrorCode,omitempty"` + CmdErrMsg *string `json:"cmdErrorMessage,omitempty"` } func (cm CommandMeta) CommandType() CommandType { @@ -134,9 +135,18 @@ func (cm CommandMeta) CommandID() string { return cm.CmdID } -func (cm *CommandMeta) Result(errCode int) { +// Result records the per-command outcome on the meta. errCode == 0 marks +// the command as applied and clears CmdErrMsg; non-zero records the +// rejection along with the human-readable engine message so clients can +// surface the reason without their own code-to-text catalog. +func (cm *CommandMeta) Result(errCode int, errMsg string) { cm.CmdErrCode = &errCode cm.CmdApplied = new(bool(errCode == 0)) + if errCode == 0 { + cm.CmdErrMsg = nil + return + } + cm.CmdErrMsg = &errMsg } type CommandRaceQuit struct { diff --git a/pkg/schema/fbs/order.fbs b/pkg/schema/fbs/order.fbs index bc6be0c..32b161c 100644 --- a/pkg/schema/fbs/order.fbs +++ b/pkg/schema/fbs/order.fbs @@ -193,6 +193,12 @@ table CommandItem { cmd_applied: bool = null; cmd_error_code: int64 = null; payload: CommandPayload (required); + // Human-readable failure reason returned by the engine when + // `cmd_applied = false`. Appended after `payload` to preserve the + // wire offsets of existing slots (FBS field IDs are allocated in + // declaration order, so inserting in the middle would shift every + // later slot). Omitted on requests and on applied commands. + cmd_error_message: string; } // UserGamesCommand is the signed-gRPC request payload for diff --git a/pkg/schema/fbs/order/CommandItem.go b/pkg/schema/fbs/order/CommandItem.go index 8bc63d3..cde005b 100644 --- a/pkg/schema/fbs/order/CommandItem.go +++ b/pkg/schema/fbs/order/CommandItem.go @@ -96,8 +96,16 @@ func (rcv *CommandItem) Payload(obj *flatbuffers.Table) bool { return false } +func (rcv *CommandItem) CmdErrorMessage() []byte { + o := flatbuffers.UOffsetT(rcv._tab.Offset(14)) + if o != 0 { + return rcv._tab.ByteVector(o + rcv._tab.Pos) + } + return nil +} + func CommandItemStart(builder *flatbuffers.Builder) { - builder.StartObject(5) + builder.StartObject(6) } func CommandItemAddCmdId(builder *flatbuffers.Builder, cmdId flatbuffers.UOffsetT) { builder.PrependUOffsetTSlot(0, flatbuffers.UOffsetT(cmdId), 0) @@ -116,6 +124,9 @@ func CommandItemAddPayloadType(builder *flatbuffers.Builder, payloadType Command func CommandItemAddPayload(builder *flatbuffers.Builder, payload flatbuffers.UOffsetT) { builder.PrependUOffsetTSlot(4, flatbuffers.UOffsetT(payload), 0) } +func CommandItemAddCmdErrorMessage(builder *flatbuffers.Builder, cmdErrorMessage flatbuffers.UOffsetT) { + builder.PrependUOffsetTSlot(5, flatbuffers.UOffsetT(cmdErrorMessage), 0) +} func CommandItemEnd(builder *flatbuffers.Builder) flatbuffers.UOffsetT { return builder.EndObject() } diff --git a/pkg/transcoder/order.go b/pkg/transcoder/order.go index f8b419c..bb13a84 100644 --- a/pkg/transcoder/order.go +++ b/pkg/transcoder/order.go @@ -125,6 +125,7 @@ type encodedCommand struct { cmdID string cmdApplied *bool cmdErrCode *int + cmdErrMsg *string payloadType fbs.CommandPayload payloadOffset flatbuffers.UOffsetT } @@ -404,6 +405,7 @@ func encodedCommandFromMeta(meta model.CommandMeta, payloadType fbs.CommandPaylo cmdID: meta.CmdID, cmdApplied: cloneBoolPointer(meta.CmdApplied), cmdErrCode: cloneIntPointer(meta.CmdErrCode), + cmdErrMsg: cloneStringPointer(meta.CmdErrMsg), payloadType: payloadType, payloadOffset: payloadOffset, } @@ -423,6 +425,11 @@ func decodeOrderCommand(flatCommand *fbs.CommandItem, index int) (model.Decodabl commandMeta.CmdErrCode = &decodedCmdErrCode } + if cmdErrMsg := flatCommand.CmdErrorMessage(); cmdErrMsg != nil { + decodedCmdErrMsg := string(cmdErrMsg) + commandMeta.CmdErrMsg = &decodedCmdErrMsg + } + payloadType := flatCommand.PayloadType() if payloadType == fbs.CommandPayloadNONE { return nil, fmt.Errorf("decode order command %d: payload type is NONE", index) @@ -915,6 +922,15 @@ func cloneIntPointer(value *int) *int { return &cloned } +func cloneStringPointer(value *string) *string { + if value == nil { + return nil + } + + cloned := *value + return &cloned +} + // UserGamesCommandToPayload converts model.UserGamesCommand to // FlatBuffers bytes suitable for the authenticated gateway transport. // `GameID` is required. @@ -1293,6 +1309,10 @@ func encodeCommandItemVector(builder *flatbuffers.Builder, commands []model.Deco return 0, fmt.Errorf("encode %s: %w", opLabel, err) } cmdID := builder.CreateString(encoded.cmdID) + var cmdErrMsg flatbuffers.UOffsetT + if encoded.cmdErrMsg != nil { + cmdErrMsg = builder.CreateString(*encoded.cmdErrMsg) + } fbs.CommandItemStart(builder) fbs.CommandItemAddCmdId(builder, cmdID) if encoded.cmdApplied != nil { @@ -1303,6 +1323,9 @@ func encodeCommandItemVector(builder *flatbuffers.Builder, commands []model.Deco } fbs.CommandItemAddPayloadType(builder, encoded.payloadType) fbs.CommandItemAddPayload(builder, encoded.payloadOffset) + if encoded.cmdErrMsg != nil { + fbs.CommandItemAddCmdErrorMessage(builder, cmdErrMsg) + } offsets[i] = fbs.CommandItemEnd(builder) } if len(offsets) == 0 { diff --git a/pkg/transcoder/order_test.go b/pkg/transcoder/order_test.go index c573669..d64fbb2 100644 --- a/pkg/transcoder/order_test.go +++ b/pkg/transcoder/order_test.go @@ -94,6 +94,7 @@ func TestUserGamesOrderResponsePayloadRoundTrip(t *testing.T) { applied := true rejected := false errCode := 7 + errMsg := "rename failed: planet does not exist" source := &model.UserGamesOrder{ GameID: uuid.MustParse("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"), UpdatedAt: 99, @@ -104,7 +105,7 @@ func TestUserGamesOrderResponsePayloadRoundTrip(t *testing.T) { Name: "alpha", }, &model.CommandPlanetRename{ - CommandMeta: commandMeta("cmd-2", model.CommandTypePlanetRename, &rejected, &errCode), + CommandMeta: commandMetaWithMsg("cmd-2", model.CommandTypePlanetRename, &rejected, &errCode, &errMsg), Number: 6, Name: "beta", }, @@ -254,10 +255,15 @@ func TestInt64ToInt(t *testing.T) { } func commandMeta(id string, cmdType model.CommandType, applied *bool, errCode *int) model.CommandMeta { + return commandMetaWithMsg(id, cmdType, applied, errCode, nil) +} + +func commandMetaWithMsg(id string, cmdType model.CommandType, applied *bool, errCode *int, errMsg *string) model.CommandMeta { return model.CommandMeta{ CmdType: cmdType, CmdID: id, CmdApplied: applied, CmdErrCode: errCode, + CmdErrMsg: errMsg, } } diff --git a/ui/frontend/src/lib/sidebar/order-tab.svelte b/ui/frontend/src/lib/sidebar/order-tab.svelte index a6d308f..4ca7499 100644 --- a/ui/frontend/src/lib/sidebar/order-tab.svelte +++ b/ui/frontend/src/lib/sidebar/order-tab.svelte @@ -153,6 +153,10 @@ Tests exercise the tab through `__galaxyDebug.seedOrderDraft` function statusOf(cmd: OrderCommand): CommandStatus { return draft?.statuses[cmd.id] ?? "draft"; } + + function errorMessageOf(cmd: OrderCommand): string | null { + return draft?.errorMessages[cmd.id] ?? null; + }
@@ -191,6 +195,7 @@ Tests exercise the tab through `__galaxyDebug.seedOrderDraft`
    {#each draft.commands as cmd, index (cmd.id)} {@const status = statusOf(cmd)} + {@const errorReason = errorMessageOf(cmd)}
  1. {describe(cmd)} + {#if status === "rejected" && errorReason !== null} + + {errorReason} + + {/if} (obj:any):any|null { return offset ? this.bb!.__union(obj, this.bb_pos + offset) : null; } +cmdErrorMessage():string|null +cmdErrorMessage(optionalEncoding:flatbuffers.Encoding):string|Uint8Array|null +cmdErrorMessage(optionalEncoding?:any):string|Uint8Array|null { + const offset = this.bb!.__offset(this.bb_pos, 14); + return offset ? this.bb!.__string(this.bb_pos + offset, optionalEncoding) : null; +} + static startCommandItem(builder:flatbuffers.Builder) { - builder.startObject(5); + builder.startObject(6); } static addCmdId(builder:flatbuffers.Builder, cmdIdOffset:flatbuffers.Offset) { @@ -84,11 +91,11 @@ static addCmdId(builder:flatbuffers.Builder, cmdIdOffset:flatbuffers.Offset) { } static addCmdApplied(builder:flatbuffers.Builder, cmdApplied:boolean) { - builder.addFieldInt8(1, +cmdApplied, null); + builder.addFieldInt8(1, +cmdApplied, 0); } static addCmdErrorCode(builder:flatbuffers.Builder, cmdErrorCode:bigint) { - builder.addFieldInt64(2, cmdErrorCode, null); + builder.addFieldInt64(2, cmdErrorCode, BigInt(0)); } static addPayloadType(builder:flatbuffers.Builder, payloadType:CommandPayload) { @@ -99,13 +106,17 @@ static addPayload(builder:flatbuffers.Builder, payloadOffset:flatbuffers.Offset) builder.addFieldOffset(4, payloadOffset, 0); } +static addCmdErrorMessage(builder:flatbuffers.Builder, cmdErrorMessageOffset:flatbuffers.Offset) { + builder.addFieldOffset(5, cmdErrorMessageOffset, 0); +} + static endCommandItem(builder:flatbuffers.Builder):flatbuffers.Offset { const offset = builder.endObject(); builder.requiredField(offset, 12) // payload return offset; } -static createCommandItem(builder:flatbuffers.Builder, cmdIdOffset:flatbuffers.Offset, cmdApplied:boolean|null, cmdErrorCode:bigint|null, payloadType:CommandPayload, payloadOffset:flatbuffers.Offset):flatbuffers.Offset { +static createCommandItem(builder:flatbuffers.Builder, cmdIdOffset:flatbuffers.Offset, cmdApplied:boolean|null, cmdErrorCode:bigint|null, payloadType:CommandPayload, payloadOffset:flatbuffers.Offset, cmdErrorMessageOffset:flatbuffers.Offset):flatbuffers.Offset { CommandItem.startCommandItem(builder); CommandItem.addCmdId(builder, cmdIdOffset); if (cmdApplied !== null) @@ -114,6 +125,7 @@ static createCommandItem(builder:flatbuffers.Builder, cmdIdOffset:flatbuffers.Of CommandItem.addCmdErrorCode(builder, cmdErrorCode); CommandItem.addPayloadType(builder, payloadType); CommandItem.addPayload(builder, payloadOffset); + CommandItem.addCmdErrorMessage(builder, cmdErrorMessageOffset); return CommandItem.endCommandItem(builder); } @@ -127,7 +139,8 @@ unpack(): CommandItemT { const temp = unionToCommandPayload(this.payloadType(), this.payload.bind(this)); if(temp === null) { return null; } return temp.unpack() - })() + })(), + this.cmdErrorMessage() ); } @@ -142,6 +155,7 @@ unpackTo(_o: CommandItemT): void { if(temp === null) { return null; } return temp.unpack() })(); + _o.cmdErrorMessage = this.cmdErrorMessage(); } } @@ -151,20 +165,23 @@ constructor( public cmdApplied: boolean|null = null, public cmdErrorCode: bigint|null = null, public payloadType: CommandPayload = CommandPayload.NONE, - public payload: CommandFleetMergeT|CommandFleetSendT|CommandPlanetProduceT|CommandPlanetRenameT|CommandPlanetRouteRemoveT|CommandPlanetRouteSetT|CommandRaceQuitT|CommandRaceRelationT|CommandRaceVoteT|CommandScienceCreateT|CommandScienceRemoveT|CommandShipClassCreateT|CommandShipClassMergeT|CommandShipClassRemoveT|CommandShipGroupBreakT|CommandShipGroupDismantleT|CommandShipGroupJoinFleetT|CommandShipGroupLoadT|CommandShipGroupMergeT|CommandShipGroupSendT|CommandShipGroupTransferT|CommandShipGroupUnloadT|CommandShipGroupUpgradeT|null = null + public payload: CommandFleetMergeT|CommandFleetSendT|CommandPlanetProduceT|CommandPlanetRenameT|CommandPlanetRouteRemoveT|CommandPlanetRouteSetT|CommandRaceQuitT|CommandRaceRelationT|CommandRaceVoteT|CommandScienceCreateT|CommandScienceRemoveT|CommandShipClassCreateT|CommandShipClassMergeT|CommandShipClassRemoveT|CommandShipGroupBreakT|CommandShipGroupDismantleT|CommandShipGroupJoinFleetT|CommandShipGroupLoadT|CommandShipGroupMergeT|CommandShipGroupSendT|CommandShipGroupTransferT|CommandShipGroupUnloadT|CommandShipGroupUpgradeT|null = null, + public cmdErrorMessage: string|Uint8Array|null = null ){} pack(builder:flatbuffers.Builder): flatbuffers.Offset { const cmdId = (this.cmdId !== null ? builder.createString(this.cmdId!) : 0); const payload = builder.createObjectOffset(this.payload); + const cmdErrorMessage = (this.cmdErrorMessage !== null ? builder.createString(this.cmdErrorMessage!) : 0); return CommandItem.createCommandItem(builder, cmdId, this.cmdApplied, this.cmdErrorCode, this.payloadType, - payload + payload, + cmdErrorMessage ); } } diff --git a/ui/frontend/src/sync/order-draft.svelte.ts b/ui/frontend/src/sync/order-draft.svelte.ts index bd093ba..df4275b 100644 --- a/ui/frontend/src/sync/order-draft.svelte.ts +++ b/ui/frontend/src/sync/order-draft.svelte.ts @@ -95,6 +95,15 @@ export interface PausedBanner { export class OrderDraftStore { commands: OrderCommand[] = $state([]); statuses: Record = $state({}); + /** + * errorMessages carries the engine-formatted rejection reason for + * each rejected command, keyed by `cmdId`. Populated from + * `submitOrder` responses and from the server-stored order on + * `hydrateFromServer`. Cleared when a command is re-mutated or + * removed. The order tab renders the message inline on rejected + * rows so the player can act without inspecting the network log. + */ + errorMessages: Record = $state({}); updatedAt = $state(0); status: Status = $state("idle"); error: string | null = $state(null); @@ -261,16 +270,29 @@ export class OrderDraftStore { this.commands = fetched.commands; this.updatedAt = fetched.updatedAt; this.recomputeStatuses(); - // Server-fetched commands echo cmdApplied=true for entries - // that survived previous turns; keep them as `applied` so - // the overlay continues to project them on the inspector. + // The engine echoes per-command `cmdApplied`/`cmdErrorCode` + // /`cmdErrorMessage` on every stored order entry. Hydrate + // the in-memory status + error maps from the fetched + // snapshot so a returning player sees the same per-command + // verdict (and rejection reason) the previous submission + // produced, instead of a synthetic blanket `applied` derived + // from the local cache. const next = { ...this.statuses }; + const nextErrors: Record = {}; for (const cmd of this.commands) { - if (next[cmd.id] === "valid") { + const status = fetched.statuses.get(cmd.id); + if (status !== undefined && next[cmd.id] !== "invalid") { + next[cmd.id] = status; + } else if (next[cmd.id] === "valid") { next[cmd.id] = "applied"; } + const msg = fetched.errorMessages.get(cmd.id); + if (msg !== null && msg !== undefined && msg !== "") { + nextErrors[cmd.id] = msg; + } } this.statuses = next; + this.errorMessages = nextErrors; await this.persist(); if ((this.syncStatus as SyncStatus) !== "paused") { this.syncStatus = "synced"; @@ -384,11 +406,15 @@ export class OrderDraftStore { } this.commands = nextCommands; const nextStatuses = { ...this.statuses }; + const nextErrors = { ...this.errorMessages }; for (const id of removed) { delete nextStatuses[id]; + delete nextErrors[id]; } nextStatuses[command.id] = validateCommand(command); + delete nextErrors[command.id]; this.statuses = nextStatuses; + this.errorMessages = nextErrors; await this.persist(); this.scheduleSync(); } @@ -408,8 +434,11 @@ export class OrderDraftStore { this.clearConflictForMutation(); this.commands = next; const nextStatuses = { ...this.statuses }; + const nextErrors = { ...this.errorMessages }; delete nextStatuses[id]; + delete nextErrors[id]; this.statuses = nextStatuses; + this.errorMessages = nextErrors; await this.persist(); this.scheduleSync(); } @@ -484,6 +513,7 @@ export class OrderDraftStore { if (this.status !== "ready") return; this.commands = []; this.statuses = {}; + this.errorMessages = {}; this.updatedAt = 0; this.conflictBanner = null; this.pausedBanner = null; @@ -571,20 +601,18 @@ export class OrderDraftStore { this.applyResultsInternal( outcome.result.results, outcome.result.updatedAt, + outcome.result.errorMessages, ); - // Even with `result.ok === true` an individual - // command may have been rejected by the engine - // (e.g. validation passed transcoders but failed - // the in-game rule). Surface that as an error in - // the sync bar so the player notices and can fix - // or remove the offending command. - const anyRejected = Array.from( - outcome.result.results.values(), - ).some((s) => s === "rejected"); - this.syncStatus = anyRejected ? "error" : "synced"; - this.syncError = anyRejected - ? "engine rejected one or more commands" - : null; + // A `success` outcome means the gateway → backend → + // engine round trip completed cleanly, even when + // individual commands were rejected by the in-game + // rules. Per-command rejection is a player-fixable + // state surfaced via the row's `rejected` highlight + // and the inline reason; the sync bar stays green so + // the banner + Retry button are reserved for genuine + // transport / engine failures the auto-retry can fix. + this.syncStatus = "synced"; + this.syncError = null; break; } case "rejected": { @@ -669,9 +697,11 @@ export class OrderDraftStore { private applyResultsInternal( results: Map, updatedAt: number, + errorMessages?: Map, ): void { const liveIds = new Set(this.commands.map((cmd) => cmd.id)); const next = { ...this.statuses }; + const nextErrors = { ...this.errorMessages }; for (const [id, status] of results.entries()) { // Drop verdicts for commands the user removed while the // request was in flight — they are no longer in the @@ -679,8 +709,15 @@ export class OrderDraftStore { // confuse the order tab and the overlay. if (!liveIds.has(id)) continue; next[id] = status; + const msg = errorMessages?.get(id) ?? null; + if (msg !== null && msg !== "") { + nextErrors[id] = msg; + } else { + delete nextErrors[id]; + } } this.statuses = next; + this.errorMessages = nextErrors; this.updatedAt = updatedAt; } diff --git a/ui/frontend/src/sync/order-load.ts b/ui/frontend/src/sync/order-load.ts index e992750..56a0b5e 100644 --- a/ui/frontend/src/sync/order-load.ts +++ b/ui/frontend/src/sync/order-load.ts @@ -63,6 +63,17 @@ export class OrderLoadError extends Error { export interface FetchedOrder { commands: OrderCommand[]; + // Per-command status keyed by cmdId. Populated from the engine's + // stored order so a returning player sees the same per-command + // verdict (applied / rejected) the previous submission produced — + // not a synthetic "applied" derived from the local cache. + statuses: Map; + // Per-command engine-formatted error code/message, keyed by cmdId. + // Both maps carry an entry for every loaded command; the value is + // null when the command was applied (no error). The message lets + // the UI surface the rejection reason without a code → text catalog. + errorCodes: Map; + errorMessages: Map; updatedAt: number; } @@ -119,7 +130,13 @@ function decodeResponse(payload: Uint8Array): FetchedOrder { const buffer = new ByteBuffer(payload); const response = UserGamesOrderGetResponse.getRootAsUserGamesOrderGetResponse(buffer); if (!response.found()) { - return { commands: [], updatedAt: 0 }; + return { + commands: [], + statuses: new Map(), + errorCodes: new Map(), + errorMessages: new Map(), + updatedAt: 0, + }; } const order = response.order(); if (order === null) { @@ -130,6 +147,9 @@ function decodeResponse(payload: Uint8Array): FetchedOrder { ); } const commands: OrderCommand[] = []; + const statuses = new Map(); + const errorCodes = new Map(); + const errorMessages = new Map(); const length = order.commandsLength(); for (let i = 0; i < length; i++) { const item = order.commands(i); @@ -137,9 +157,20 @@ function decodeResponse(payload: Uint8Array): FetchedOrder { const cmd = decodeCommand(item); if (cmd === null) continue; commands.push(cmd); + // The engine echoes `cmd_applied = false` only when the order + // was rejected per-command; missing / true both mean applied. + const applied = item.cmdApplied(); + statuses.set(cmd.id, applied === false ? "rejected" : "applied"); + const code = item.cmdErrorCode(); + errorCodes.set(cmd.id, code === null ? null : Number(code)); + const msg = item.cmdErrorMessage(); + errorMessages.set(cmd.id, msg === null ? null : msg); } return { commands, + statuses, + errorCodes, + errorMessages, updatedAt: Number(order.updatedAt()), }; } diff --git a/ui/frontend/src/sync/submit.ts b/ui/frontend/src/sync/submit.ts index dba5315..2acc5ba 100644 --- a/ui/frontend/src/sync/submit.ts +++ b/ui/frontend/src/sync/submit.ts @@ -82,6 +82,7 @@ export interface SubmitSuccess { ok: true; results: Map; errorCodes: Map; + errorMessages: Map; updatedAt: number; } @@ -511,6 +512,7 @@ function decodeOrderResponse( ): SubmitSuccess { const results = new Map(); const errorCodes = new Map(); + const errorMessages = new Map(); let updatedAt = 0; if (payload.length === 0) { @@ -518,8 +520,9 @@ function decodeOrderResponse( for (const cmd of commands) { results.set(cmd.id, "applied"); errorCodes.set(cmd.id, null); + errorMessages.set(cmd.id, null); } - return { ok: true, results, errorCodes, updatedAt }; + return { ok: true, results, errorCodes, errorMessages, updatedAt }; } const buffer = new ByteBuffer(payload); @@ -531,8 +534,9 @@ function decodeOrderResponse( for (const cmd of commands) { results.set(cmd.id, "applied"); errorCodes.set(cmd.id, null); + errorMessages.set(cmd.id, null); } - return { ok: true, results, errorCodes, updatedAt }; + return { ok: true, results, errorCodes, errorMessages, updatedAt }; } for (let i = 0; i < length; i++) { @@ -542,8 +546,10 @@ function decodeOrderResponse( if (cmdId === null) continue; const applied = item.cmdApplied(); const errorCode = item.cmdErrorCode(); + const errorMessage = item.cmdErrorMessage(); results.set(cmdId, applied === false ? "rejected" : "applied"); errorCodes.set(cmdId, errorCode === null ? null : Number(errorCode)); + errorMessages.set(cmdId, errorMessage === null ? null : errorMessage); } // Defensive: any submitted command not echoed back falls back to @@ -552,10 +558,11 @@ function decodeOrderResponse( if (!results.has(cmd.id)) { results.set(cmd.id, "applied"); errorCodes.set(cmd.id, null); + errorMessages.set(cmd.id, null); } } - return { ok: true, results, errorCodes, updatedAt }; + return { ok: true, results, errorCodes, errorMessages, updatedAt }; } function decodeError( diff --git a/ui/frontend/tests/helpers/fake-order-client.ts b/ui/frontend/tests/helpers/fake-order-client.ts index 5be01d3..6286d4f 100644 --- a/ui/frontend/tests/helpers/fake-order-client.ts +++ b/ui/frontend/tests/helpers/fake-order-client.ts @@ -174,11 +174,25 @@ export function recordingClient( * decode a realistic payload without standing up a full mock * gateway. */ +/** + * FakeFetchStatus carries the per-command result fields the engine + * would echo on `user.games.order.get` (cmdApplied / cmdErrorCode / + * cmdErrorMessage). The helper omits each set only when the + * corresponding pointer is `undefined`, mimicking the engine's + * `omitempty` semantics. + */ +export interface FakeFetchStatus { + applied?: boolean; + errorCode?: number; + errorMessage?: string; +} + export function fakeFetchClient( gameId: string, commands: OrderCommand[], updatedAt: number, found = true, + statuses?: Record, ): { client: GalaxyClient } { const client: GalaxyClient = { async executeCommand(messageType: string) { @@ -187,7 +201,7 @@ export function fakeFetchClient( } return { resultCode: "ok", - payloadBytes: encodeOrderGet(gameId, commands, updatedAt, found), + payloadBytes: encodeOrderGet(gameId, commands, updatedAt, found, statuses ?? {}), }; }, } as unknown as GalaxyClient; @@ -200,6 +214,10 @@ function encodeApplied( applied: boolean, ): Uint8Array { const builder = new Builder(256); + // See `encodeOrderGet` — flatbuffers@25 elides `cmd_applied=false` + // against its int8 default; mirror the Go transcoder's force-slot + // behaviour so the boolean survives the round trip in tests. + builder.forceDefaults(true); const itemOffsets = cmdIds.map((id) => { const cmdIdOffset = builder.createString(id); const nameOffset = builder.createString("ignored"); @@ -235,8 +253,15 @@ function encodeOrderGet( commands: OrderCommand[], updatedAt: number, found: boolean, + statuses: Record, ): Uint8Array { const builder = new Builder(256); + // flatbuffers@25 elides fields equal to their generated default; the + // Go transcoder works around this via explicit `Slot()` calls. + // Mirror that behaviour here so `cmd_applied=false` and + // `cmd_error_code=0` round-trip through the helper instead of being + // silently dropped. + builder.forceDefaults(true); let orderOffset = 0; if (found) { @@ -246,6 +271,11 @@ function encodeOrderGet( } const cmdIdOffset = builder.createString(cmd.id); const nameOffset = builder.createString(cmd.name); + const status = statuses[cmd.id] ?? {}; + const errMsgOffset = + status.errorMessage !== undefined + ? builder.createString(status.errorMessage) + : 0; const inner = CommandPlanetRename.createCommandPlanetRename( builder, BigInt(cmd.planetNumber), @@ -253,8 +283,17 @@ function encodeOrderGet( ); CommandItem.startCommandItem(builder); CommandItem.addCmdId(builder, cmdIdOffset); + if (status.applied !== undefined) { + CommandItem.addCmdApplied(builder, status.applied); + } + if (status.errorCode !== undefined) { + CommandItem.addCmdErrorCode(builder, BigInt(status.errorCode)); + } CommandItem.addPayloadType(builder, CommandPayload.CommandPlanetRename); CommandItem.addPayload(builder, inner); + if (errMsgOffset !== 0) { + CommandItem.addCmdErrorMessage(builder, errMsgOffset); + } return CommandItem.endCommandItem(builder); }); const commandsVec = UserGamesOrder.createCommandsVector(builder, itemOffsets); diff --git a/ui/frontend/tests/order-draft.test.ts b/ui/frontend/tests/order-draft.test.ts index 06c9c01..ec7533e 100644 --- a/ui/frontend/tests/order-draft.test.ts +++ b/ui/frontend/tests/order-draft.test.ts @@ -10,12 +10,13 @@ import "@testing-library/jest-dom/vitest"; import "fake-indexeddb/auto"; import { waitFor } from "@testing-library/svelte"; -import { afterEach, beforeEach, describe, expect, test } from "vitest"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import type { IDBPDatabase } from "idb"; import { IDBCache } from "../src/platform/store/idb-cache"; import { openGalaxyDB, type GalaxyDB } from "../src/platform/store/idb"; import type { Cache } from "../src/platform/store/index"; +import type { GalaxyClient } from "../src/api/galaxy-client"; import { OrderDraftStore } from "../src/sync/order-draft.svelte"; import type { OrderCommand } from "../src/sync/order-types"; @@ -448,6 +449,51 @@ describe("OrderDraftStore", () => { store.dispose(); }); + test("hydrateFromServer preserves per-command rejected status and error message", async () => { + const { fakeFetchClient } = await import("./helpers/fake-order-client"); + const { client } = fakeFetchClient( + GAME_ID, + [ + { + kind: "planetRename", + id: "ok-1", + planetNumber: 7, + name: "OkTown", + }, + { + kind: "planetRename", + id: "bad-1", + planetNumber: 8, + name: "Doomed", + }, + ], + 42, + true, + { + "ok-1": { applied: true, errorCode: 0 }, + "bad-1": { + applied: false, + errorCode: 3003, + errorMessage: 'Entity does not exists: planet #99', + }, + }, + ); + + const store = new OrderDraftStore(); + await store.init({ cache, gameId: GAME_ID }); + await store.hydrateFromServer({ client, turn: 5 }); + + expect(store.commands).toHaveLength(2); + expect(store.statuses["ok-1"]).toBe("applied"); + expect(store.statuses["bad-1"]).toBe("rejected"); + expect(store.errorMessages["ok-1"]).toBeUndefined(); + expect(store.errorMessages["bad-1"]).toBe( + "Entity does not exists: planet #99", + ); + expect(store.syncStatus).toBe("synced"); + store.dispose(); + }); + test("hydrate empties the local cache when server returns found=false", async () => { // First seed a local draft. const seeded = new OrderDraftStore(); @@ -555,6 +601,85 @@ describe("OrderDraftStore auto-sync", () => { store.dispose(); }); + test("per-command rejection in a successful response keeps syncStatus 'synced'", async () => { + // Per-command rejection means the round trip succeeded — only + // individual commands were rejected by the in-game rules — so + // the sync bar must stay green. A blanket `syncStatus = "error"` + // + Retry button only fits genuine transport / engine failures. + const { Builder } = await import("flatbuffers"); + const fbs = await import("../src/proto/galaxy/fbs/order"); + const common = await import("../src/proto/galaxy/fbs/common"); + const { uuidToHiLo } = await import("../src/api/game-state"); + + const cmd = { + kind: "planetRename" as const, + id: "rej-1", + planetNumber: 9, + name: "Doomed", + }; + + const exec = vi.fn(async () => { + const builder = new Builder(256); + builder.forceDefaults(true); + const cmdIdOffset = builder.createString(cmd.id); + const nameOffset = builder.createString("ignored"); + const errMsg = builder.createString( + "Entity does not exists: planet #99", + ); + const inner = fbs.CommandPlanetRename.createCommandPlanetRename( + builder, + BigInt(0), + nameOffset, + ); + fbs.CommandItem.startCommandItem(builder); + fbs.CommandItem.addCmdId(builder, cmdIdOffset); + fbs.CommandItem.addCmdApplied(builder, false); + fbs.CommandItem.addCmdErrorCode(builder, BigInt(3003)); + fbs.CommandItem.addPayloadType( + builder, + fbs.CommandPayload.CommandPlanetRename, + ); + fbs.CommandItem.addPayload(builder, inner); + fbs.CommandItem.addCmdErrorMessage(builder, errMsg); + const item = fbs.CommandItem.endCommandItem(builder); + const commandsVec = fbs.UserGamesOrderResponse.createCommandsVector( + builder, + [item], + ); + const [hi, lo] = uuidToHiLo(GAME_ID); + const gameIdOffset = common.UUID.createUUID(builder, hi, lo); + fbs.UserGamesOrderResponse.startUserGamesOrderResponse(builder); + fbs.UserGamesOrderResponse.addGameId(builder, gameIdOffset); + fbs.UserGamesOrderResponse.addUpdatedAt(builder, BigInt(99)); + fbs.UserGamesOrderResponse.addCommands(builder, commandsVec); + const offset = fbs.UserGamesOrderResponse.endUserGamesOrderResponse( + builder, + ); + builder.finish(offset); + return { + resultCode: "ok", + payloadBytes: builder.asUint8Array(), + }; + }); + const client = { executeCommand: exec } as unknown as GalaxyClient; + + const store = new OrderDraftStore(); + await store.init({ cache, gameId: GAME_ID }); + store.bindClient(client); + await store.add(cmd); + await waitFor(() => { + expect(exec).toHaveBeenCalled(); + expect(store.statuses[cmd.id]).toBe("rejected"); + }); + + expect(store.syncStatus).toBe("synced"); + expect(store.syncError).toBeNull(); + expect(store.errorMessages[cmd.id]).toBe( + "Entity does not exists: planet #99", + ); + store.dispose(); + }); + test("non-ok response marks every in-flight command as rejected", async () => { const { recordingClient } = await import("./helpers/fake-order-client"); const handle = recordingClient(GAME_ID, "rejected"); diff --git a/ui/frontend/tests/order-queue.test.ts b/ui/frontend/tests/order-queue.test.ts index 0091420..39ad5b5 100644 --- a/ui/frontend/tests/order-queue.test.ts +++ b/ui/frontend/tests/order-queue.test.ts @@ -69,6 +69,7 @@ function success(updatedAt = 1): SubmitSuccess { ok: true, results: new Map([["id", "applied"]]), errorCodes: new Map([["id", null]]), + errorMessages: new Map([["id", null]]), updatedAt, }; } diff --git a/ui/frontend/tests/submit.test.ts b/ui/frontend/tests/submit.test.ts index 369bc0a..13d0380 100644 --- a/ui/frontend/tests/submit.test.ts +++ b/ui/frontend/tests/submit.test.ts @@ -40,13 +40,28 @@ function mockClient( } function buildResponse( - commands: { id: string; applied: boolean | null; errorCode: number | null }[], + commands: { + id: string; + applied: boolean | null; + errorCode: number | null; + errorMessage?: string | null; + }[], updatedAt: number, ): Uint8Array { const builder = new Builder(256); + // flatbuffers@25 skips fields equal to their generated default when + // writing — the Go transcoder works around this via explicit + // `Slot()` calls. This fixture stands in for the engine + gateway, + // so we flip `forceDefaults` to keep `cmd_applied=false` (=== int8 + // default 0) and `cmd_error_code=0` from being silently elided. + builder.forceDefaults(true); const itemOffsets = commands.map((c) => { const cmdIdOffset = builder.createString(c.id); const nameOffset = builder.createString("ignored"); + const errMsgOffset = + c.errorMessage !== undefined && c.errorMessage !== null + ? builder.createString(c.errorMessage) + : 0; const payloadOffset = CommandPlanetRename.createCommandPlanetRename( builder, BigInt(0), @@ -60,6 +75,9 @@ function buildResponse( } CommandItem.addPayloadType(builder, CommandPayload.CommandPlanetRename); CommandItem.addPayload(builder, payloadOffset); + if (errMsgOffset !== 0) { + CommandItem.addCmdErrorMessage(builder, errMsgOffset); + } return CommandItem.endCommandItem(builder); }); const commandsVec = UserGamesOrderResponse.createCommandsVector(builder, itemOffsets);