Extract an abstract EntityList base for the HUD lists #146

New issue

Closed

opened 2026-10-10 15:00:11 +01:00 by liamjd · 0 comments

liamjd commented

2026-10-10 15:00:11 +01:00

Owner

Part of milestone #6 (Selection and inspection). Depends on #140 and #141 being merged.

PawnList (#140) and BuildingList (#141) are near-copies, in both the scripts and the scenes. The only real differences are which GameState signals add and remove rows, and which card scene a row is built from. Pull the shared behaviour into an abstract base class and a shared base scene, so the next list (or a fix to the expand/collapse animation) is made once.

This is a refactor. Neither list should behave any differently afterwards: all the rules in #140 and #141 (select on click, NONE mode only, dimmed outside NONE, catches the mouse, follows the selection back) still apply.

Design

Script: ui/lists/entity_list.gd, @abstract class_name EntityList extends MarginContainer. It uses the same @abstract annotation as Entity.

Key _cards by the entity, not its id: Dictionary[Entity, Button]. The base class then never needs an id.
_on_selection_changed checks _cards.has(new) in place of new is Pawn / new is Building, so the type check goes away.
One generic _entity_selected(toggled: bool, entity: Entity) replaces _pawn_selected and _building_selected.
Expand/collapse (_toggle_expanded, _resize_list_container, the tween), interaction-mode handling, and add/remove all move into the base class.
Two abstract hooks are left for the subclasses:
- @abstract func _connect_entity_signals() -> void: connect the spawn/remove signals to _add_entity / _remove_entity. Called from the base _ready.
- @abstract func _make_card(entity: Entity) -> Button: instantiate and populate the card. It isn't in the tree yet, so its @onready vars are unset.
PawnList and BuildingList become extends EntityList, holding only CARD_SCENE and the two hooks.

_remove_entity must remove_child the card before queue_free(), then resize. queue_free is deferred, so a card that is only queued still counts towards get_combined_minimum_size(), and the list ends up one row too tall (found during #141).

Scenes: script inheritance doesn't share the .tscn layout, so use scene inheritance.

ui/lists/entity_list.tscn holds the full layout (header button, arrow label, title label, clipper, row container) with neutral unique names, e.g. %ListContainer.
Its root has no script. An abstract script on a scene root fails to instantiate when that scene is opened or run on its own.
pawn_list.tscn and building_list.tscn are recreated as inherited scenes (Scene > New Inherited Scene). Each attaches its concrete script to the root and sets the title text.
Keep the existing uid= values for pawn_list.tscn / building_list.tscn if possible. game.tscn references them.

Out of scope: a shared EntityCard base for PawnCard / BuildingCard. They share only the toggle-button shape, and each _process reads different fields. Revisit if a third list appears.

Acceptance criteria

EntityList exists, is @abstract, and holds all shared list behaviour.
PawnList and BuildingList extend it and implement only _connect_entity_signals and _make_card (plus CARD_SCENE).
entity_list.tscn is the only place the list layout is defined. Both list scenes inherit it.
Both lists still meet every acceptance criterion of #140 and #141, checked headed in the running game.
Removing a pawn or building shrinks its list by exactly one row, with the animation.
Adding an entity while a list is collapsed leaves it collapsed.
The editor scan has been run after adding class_name EntityList, and the GUT suite is green.

Part of milestone #6 (Selection and inspection). Depends on #140 and #141 being merged. `PawnList` (#140) and `BuildingList` (#141) are near-copies, in both the scripts and the scenes. The only real differences are which `GameState` signals add and remove rows, and which card scene a row is built from. Pull the shared behaviour into an abstract base class and a shared base scene, so the next list (or a fix to the expand/collapse animation) is made once. This is a refactor. Neither list should behave any differently afterwards: all the rules in #140 and #141 (select on click, NONE mode only, dimmed outside NONE, catches the mouse, follows the selection back) still apply. ## Design **Script:** `ui/lists/entity_list.gd`, `@abstract class_name EntityList extends MarginContainer`. It uses the same `@abstract` annotation as `Entity`. - Key `_cards` by the entity, not its id: `Dictionary[Entity, Button]`. The base class then never needs an id. - `_on_selection_changed` checks `_cards.has(new)` in place of `new is Pawn` / `new is Building`, so the type check goes away. - One generic `_entity_selected(toggled: bool, entity: Entity)` replaces `_pawn_selected` and `_building_selected`. - Expand/collapse (`_toggle_expanded`, `_resize_list_container`, the tween), interaction-mode handling, and add/remove all move into the base class. - Two abstract hooks are left for the subclasses: - `@abstract func _connect_entity_signals() -> void`: connect the spawn/remove signals to `_add_entity` / `_remove_entity`. Called from the base `_ready`. - `@abstract func _make_card(entity: Entity) -> Button`: instantiate and populate the card. It isn't in the tree yet, so its `@onready` vars are unset. - `PawnList` and `BuildingList` become `extends EntityList`, holding only `CARD_SCENE` and the two hooks. `_remove_entity` must `remove_child` the card **before** `queue_free()`, then resize. `queue_free` is deferred, so a card that is only queued still counts towards `get_combined_minimum_size()`, and the list ends up one row too tall (found during #141). **Scenes:** script inheritance doesn't share the `.tscn` layout, so use scene inheritance. - `ui/lists/entity_list.tscn` holds the full layout (header button, arrow label, title label, clipper, row container) with neutral unique names, e.g. `%ListContainer`. - **Its root has no script.** An abstract script on a scene root fails to instantiate when that scene is opened or run on its own. - `pawn_list.tscn` and `building_list.tscn` are recreated as inherited scenes (Scene > New Inherited Scene). Each attaches its concrete script to the root and sets the title text. - Keep the existing `uid=` values for `pawn_list.tscn` / `building_list.tscn` if possible. `game.tscn` references them. **Out of scope:** a shared `EntityCard` base for `PawnCard` / `BuildingCard`. They share only the toggle-button shape, and each `_process` reads different fields. Revisit if a third list appears. ## Acceptance criteria - [ ] `EntityList` exists, is `@abstract`, and holds all shared list behaviour. - [ ] `PawnList` and `BuildingList` extend it and implement only `_connect_entity_signals` and `_make_card` (plus `CARD_SCENE`). - [ ] `entity_list.tscn` is the only place the list layout is defined. Both list scenes inherit it. - [ ] Both lists still meet every acceptance criterion of #140 and #141, checked headed in the running game. - [ ] Removing a pawn or building shrinks its list by exactly one row, with the animation. - [ ] Adding an entity while a list is collapsed leaves it collapsed. - [ ] The editor scan has been run after adding `class_name EntityList`, and the GUT suite is green.

liamjd added this to the Selection and inspection milestone

2026-10-10 15:00:11 +01:00

liamjd added the

label

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

liamjd referenced this issue

2026-10-10 15:04:23 +01:00

140-pawn-list-on-hud #147

liamjd referenced this issue from a commit

2026-10-10 15:22:13 +01:00

Extract an abstract EntityList base for the HUD lists (#146)

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