dedicated menu for resources movement #24

Closed
opened 2026-08-11 22:09:23 +00:00 by metaheavens · 3 comments
Owner

moving resources between the fleet and a building should have a dedicated action button (not inside the specific menu, like ship building or tech tree). this button should open a dedicated menu to move resources between fleet and the current building. mothership should not automatically fetch the fleet resources when touching it. when inside a building, they should all share the same top level bar of available resources, this would unify ui between ship crafter and the skill tree a bit

moving resources between the fleet and a building should have a dedicated action button (not inside the specific menu, like ship building or tech tree). this button should open a dedicated menu to move resources between fleet and the current building. mothership should not automatically fetch the fleet resources when touching it. when inside a building, they should all share the same top level bar of available resources, this would unify ui between ship crafter and the skill tree a bit
Author
Owner

Implemented.

Dedicated action button. A new transfer button sits in the UI action column, visible whenever the fleet is on any building. It sits alongside the action button rather than being another thing that button turns into — crafting and moving ore are independent choices at the same structure. It opens a dedicated TransferMenu: both directions, two amounts each (a 25-ore batch, or All).

The mothership is no longer a special case. GameState.building_holdings() returns the bank for a mothership and per-building stock for everything else, so one panel serves every structure — unloading at a mothership banks the ore, loading there draws on it. New transfer_to_building / transfer_to_fleet primitives cap by what is actually at each end, so there is no overdraft in either direction.

Mothership auto-fetch removed. BuildingField._process is gone entirely; arriving at the mothership no longer banks the hold. That edge-triggered move existed to stop ore you had just withdrawn being instantly re-banked. With transfers explicit and bidirectional it was the odd case out, and it silently overrode a player who had loaded the hold in order to carry ore somewhere else.

Shared resource bar. New ResourceBar.gd — BANK / HOLD / HERE in the game's own hexagon-and-segment style — now heads the craft, research, lab and transfer menus. The crafter used to spell those figures into a run-on status string and the research screen printed a bank total inside a button caption; both are gone, along with LoadOreButton (research) and StoreOreButton (crafter).

Craft menu reflowed. Removing "Store ore here" freed a button-sized block at the bottom, so the stack moved down into it — without that, the new header squeezed the status line into the shield button. Menu titles were also narrowed so they stop running under the close button; that overlap predated this change but moving the titles up to make room for the bar made it worse.

Tests: 53/53 passing. New test_resource_transfer.gd covers the button appearing and binding to the right building, both directions at an ordinary building, the mothership-is-the-bank case, a real tap through the input path, empty-end messages, the bar following the panel, and ore conservation. test_fleet_cargo and test_manual_supply_route asserted the old auto-banking, so they now check both halves: parking changes nothing, the explicit transfer moves everything. test_crafter_feedback lost its store-ore stages.


One loose end, not done here: deliver_cargo, withdraw_to_fleet and unload_into_building are now called only from tests, and cargo_delivered has no listeners (which predates this change). Folding them into the new primitives means touching two passing tests that are not about this issue, so I left them. Worth its own issue if wanted.

Implemented. **Dedicated action button.** A new transfer button sits in the UI action column, visible whenever the fleet is on any building. It sits *alongside* the action button rather than being another thing that button turns into — crafting and moving ore are independent choices at the same structure. It opens a dedicated `TransferMenu`: both directions, two amounts each (a 25-ore batch, or All). **The mothership is no longer a special case.** `GameState.building_holdings()` returns the bank for a mothership and per-building stock for everything else, so one panel serves every structure — unloading at a mothership banks the ore, loading there draws on it. New `transfer_to_building` / `transfer_to_fleet` primitives cap by what is actually at each end, so there is no overdraft in either direction. **Mothership auto-fetch removed.** `BuildingField._process` is gone entirely; arriving at the mothership no longer banks the hold. That edge-triggered move existed to stop ore you had just withdrawn being instantly re-banked. With transfers explicit and bidirectional it was the odd case out, and it silently overrode a player who had loaded the hold in order to carry ore somewhere else. **Shared resource bar.** New `ResourceBar.gd` — BANK / HOLD / HERE in the game's own hexagon-and-segment style — now heads the craft, research, lab and transfer menus. The crafter used to spell those figures into a run-on status string and the research screen printed a bank total inside a button caption; both are gone, along with `LoadOreButton` (research) and `StoreOreButton` (crafter). **Craft menu reflowed.** Removing "Store ore here" freed a button-sized block at the bottom, so the stack moved down into it — without that, the new header squeezed the status line into the shield button. Menu titles were also narrowed so they stop running under the close button; that overlap predated this change but moving the titles up to make room for the bar made it worse. **Tests: 53/53 passing.** New `test_resource_transfer.gd` covers the button appearing and binding to the right building, both directions at an ordinary building, the mothership-is-the-bank case, a real tap through the input path, empty-end messages, the bar following the panel, and ore conservation. `test_fleet_cargo` and `test_manual_supply_route` asserted the old auto-banking, so they now check both halves: parking changes nothing, the explicit transfer moves everything. `test_crafter_feedback` lost its store-ore stages. --- One loose end, not done here: `deliver_cargo`, `withdraw_to_fleet` and `unload_into_building` are now called only from tests, and `cargo_delivered` has no listeners (which predates this change). Folding them into the new primitives means touching two passing tests that are not about this issue, so I left them. Worth its own issue if wanted.
Author
Owner

Loose end closed in b6427b0.

Removed withdraw_to_fleet, unload_into_building, deliver_cargo and the cargo_delivered signal. Each function was a special case of transfer_to_building / transfer_to_fleet, none had a caller left outside tests, and the signal had no listeners at all (declared once, emitted twice, connected nowhere).

It was not only tidying. unload_into_building routed through deposit_to_building, which does not special-case the mothership - it writes b.stock, while building_holdings() returns rocks for a mothership and ignores stock entirely. So unload_into_building(mothership_id) would have parked ore in a field nothing reads: it would have vanished from the player's view with no error. Nothing called it that way, but it is a plausibly-named function sitting there for someone to reach for later. transfer_to_building handles the mothership correctly, which is the whole point of the #24 primitives.

The cost estimate in the note above was wrong, in the useful direction. I expected folding this in to be awkward for test_cargo_readout, which places no buildings and is a pure display test. It is not awkward: the transfer primitives work off GameState records rather than spawned nodes, so one add_building() in setup is enough - no BuildingField, no waiting for a node, and it stays a display test. test_manual_supply_route already had a mothership, so its three call sites moved across mechanically, and its overdraft check still holds because transfer_to_fleet caps by building_holdings(), which is the bank at a mothership.

CARGO_LOAD_BATCH stays - the transfer menu's batch buttons use it. Only its role as withdraw_to_fleet's default argument is gone.

Both touched tests still assert what they claim:

withdrew 6 at the mothership -> bank=34 hold=6
hold kept its 6 across an arrival (no auto-banking)
withdrawing 999 from a bank of 3 took 3

carrying 7 -> cargo=7 banked=42
delivered -> cargo hidden, banked shows 49

Full suite: 53/53.

Loose end closed in b6427b0. Removed `withdraw_to_fleet`, `unload_into_building`, `deliver_cargo` and the `cargo_delivered` signal. Each function was a special case of `transfer_to_building` / `transfer_to_fleet`, none had a caller left outside tests, and the signal had no listeners at all (declared once, emitted twice, connected nowhere). **It was not only tidying.** `unload_into_building` routed through `deposit_to_building`, which does *not* special-case the mothership - it writes `b.stock`, while `building_holdings()` returns `rocks` for a mothership and ignores `stock` entirely. So `unload_into_building(mothership_id)` would have parked ore in a field nothing reads: it would have vanished from the player's view with no error. Nothing called it that way, but it is a plausibly-named function sitting there for someone to reach for later. `transfer_to_building` handles the mothership correctly, which is the whole point of the #24 primitives. **The cost estimate in the note above was wrong**, in the useful direction. I expected folding this in to be awkward for `test_cargo_readout`, which places no buildings and is a pure display test. It is not awkward: the transfer primitives work off `GameState` records rather than spawned nodes, so one `add_building()` in setup is enough - no `BuildingField`, no waiting for a node, and it stays a display test. `test_manual_supply_route` already had a mothership, so its three call sites moved across mechanically, and its overdraft check still holds because `transfer_to_fleet` caps by `building_holdings()`, which *is* the bank at a mothership. `CARGO_LOAD_BATCH` stays - the transfer menu's batch buttons use it. Only its role as `withdraw_to_fleet`'s default argument is gone. Both touched tests still assert what they claim: ``` withdrew 6 at the mothership -> bank=34 hold=6 hold kept its 6 across an arrival (no auto-banking) withdrawing 999 from a bank of 3 took 3 carrying 7 -> cargo=7 banked=42 delivered -> cargo hidden, banked shows 49 ``` Full suite: **53/53**.
Author
Owner

Implemented and verified in commit 8255246 (Add timed craft queue and hangar).

The dedicated resource-transfer flow remains covered by test_resource_transfer.gd, which passes from the tracked checkout. I also fixed the test runner so memory pressure and initialization errors report clearly instead of appearing to hang.

Closing #24 as complete.

Implemented and verified in commit 8255246 (`Add timed craft queue and hangar`). The dedicated resource-transfer flow remains covered by `test_resource_transfer.gd`, which passes from the tracked checkout. I also fixed the test runner so memory pressure and initialization errors report clearly instead of appearing to hang. Closing #24 as complete.
Sign in to join this conversation.
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
metaheavens/spaceship#24
No description provided.