liamjd/UntitledColonyBuilder

Fork 0

Split InspectorPanel into sections so it can host controls #152

New issue

Closed

opened 2026-10-10 18:17:04 +01:00 by liamjd · 1 comment

liamjd commented

2026-10-10 18:17:04 +01:00

Owner

Part of milestone #6 (Selection and inspection). Follows #142. Blocks #136.

The inspector from #142 turns Dictionary[String, String] rows into Labels, and rebuilds every Label on each tick. That works for read-only text, but it cannot host controls. A button rebuilt ten times a second loses its hover and pressed state, and a rename LineEdit would lose what is being typed.

Controls are coming, starting with #136's worker slots and later things like wages, renaming a pawn, or pausing construction. Nobody knows yet exactly which. So rather than make the rows describe widgets, split the panel into sections: small self-contained scenes, stacked in the panel. Each decides whether it applies to the selected entity and updates itself in place. A new control is then a new section, not a change to the panel.

This is a refactor. The panel should show exactly what it shows today, using one section.

Rough shape

These are starting points, not a spec. Liam is working out the detail.

An abstract InspectorSection (a Control) that answers applies_to(entity) and refresh(entity).
The panel keeps the timing: it listens to selection_changed and GameClock.tick as now, shows or hides sections, and refreshes the visible ones. Sections do not subscribe to those signals themselves.
Today's rows become the first section, reusing title_for / pawn_rows / building_rows.
Dependencies reach sections from the panel, as one small context object (tasks, buildings, later pawns) rather than more fields wired in game.gd, much as TaskContext does for tasks.
Sections change the game only through registry methods (for example TaskManager.set_open_slots()), never by writing records directly.

Acceptance criteria

An abstract section type exists, and the panel renders the selection through sections rather than building rows itself.
Today's information display is the first section. The panel looks and behaves as it does after #142: checked headed with tools/screenshot_inspector.gd.
Adding a section needs no change to the panel's code.
A section that does not apply to the selected entity is hidden.
The #142 tests still pass, with changes only where a test reached into something that moved: %Rows, the row builders and their constants (now on DetailsSection), and panel.tasks / panel.buildings (now panel.context). See Decisions.
At least one test exercises a section that does not apply, for example a stub section that applies only to buildings.

Decisions (2026-10-11)

Row builders move to the section. pawn_rows(), building_rows(), IDLE_TEXT and NONE_TEXT move to DetailsSection, so the panel no longer knows what a pawn row is. title_for() stays on the panel, because the title belongs to the panel. The #142 builder tests change InspectorPanel. to DetailsSection. and nothing else.
A context object replaces the panel's fields. panel.context: InspectorContext holds tasks and buildings, and task_for() moves onto it from the panel. game.gd builds it the way it builds TaskContext. There are no forwarding panel.tasks / panel.buildings properties, so there is only one way to inject.

Out of scope

Any interactive control. The first one is #136.
Styling, pending art and theming (#21).

Part of milestone #6 (Selection and inspection). Follows #142. Blocks #136. The inspector from #142 turns `Dictionary[String, String]` rows into Labels, and rebuilds every Label on each tick. That works for read-only text, but it cannot host controls. A button rebuilt ten times a second loses its hover and pressed state, and a rename `LineEdit` would lose what is being typed. Controls are coming, starting with #136's worker slots and later things like wages, renaming a pawn, or pausing construction. Nobody knows yet exactly which. So rather than make the rows describe widgets, split the panel into **sections**: small self-contained scenes, stacked in the panel. Each decides whether it applies to the selected entity and updates itself in place. A new control is then a new section, not a change to the panel. This is a refactor. The panel should show exactly what it shows today, using one section. ## Rough shape These are starting points, not a spec. Liam is working out the detail. - An abstract `InspectorSection` (a Control) that answers `applies_to(entity)` and `refresh(entity)`. - The panel keeps the timing: it listens to `selection_changed` and `GameClock.tick` as now, shows or hides sections, and refreshes the visible ones. Sections do not subscribe to those signals themselves. - Today's rows become the first section, reusing `title_for` / `pawn_rows` / `building_rows`. - Dependencies reach sections from the panel, as one small context object (tasks, buildings, later pawns) rather than more fields wired in `game.gd`, much as `TaskContext` does for tasks. - Sections change the game only through registry methods (for example `TaskManager.set_open_slots()`), never by writing records directly. ## Acceptance criteria - [ ] An abstract section type exists, and the panel renders the selection through sections rather than building rows itself. - [ ] Today's information display is the first section. The panel looks and behaves as it does after #142: checked headed with `tools/screenshot_inspector.gd`. - [ ] Adding a section needs no change to the panel's code. - [ ] A section that does not apply to the selected entity is hidden. - [ ] The #142 tests still pass, with changes only where a test reached into something that moved: `%Rows`, the row builders and their constants (now on `DetailsSection`), and `panel.tasks` / `panel.buildings` (now `panel.context`). See Decisions. - [ ] At least one test exercises a section that does not apply, for example a stub section that applies only to buildings. ## Decisions (2026-10-11) - **Row builders move to the section.** `pawn_rows()`, `building_rows()`, `IDLE_TEXT` and `NONE_TEXT` move to `DetailsSection`, so the panel no longer knows what a pawn row is. `title_for()` stays on the panel, because the title belongs to the panel. The #142 builder tests change `InspectorPanel.` to `DetailsSection.` and nothing else. - **A context object replaces the panel's fields.** `panel.context: InspectorContext` holds `tasks` and `buildings`, and `task_for()` moves onto it from the panel. `game.gd` builds it the way it builds `TaskContext`. There are no forwarding `panel.tasks` / `panel.buildings` properties, so there is only one way to inject. ## Out of scope - Any interactive control. The first one is #136. - Styling, pending art and theming (#21).

liamjd added this to the Selection and inspection milestone

2026-10-10 18:17:08 +01:00

liamjd added the

label

2026-10-10 18:17:08 +01:00

liamjd referenced this issue

2026-10-10 18:17:18 +01:00

Worker slot control on a selected construction site #136

liamjd added a new dependency

2026-10-10 18:17:18 +01:00

#136 Worker slot control on a selected construction site

liamjd commented

2026-10-11 07:28:37 +01:00

Author

Owner

Spec (drafted with Claude)

The tests are on branch 152-inspector-panel-sections: test/unit/test_inspector_sections.gd (new), test/unit/test_inspector_panel.gd (renames only), and test/support/stub_inspector_section.gd (new). They and this outline are one spec.

2.1 Files

File	New / changed	Change
`ui/inspector/inspector_section.gd`	new	`@abstract class_name InspectorSection extends VBoxContainer`. The contract: `applies_to()` and `refresh()`.
`ui/inspector/inspector_context.gd`	new	`class_name InspectorContext extends RefCounted`: `tasks`, `buildings`, `task_for()`.
`ui/inspector/sections/details_section.gd`	new	`class_name DetailsSection extends InspectorSection`. Today's rows: `pawn_rows`, `building_rows`, `IDLE_TEXT`, `NONE_TEXT`, `_render` all move here unchanged.
`ui/inspector/sections/details_section.tscn`	new	Root `DetailsSection` plus `%Rows`. See 2.6.
`ui/inspector/inspector_panel.gd`	changed	Loses `tasks`, `buildings`, the row builders and their constants, `_task_for`, `_render` and `rows_grid`. Gains `context`, `sections_box` and `sections()`. `refresh()` drives the sections.
`ui/inspector/inspector_panel.tscn`	changed	`%Rows` (GridContainer) is replaced by `%Sections` (VBoxContainer) holding one `%DetailsSection` instance.
`common/game.gd`	changed	Builds an `InspectorContext` and assigns it to `inspector_panel.context`, replacing the two field assignments.
`tools/screenshot_inspector.gd`	unchanged	Reused for the headed check. It reaches no node inside the panel.

2.2 API: full signatures

# ui/inspector/inspector_section.gd - @abstract class_name InspectorSection extends VBoxContainer
@abstract func applies_to(entity: Entity, ctx: InspectorContext) -> bool
@abstract func refresh(entity: Entity, ctx: InspectorContext) -> void

# ui/inspector/inspector_context.gd - class_name InspectorContext extends RefCounted
var tasks: TaskManager = null    # null is supported: a pawn's task resolves to null
var buildings: Buildings = null  # null is supported: a building's task resolves to null
func task_for(entity: Entity) -> Task   # null: no task, the registry it needs is null, entity is null, or not a Pawn/Building

# ui/inspector/sections/details_section.gd - class_name DetailsSection extends InspectorSection
const IDLE_TEXT := "idle"   # moved from InspectorPanel
const NONE_TEXT := "none"   # moved from InspectorPanel
@onready var rows_grid: GridContainer = %Rows
static func pawn_rows(pawn: Pawn, task: Task) -> Dictionary[String, String]          # moved verbatim
static func building_rows(building: Building, task: Task) -> Dictionary[String, String]  # moved verbatim
func applies_to(entity: Entity, _ctx: InspectorContext) -> bool
func refresh(entity: Entity, ctx: InspectorContext) -> void
func _render(rows: Dictionary[String, String]) -> void                               # moved verbatim

# ui/inspector/inspector_panel.gd - class_name InspectorPanel extends PanelContainer
var context: InspectorContext = InspectorContext.new()   # never null; game.gd replaces it with a filled one
@onready var title_lbl: Label = %TitleLbl                # unchanged
@onready var sections_box: VBoxContainer = %Sections     # replaces rows_grid
static func title_for(entity: Entity) -> String          # unchanged, stays here
func sections() -> Array[InspectorSection]
func refresh() -> void                                   # same signature, new body
func _on_selection_changed(_new: Entity, _previous: Entity) -> void  # unchanged
func _on_tick(_run_tick: int) -> void                    # unchanged
# removed: tasks, buildings, IDLE_TEXT, NONE_TEXT, pawn_rows, building_rows, rows_grid, _task_for, _render

InspectorSection extends VBoxContainer, not plain Control. A plain Control takes no minimum size from its children, so a section would collapse to zero height in the panel's VBox. A VBoxContainer stacks whatever the section holds. A section that wants a row of buttons (#136) puts an HBoxContainer inside it.

InspectorSection is @abstract, like EntityList, so nothing can instantiate it directly. Concrete sections are Nodes, so tests autofree them or free them with the panel. InspectorContext is RefCounted and is never freed.

Error paths: none. There is no push_error and no error return anywhere in this change, and no test matches on error text. A null registry and a null entity both resolve to a null task. panel.context is never set to null: game.gd replaces it, and nothing else assigns it.

2.3 Each method's job, in one line

New and changed:

InspectorSection.applies_to(entity, ctx): whether this section has anything to show for entity. The panel asks again on every refresh, so the answer can change while the same entity stays selected.
InspectorSection.refresh(entity, ctx): brings the section's nodes up to date for entity. It is only called while the section applies, and entity is never null. A section with controls updates its nodes in place here and does not rebuild them.
InspectorContext.task_for(entity): the body of today's InspectorPanel._task_for(), moved. For a Pawn: tasks.get_task(pawn.current_task_id) when tasks != null, else null. For a Building: buildings.task_for(building.id) when buildings != null, else null. Anything else, including null: null.
DetailsSection.applies_to(entity, _ctx): entity is Pawn or entity is Building.
DetailsSection.refresh(entity, ctx): for a pawn, _render(pawn_rows(entity as Pawn, ctx.task_for(entity))), and the same with building_rows for a building.
DetailsSection.pawn_rows / building_rows / _render: unchanged from #142. _render still frees and rebuilds the Labels. That is fine for read-only text, and it only touches this section's own %Rows.
InspectorPanel.sections(): the children of sections_box that are InspectorSection, in tree order. Other children, such as a separator, are skipped. It reads the tree on every call and never caches in _ready(), so a section added later is picked up.
InspectorPanel.refresh(): see 2.5.

Existing methods the new code relies on:

GameState.selected (setter): ignores a no-op assignment. Otherwise it accepts only null, a Pawn or a Building, and emits selection_changed(new, previous) itself. Anything else is a push_error and is not stored. So the panel never sees another entity type selected.
GameState._clear_if_selected(entity): connected to pawn_removed / building_removed, and sets selected = null, which emits selection_changed. This is how removal hides the panel. The panel has no handler of its own for removal.
TaskManager.get_task(task_id) -> Task: returns null for an unknown id. Never errors.
Buildings.task_for(building_id) -> Task: returns null when tasks == null. Otherwise it scans tasks.all() for a ConstructionTask or DeconstructionTask whose building_id matches. Returns null if none matches.

2.4 Signals

No new signals, and no signal changes. Sections connect to nothing: the panel holds all the timing.

Signal	Declared on	Custom / built-in	Emitted by (method)	Listened to by (method)	Connected where
`selection_changed(new: Entity, previous: Entity)`	`GameState`	custom	`GameState.selected` setter	`InspectorPanel._on_selection_changed()`	`InspectorPanel._ready()` (unchanged)
`tick(run_tick: int)`	`GameClock`	custom	`GameClock`	`InspectorPanel._on_tick()`	`InspectorPanel._ready()` (unchanged)

2.5 Wiring pattern

GameState.selection_changed -> InspectorPanel._on_selection_changed(_new, _previous)   # connected in InspectorPanel._ready()
                               calls refresh()

GameClock.tick              -> InspectorPanel._on_tick(_run_tick)                       # connected in InspectorPanel._ready()
                               if GameState.selected != null: refresh()

InspectorPanel.refresh():
    1. entity := GameState.selected
    2. if entity == null: visible = false, then return.            # conditional; sections are NOT touched or refreshed
    3. visible = true                                              # always, from here on
    4. title_lbl.text = title_for(entity)                          # always
    5. for section in sections():                                  # always
         a. section.visible = section.applies_to(entity, context)  # always, for every section
         b. if section.visible: section.refresh(entity, context)   # conditional: only sections that apply

Step 2 keeps a section from ever being handed null, so no section has to guard against it. Step 5a runs on every refresh, ticks included. That is how #136's slot control disappears when the site's task ends while the same building stays selected.

_ready() still calls refresh() once, so a panel made over an existing selection starts correct. The sections' own @onready vars are set by then, because children are readied before their parent.

game.gd, replacing lines 72-73 (still inside the not Engine.is_editor_hint() guard):

var inspector_context := InspectorContext.new()
inspector_context.tasks = task_manager
inspector_context.buildings = buildings
inspector_panel.context = inspector_context

GDScript rules this relies on:

Builders are pure and never emit. title_for, pawn_rows and building_rows stay static and only return values. task_for and applies_to read and never change anything.
The handler is the only thing driving refresh. Nothing here emits a signal.
Signals ignore return values. A section cannot answer the panel through a signal, so applies_to is a method call, not a signal.
Sections change the game only through registry methods (for example TaskManager.set_open_slots() in #136), never by writing to records directly. Nothing in this ticket writes at all.

2.6 Scene structure

inspector_panel.tscn
InspectorPanel (PanelContainer, inspector_panel.gd)     # visible = false and mouse_filter STOP stay as they are
└── MarginContainer
    └── VBoxContainer
        ├── %TitleLbl (Label)                           # unchanged
        └── %Sections (VBoxContainer)                   # replaces %Rows. Every InspectorSection child is a section
            └── %DetailsSection (instance of details_section.tscn)   # unique_name_in_owner set in the PANEL scene

details_section.tscn
DetailsSection (VBoxContainer, details_section.gd)
└── %Rows (GridContainer)                               # columns = 2, set in the scene

Delete %Rows from the panel scene and add %Sections as a new node, rather than retyping it, so the editor gives it a fresh unique_id. Keep the uid= and unique_id= values of every other node.
%DetailsSection has to be marked unique on the instance in the panel scene ("Access as Unique Name" on the instanced node). The tests reach it with panel.get_node("%DetailsSection"). %Rows is unique inside details_section.tscn only, which is what the section's own @onready uses.
The details section sits inside the panel's scene, not in game.tscn. "Adding a section" therefore means adding an instance under %Sections in inspector_panel.tscn, which changes the scene but not inspector_panel.gd.
The abstract base has no scene. Unlike EntityList, nothing inherits a layout: each section scene has its own concrete script on its root.
ui/inspector/sections/ is a new folder for section scenes, so #136's slot section sits beside this one.

Names the tests depend on: %Sections (through panel.sections_box), %DetailsSection, %TitleLbl (through panel.title_lbl), and %Rows (through DetailsSection.rows_grid). Renaming any of them breaks the tests.

2.7 Tests

test/unit/test_inspector_panel.gd (#142's tests). What they check is unchanged; only the paths moved:

InspectorPanel.pawn_rows / building_rows / IDLE_TEXT / NONE_TEXT become DetailsSection.….
panel.rows_grid becomes _details(panel).rows_grid, through a new _details() helper that returns %DetailsSection.
panel.tasks / panel.buildings become panel.context.tasks / panel.context.buildings.

test/unit/test_inspector_sections.gd (new):

InspectorContext.task_for: a pawn's task comes from the manager, a building's from the registry, and each is null without its registry. Null has no task.
Sections are discovered: %DetailsSection is the only section by default; a section added at runtime with add_child() is listed after it; a non-section child is neither listed nor hidden.
Applies / does not apply (stub section that applies only to buildings): hidden and never refreshed for a pawn; shown and refreshed with the building and with the panel's own context (including a replaced one); refreshed again on a tick; hidden on the next tick once it stops applying and shown again when it applies again; hidden when the selection switches to a pawn.
Deselect: the panel hides, and no section is refreshed with null, on the selection change or on a later tick.
DetailsSection: applies to a pawn and a building, not to another entity, and is visible in the panel for both.

test/support/stub_inspector_section.gd (new, no class_name, preloaded by path like stub_task.gd): applies only to buildings while applies is true, and records refresh_calls, last_entity and last_ctx.

Coverage against the acceptance criteria:

AC	Covered by
Abstract section type; the panel renders through sections	tests: `test_the_details_section_is_stacked_by_default`, and the #142 panel tests through `_details()`
Today's display is the first section and looks as after #142	tests for behaviour; by eye for the look, headed with `tools/screenshot_inspector.gd`
Adding a section needs no panel code change	tests: the stub is added with `add_child()` alone. Also review: `inspector_panel.gd` names no concrete section
A section that does not apply is hidden	tests: the stub tests
#142 tests pass with changes only for moved things	tests; review: the diff of `test_inspector_panel.gd` is renames only
A test exercises a section that does not apply	tests: the stub tests

2.8 Before posting, check that

Liam could write every signature without opening another file
every signal has an emitter, a listener and a connection site (no new signals)
no builder emits, and no flow relies on a signal's return value
every node the tests reach is in 2.6 with its exact name
every existing method the code leans on is in 2.3
every error path has its return value, and any text a test matches on (none)
every changed signal has its exact emit(...) call written out (none changed)
design constraints from CLAUDE.md that apply are named. None of the gameplay constraints bear on a UI refactor. The one that applies is the issue's own rule: sections change the game only through registry methods.

The spec was checked against a throwaway implementation that followed this outline. The full suite went green (43 scripts, 948 tests), and the implementation was then discarded.

## Spec (drafted with Claude) The tests are on branch `152-inspector-panel-sections`: `test/unit/test_inspector_sections.gd` (new), `test/unit/test_inspector_panel.gd` (renames only), and `test/support/stub_inspector_section.gd` (new). They and this outline are one spec. ### 2.1 Files | File | New / changed | Change | |---|---|---| | `ui/inspector/inspector_section.gd` | new | `@abstract class_name InspectorSection extends VBoxContainer`. The contract: `applies_to()` and `refresh()`. | | `ui/inspector/inspector_context.gd` | new | `class_name InspectorContext extends RefCounted`: `tasks`, `buildings`, `task_for()`. | | `ui/inspector/sections/details_section.gd` | new | `class_name DetailsSection extends InspectorSection`. Today's rows: `pawn_rows`, `building_rows`, `IDLE_TEXT`, `NONE_TEXT`, `_render` all move here unchanged. | | `ui/inspector/sections/details_section.tscn` | new | Root `DetailsSection` plus `%Rows`. See 2.6. | | `ui/inspector/inspector_panel.gd` | changed | Loses `tasks`, `buildings`, the row builders and their constants, `_task_for`, `_render` and `rows_grid`. Gains `context`, `sections_box` and `sections()`. `refresh()` drives the sections. | | `ui/inspector/inspector_panel.tscn` | changed | `%Rows` (GridContainer) is replaced by `%Sections` (VBoxContainer) holding one `%DetailsSection` instance. | | `common/game.gd` | changed | Builds an `InspectorContext` and assigns it to `inspector_panel.context`, replacing the two field assignments. | | `tools/screenshot_inspector.gd` | unchanged | Reused for the headed check. It reaches no node inside the panel. | ### 2.2 API: full signatures ```gdscript # ui/inspector/inspector_section.gd - @abstract class_name InspectorSection extends VBoxContainer @abstract func applies_to(entity: Entity, ctx: InspectorContext) -> bool @abstract func refresh(entity: Entity, ctx: InspectorContext) -> void # ui/inspector/inspector_context.gd - class_name InspectorContext extends RefCounted var tasks: TaskManager = null # null is supported: a pawn's task resolves to null var buildings: Buildings = null # null is supported: a building's task resolves to null func task_for(entity: Entity) -> Task # null: no task, the registry it needs is null, entity is null, or not a Pawn/Building # ui/inspector/sections/details_section.gd - class_name DetailsSection extends InspectorSection const IDLE_TEXT := "idle" # moved from InspectorPanel const NONE_TEXT := "none" # moved from InspectorPanel @onready var rows_grid: GridContainer = %Rows static func pawn_rows(pawn: Pawn, task: Task) -> Dictionary[String, String] # moved verbatim static func building_rows(building: Building, task: Task) -> Dictionary[String, String] # moved verbatim func applies_to(entity: Entity, _ctx: InspectorContext) -> bool func refresh(entity: Entity, ctx: InspectorContext) -> void func _render(rows: Dictionary[String, String]) -> void # moved verbatim # ui/inspector/inspector_panel.gd - class_name InspectorPanel extends PanelContainer var context: InspectorContext = InspectorContext.new() # never null; game.gd replaces it with a filled one @onready var title_lbl: Label = %TitleLbl # unchanged @onready var sections_box: VBoxContainer = %Sections # replaces rows_grid static func title_for(entity: Entity) -> String # unchanged, stays here func sections() -> Array[InspectorSection] func refresh() -> void # same signature, new body func _on_selection_changed(_new: Entity, _previous: Entity) -> void # unchanged func _on_tick(_run_tick: int) -> void # unchanged # removed: tasks, buildings, IDLE_TEXT, NONE_TEXT, pawn_rows, building_rows, rows_grid, _task_for, _render ``` `InspectorSection` extends **VBoxContainer**, not plain `Control`. A plain `Control` takes no minimum size from its children, so a section would collapse to zero height in the panel's VBox. A VBoxContainer stacks whatever the section holds. A section that wants a row of buttons (#136) puts an HBoxContainer inside it. `InspectorSection` is `@abstract`, like `EntityList`, so nothing can instantiate it directly. Concrete sections are `Node`s, so tests `autofree` them or free them with the panel. `InspectorContext` is `RefCounted` and is never freed. **Error paths:** none. There is no `push_error` and no error return anywhere in this change, and no test matches on error text. A null registry and a null entity both resolve to a null task. `panel.context` is never set to null: `game.gd` replaces it, and nothing else assigns it. ### 2.3 Each method's job, in one line New and changed: - `InspectorSection.applies_to(entity, ctx)`: whether this section has anything to show for `entity`. The panel asks again on every refresh, so the answer can change while the same entity stays selected. - `InspectorSection.refresh(entity, ctx)`: brings the section's nodes up to date for `entity`. It is only called while the section applies, and `entity` is never null. A section with controls updates its nodes in place here and does not rebuild them. - `InspectorContext.task_for(entity)`: the body of today's `InspectorPanel._task_for()`, moved. For a `Pawn`: `tasks.get_task(pawn.current_task_id)` when `tasks != null`, else null. For a `Building`: `buildings.task_for(building.id)` when `buildings != null`, else null. Anything else, including null: null. - `DetailsSection.applies_to(entity, _ctx)`: `entity is Pawn or entity is Building`. - `DetailsSection.refresh(entity, ctx)`: for a pawn, `_render(pawn_rows(entity as Pawn, ctx.task_for(entity)))`, and the same with `building_rows` for a building. - `DetailsSection.pawn_rows` / `building_rows` / `_render`: unchanged from #142. `_render` still frees and rebuilds the Labels. That is fine for read-only text, and it only touches this section's own `%Rows`. - `InspectorPanel.sections()`: the children of `sections_box` that are `InspectorSection`, in tree order. Other children, such as a separator, are skipped. It reads the tree on every call and never caches in `_ready()`, so a section added later is picked up. - `InspectorPanel.refresh()`: see 2.5. Existing methods the new code relies on: - `GameState.selected` (setter): ignores a no-op assignment. Otherwise it accepts only `null`, a `Pawn` or a `Building`, and emits `selection_changed(new, previous)` itself. Anything else is a `push_error` and is not stored. So the panel never sees another entity type selected. - `GameState._clear_if_selected(entity)`: connected to `pawn_removed` / `building_removed`, and sets `selected = null`, which emits `selection_changed`. This is how removal hides the panel. The panel has no handler of its own for removal. - `TaskManager.get_task(task_id) -> Task`: returns null for an unknown id. Never errors. - `Buildings.task_for(building_id) -> Task`: returns null when `tasks == null`. Otherwise it scans `tasks.all()` for a `ConstructionTask` or `DeconstructionTask` whose `building_id` matches. Returns null if none matches. ### 2.4 Signals No new signals, and no signal changes. Sections connect to nothing: the panel holds all the timing. | Signal | Declared on | Custom / built-in | Emitted by (method) | Listened to by (method) | Connected where | |---|---|---|---|---|---| | `selection_changed(new: Entity, previous: Entity)` | `GameState` | custom | `GameState.selected` setter | `InspectorPanel._on_selection_changed()` | `InspectorPanel._ready()` (unchanged) | | `tick(run_tick: int)` | `GameClock` | custom | `GameClock` | `InspectorPanel._on_tick()` | `InspectorPanel._ready()` (unchanged) | ### 2.5 Wiring pattern ``` GameState.selection_changed -> InspectorPanel._on_selection_changed(_new, _previous) # connected in InspectorPanel._ready() calls refresh() GameClock.tick -> InspectorPanel._on_tick(_run_tick) # connected in InspectorPanel._ready() if GameState.selected != null: refresh() InspectorPanel.refresh(): 1. entity := GameState.selected 2. if entity == null: visible = false, then return. # conditional; sections are NOT touched or refreshed 3. visible = true # always, from here on 4. title_lbl.text = title_for(entity) # always 5. for section in sections(): # always a. section.visible = section.applies_to(entity, context) # always, for every section b. if section.visible: section.refresh(entity, context) # conditional: only sections that apply ``` Step 2 keeps a section from ever being handed null, so no section has to guard against it. Step 5a runs on every refresh, ticks included. That is how #136's slot control disappears when the site's task ends while the same building stays selected. `_ready()` still calls `refresh()` once, so a panel made over an existing selection starts correct. The sections' own `@onready` vars are set by then, because children are readied before their parent. `game.gd`, replacing lines 72-73 (still inside the `not Engine.is_editor_hint()` guard): ```gdscript var inspector_context := InspectorContext.new() inspector_context.tasks = task_manager inspector_context.buildings = buildings inspector_panel.context = inspector_context ``` GDScript rules this relies on: - **Builders are pure and never emit.** `title_for`, `pawn_rows` and `building_rows` stay static and only return values. `task_for` and `applies_to` read and never change anything. - **The handler is the only thing driving refresh.** Nothing here emits a signal. - **Signals ignore return values.** A section cannot answer the panel through a signal, so `applies_to` is a method call, not a signal. - Sections change the game only through registry methods (for example `TaskManager.set_open_slots()` in #136), never by writing to records directly. Nothing in this ticket writes at all. ### 2.6 Scene structure ``` inspector_panel.tscn InspectorPanel (PanelContainer, inspector_panel.gd) # visible = false and mouse_filter STOP stay as they are └── MarginContainer └── VBoxContainer ├── %TitleLbl (Label) # unchanged └── %Sections (VBoxContainer) # replaces %Rows. Every InspectorSection child is a section └── %DetailsSection (instance of details_section.tscn) # unique_name_in_owner set in the PANEL scene details_section.tscn DetailsSection (VBoxContainer, details_section.gd) └── %Rows (GridContainer) # columns = 2, set in the scene ``` - **Delete `%Rows` from the panel scene and add `%Sections` as a new node**, rather than retyping it, so the editor gives it a fresh `unique_id`. Keep the `uid=` and `unique_id=` values of every other node. - `%DetailsSection` has to be marked unique **on the instance in the panel scene** ("Access as Unique Name" on the instanced node). The tests reach it with `panel.get_node("%DetailsSection")`. `%Rows` is unique inside `details_section.tscn` only, which is what the section's own `@onready` uses. - The details section sits inside the panel's scene, not in `game.tscn`. "Adding a section" therefore means adding an instance under `%Sections` in `inspector_panel.tscn`, which changes the scene but not `inspector_panel.gd`. - The abstract base has no scene. Unlike `EntityList`, nothing inherits a layout: each section scene has its own concrete script on its root. - `ui/inspector/sections/` is a new folder for section scenes, so #136's slot section sits beside this one. Names the tests depend on: `%Sections` (through `panel.sections_box`), `%DetailsSection`, `%TitleLbl` (through `panel.title_lbl`), and `%Rows` (through `DetailsSection.rows_grid`). Renaming any of them breaks the tests. ### 2.7 Tests `test/unit/test_inspector_panel.gd` (#142's tests). What they check is unchanged; only the paths moved: - `InspectorPanel.pawn_rows` / `building_rows` / `IDLE_TEXT` / `NONE_TEXT` become `DetailsSection.…`. - `panel.rows_grid` becomes `_details(panel).rows_grid`, through a new `_details()` helper that returns `%DetailsSection`. - `panel.tasks` / `panel.buildings` become `panel.context.tasks` / `panel.context.buildings`. `test/unit/test_inspector_sections.gd` (new): - **InspectorContext.task_for**: a pawn's task comes from the manager, a building's from the registry, and each is null without its registry. Null has no task. - **Sections are discovered**: `%DetailsSection` is the only section by default; a section added at runtime with `add_child()` is listed after it; a non-section child is neither listed nor hidden. - **Applies / does not apply** (stub section that applies only to buildings): hidden and never refreshed for a pawn; shown and refreshed with the building and with the panel's own `context` (including a replaced one); refreshed again on a tick; hidden on the next tick once it stops applying and shown again when it applies again; hidden when the selection switches to a pawn. - **Deselect**: the panel hides, and no section is refreshed with null, on the selection change or on a later tick. - **DetailsSection**: applies to a pawn and a building, not to another entity, and is visible in the panel for both. `test/support/stub_inspector_section.gd` (new, no `class_name`, preloaded by path like `stub_task.gd`): applies only to buildings while `applies` is true, and records `refresh_calls`, `last_entity` and `last_ctx`. Coverage against the acceptance criteria: | AC | Covered by | |---|---| | Abstract section type; the panel renders through sections | tests: `test_the_details_section_is_stacked_by_default`, and the #142 panel tests through `_details()` | | Today's display is the first section and looks as after #142 | tests for behaviour; **by eye** for the look, headed with `tools/screenshot_inspector.gd` | | Adding a section needs no panel code change | tests: the stub is added with `add_child()` alone. Also review: `inspector_panel.gd` names no concrete section | | A section that does not apply is hidden | tests: the stub tests | | #142 tests pass with changes only for moved things | tests; review: the diff of `test_inspector_panel.gd` is renames only | | A test exercises a section that does not apply | tests: the stub tests | ### 2.8 Before posting, check that - [x] Liam could write every signature without opening another file - [x] every signal has an emitter, a listener and a connection site (no new signals) - [x] no builder emits, and no flow relies on a signal's return value - [x] every node the tests reach is in 2.6 with its exact name - [x] every existing method the code leans on is in 2.3 - [x] every error path has its return value, and any text a test matches on (none) - [x] every changed signal has its exact `emit(...)` call written out (none changed) - [x] design constraints from CLAUDE.md that apply are named. None of the gameplay constraints bear on a UI refactor. The one that applies is the issue's own rule: sections change the game only through registry methods. The spec was checked against a throwaway implementation that followed this outline. The full suite went green (43 scripts, 948 tests), and the implementation was then discarded.

liamjd referenced this issue from a commit

2026-10-11 08:32:22 +01:00

Spec inspector sections ahead of implementation (#152)

liamjd referenced this issue from a commit

2026-10-11 08:32:22 +01:00

Split InspectorPanel into sections (#152)

liamjd referenced this issue from a pull request that will close it,

2026-10-11 08:32:32 +01:00

Split InspectorPanel into sections (#152) #153

liamjd closed this issue

2026-10-11 08:35:33 +01:00

liamjd referenced this issue from a commit