Skip to content

Gather orders on fog-remembered resources are accepted, then silently dropped by the sim #160

Description

@Knorcedger

Player problem

Tap a resource the player only remembers through the fog (drawn from PlayerResourceMemory) after the real node is mined out or chopped down: the game accepts the order — "Gather" undo toast, military escorts even walk over — but the villagers never move and no feedback explains why. This is the exact dropped-command UX #123/#128 set out to eliminate.

Verified chain (at 3ae083c)

  • world.ts:735-738pickAt pushes fog projections from resourceMemory.hiddenMissing(state), which yields exactly the records whose id is not in state.entities under explored fog (resourceMemory.ts:194-205). The projection is kind:'resource', gaia, remembered amountLeft > 0, so it satisfies isVillagerGatherTarget (input.ts:113-115).
  • input.ts:736-741 — the gather command is issued with the stale targetId, no state.entities.has check; can('gather') passes.
  • sim/gather.ts:113-116 — the sim drops the command silently (if (!target || !view) return;), pinned by sim/flee.test.ts:333 and sim/qafuzz.test.ts:80.
  • game.ts:945-946 — order-feedback ping renders only for a live entity, so nothing appears; CommandAdmission previews only build/train/research (admission.ts:7-11), so nothing blocks the order either.

Proposed fix (verified to apply cleanly; needs the additions below)

Downgrade to an honest move toward the remembered tile — arriving reveals it, which is what clears the ghost. This mirrors the input layer's own "downgraded to an HONEST move" convention (input.ts:714-719) and the sim's rally degradation (sim/commands.ts:241-244).

--- a/packages/game/src/input.ts
+++ b/packages/game/src/input.ts
@@ -734,6 +734,15 @@ export class InputController {
       return;
     }
     if (gatherTarget && villagers.length > 0) {
+      // A remembered resource whose node is GONE from the sim (mined out under
+      // the fog): pickAt still offers it so the ghost stays orderable, but the
+      // sim silently drops a gather at a stale id. Walk the group there instead
+      // — arriving reveals the tile, which is what clears the ghost.
+      if (gatherTarget.kind === 'resource'
+        && this.host.getState().entities?.has(gatherTarget.id) === false) {
+        moveTo(gatherTarget.x, gatherTarget.y, unitIds, 'Move');
+        return;
+      }
       if (can('gather')) {
         this.host.issueWithUndo(
           { kind: 'gather', player: human, units: villagers.map((e) => e.id), targetId: gatherTarget.id },

Completions the review of this draft identified:

  1. The depleted-tree variant is not covered. Wood depletes to a stump that stays in state.entities (sim/gather.ts:153-160, sim/entities.ts:152-157), so the entities.has guard never fires — yet the memory still shows the full tree, the gather is issued, and classifyGatherTarget drops it on amountLeft <= 0 (gather.ts:73), this time with a ping on the invisible stump. The guard should also downgrade when the live record is no longer a valid gather target.
  2. Add an input.test.ts case (the mock-pickAt harness at line 94 fits) pinning both variants.
  3. Consider the toast-label convention (a bare 'Move' drops the explanatory suffix other downgrades carry), and reuse the in-scope st instead of re-calling getState().

Acceptance criteria

  • Ordering villagers onto a remembered-but-gone (or remembered-but-depleted) resource moves them to its tile; no silently-dropped command remains reachable from pickAt ghosts.
  • New test coverage for both variants; npm run typecheck and npm run test green.

Related observation (no action)

An automated sweep also flagged packages/ai/src/resourceMemory.ts vs packages/game/src/resourceMemory.ts as duplicate mechanisms. Verified: the divergence is intentional (deterministic bot knowledge with LRU cap and depleted-node deletion vs. presentation-layer ghosts with stumps, persistence, and projection caching; each pinned by its own suite), and consolidating into @bf/game is blocked by the dependency direction (@bf/game already imports @bf/ai, ARCHITECTURE.md:5-11). Only ~25-30 lines are truly shared. Recorded here so it is not re-reported.

Provenance

Found by an automated review pass that over-scoped into merged history during #159; every claim above was then independently verified against the working tree at 3ae083c. The feature landed via #98/#106/#136.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions