Port object commands to native

Author: gajop

.luacheckrc

@@ -21,6 +21,30 @@ std=lua51
 
 files["libs_sb/utils/luaunit.lua"] = { ignore = {"581"} }
 
+-- Third-party code bundled into the tree (git submodules, vendored libraries,
+-- Chili UI skins, engine widgets/gadgets). Not SBC editor source, so not linted.
+exclude_files = {
+    "libs_sb/chiliui/**",
+    "libs_sb/chonsole/**",
+    "libs_sb/s11n/**",
+    "libs_sb/spring-launcher/**",
+    "libs_sb/i18n/**",
+    "libs_sb/kernel/**",
+    "libs_sb/lcs/**",
+    "libs_sb/springmon/**",
+    "libs_sb/chilifx/**",
+    "libs_sb/chotify/**",
+    "libs_sb/json.lua",
+    "libs_sb/MessagePack.lua",
+    "libs_sb/savetable.lua",
+    "LuaUI/Configs/chili/**",
+    "LuaUI/Configs/chilitip_conf.lua",
+    "LuaUI/widgets/gui_chili_selections_and_cursortip.lua",
+    "LuaUI/widgets/hide_default_layout.lua",
+    "LuaRules/Configs/icon_generator.lua",
+    "LuaRules/Gadgets/unit_icongenerator.lua",
+}
+
 globals = {
     -- std extensions
     "math.round", "math.bit_or",

LuaUI/widgets/api_sb_chili.lua

@@ -226,12 +226,12 @@ end
 
 
 local keyPressed = true
-function widget:KeyPress(key, mods, isRepeat, label, unicode)
+function widget:KeyPress(key, keyMods, isRepeat, label, unicode)
 	if Spring.IsGUIHidden() or totalHideInterface then
 		return false
 	end
 
-	keyPressed = screen0:KeyPress(key, mods, isRepeat, label, unicode)
+	keyPressed = screen0:KeyPress(key, keyMods, isRepeat, label, unicode)
 	return keyPressed
 end
 

docs/design/objects.md

@@ -0,0 +1,55 @@
+---
+name: Object Commands
+description: Add/remove/set flow for units, features, and areas in native SpringBoard
+---
+
+# Object Commands
+
+Objects are addressed by **model id**, not Spring id. The model id is stable
+across undo/redo and save/load. Units/features also have a Spring id; areas do
+not, so their model id is the object id. `ObjectManager` owns the per-kind
+handlers and asks each handler to translate model id to Spring id when needed.
+
+```mermaid
+flowchart LR
+    Lua["Lua command JSON"] --> Cmd["Rust command"]
+    Cmd --> Mgr["ObjectManager"]
+    Mgr --> Kind["ObjectHandler: area/unit/feature"]
+    Kind --> Fields["typed field registry"]
+    Fields --> State["engine or editor state"]
+    Kind --> Events["ObjectEvent queue"]
+    Mgr --> Bridge["event_bridge"]
+    Bridge --> LuaUI["LuaUI object mirror"]
+```
+
+`AddObjectCommand` parses wire params through `objects::codec` into typed
+`ObjectData`, creates the object, applies its fields, and records an added
+event. `RemoveObjectCommand` reads the object before destruction so undo can
+re-add the same model id. `SetObjectParamCommand` captures old typed field
+values on first execute, parses new typed values, then applies one field or a
+field set.
+
+Fields are registered next to each kind's s11n with `inventory::submit!`.
+Each field implements `TypedField<Model>` and declares its Rust `Value`,
+descriptor, getter, and setter together. Runtime dispatch erases values only to
+share one registry; field implementations themselves get typed values, not JSON.
+
+The current wire format is still JSON. `codec.rs` is the boundary that converts
+JSON to typed `FieldValue` and back for command input, save-shaped reads, tests,
+and LuaUI notifications. Model code should not serialize just to apply fields.
+
+Set-many intentionally iterates over the supplied typed fields. Unit/feature
+handlers also preserve the old Lua ordering quirks around rotation and movement,
+because engine position updates can disturb facing. That behavior belongs in the
+kind handler, not the UI.
+
+Models emit `ObjectEvent::{Added,Removed,Updated}` only. They do not know about
+Lua. `ObjectManager` drains events after add/remove/set; `event_bridge` turns
+those events into widget commands so LuaUI's object mirror stays current. Later
+Rust UI/state consumers can subscribe at the same event boundary without changing
+the models.
+
+Known pressure points: generic object commands still carry JSON at the command
+boundary; descriptors are collected into `Vec`s from inventory on lookup; field
+application is name-based after parsing; notifications currently still bridge to
+LuaUI because the frontend mirror is Lua-owned during the port.

docs/porting/01-slices.md

@@ -34,7 +34,7 @@ spread across the slices they belong to, not deferred as a catch-all.
 | 2 | [Heightmap](#2-heightmap) — load + save + import / export (async IO) | done (stable; native IO seam, 16-bit PNG, raw-f32 save/load by path, import undoable) |
 | 3 | [Map settings](#3-map-settings) — sun / atmosphere / water / map-rendering | review (in stable; all setters Rust-only with `Gfx`-snapshot undo) |
 | 4 | [Textures](#4-textures) — diffuse / shading / terrain texture / cache + grass + DNTS | review (in stable; Rust owns paint + cache + stroke close + undo/redo) |
-| 5 | [Objects](#5-objects) — units & features add / remove / set / move (needs s11n) | wip (not in stable) |
+| 5 | [Objects](#5-objects) — units & features add / remove / set / move (needs s11n) | review (in stable) |
 | 6 | [Areas](#6-areas) | wip (not in stable) |
 | 7 | [Teams & diplomacy](#7-teams--diplomacy) | wip (not in stable) |
 | 8 | [Project lifecycle](#8-project-lifecycle) — save / load / export / sync / start / stop + scenario-info | wip — core (not in stable) |

docs/porting/review-queue.md

@@ -7,33 +7,39 @@ description: Async review pipeline — items Claude has implemented that are wai
 
 Items Claude has finished implementing, awaiting human gates. Top of the list = oldest pending.
 
-Each row has: link to the Rust file, the Lua file it replaces, a one-line description, and what to verify when testing. Claude appends; user moves items off as they're reviewed → tested → committed.
+Each item is a one-line description and the steps to verify it in-game (read the diff for the code). Claude appends; user removes items as they're reviewed → tested → committed. Oldest first.
 
-| # | Item | Files | State | What to test |
-|--:|------|-------|-------|--------------|
-| 3 | **Map settings** (slice 3). Sun direction + sun lighting, atmosphere (sky/fog), water params, map-rendering (splat scales/mults, void water/ground), and global-LOS — all Rust-only (`nativeCommandsOnly`). Each setter applies via `UnsyncedCtrl`/`SyncedCtrl` and snapshots the keys it touches via the matching `Gfx::Get*` for undo (map-rendering and global-LOS have no undo, matching their Lua). Water re-selects the current water mode after applying (`Display::GetWaterMode` + `Messages::SendCommands("water", N)`) so the renderer picks up new params — the native equivalent of Lua's `SendCommands('water '..GetWaterMode())`. Partial opts (editors send one key at a time). | New: [`map_settings/`](../../native/src/sbc/map_settings/)`commands/{set_sun_parameters,set_sun_lighting,set_atmosphere,set_water_params,set_map_rendering_params,set_global_los}_command.rs`, [`map_settings/tests/test_map_settings.rs`](../../native/src/sbc/map_settings/tests/test_map_settings.rs). Wiring: `mod map_settings;` in [sbc/mod.rs](../../native/src/sbc/mod.rs), flips in [command_manager.lua](../../scen_edit/command/command_manager.lua). **Lua command files unchanged** — only the dispatch flip. | review | `just test-integration map_settings` → 6 tests pass (each sets a value, reads it back via `Gfx::Get*`, undoes where applicable). In-editor: Lighting / Sky / Water / Terrain-settings editors → change sun direction, a ground color, fog color, a water param, splat scales → visible change; **Ctrl+Z** restores sun / lighting / atmosphere / water (map-rendering & global-LOS intentionally don't undo). |
+## Objects (slice 5) — units, features & areas add / remove / move / set-param — *review*
+
+`just test-integration objects` → 4 tests pass. In-editor:
+
+1. Place a **unit** and a **feature**; drag to move, rotate, edit a property (health/mass) in the property window — each change shows in the engine.
+2. Add an **area** (rect); drag to move and resize it.
+3. **Ctrl+Z / Ctrl+Y** each of the above — undo restores, redo re-applies.
+4. Place an **aircraft** unit and confirm its `crashing` state round-trips (only field not covered by automated tests).
+5. **Identity probe:** add a unit and an area → undo → redo → **save + reload the project**; the objects and any trigger referencing them must survive.
 
 ## States
 
 - **review** — Claude says implemented, lint + tests green; user hasn't read the code yet
 - **tested** — user read it and ran it in-game; ready to commit
-- **(removed from this file)** — committed; row deleted, phase doc updated to **done**
+- **(removed from this file)** — committed; item deleted, phase doc updated to **done**
 
 ## Claude's flow
 
 When finishing a port:
 
 1. Update the matching phase doc — set status **review**.
-2. Append a row here with a short test recipe.
+2. Append an item here (a `##` section) with a short test recipe.
 3. Move on to the next item (do not wait).
 
 ## User's flow
 
-1. **Review**: read the linked Rust file. If happy, mark row **tested** after testing it next.
-2. **Test**: launch SBC, do the steps in "What to test". If happy, tell Claude to commit.
-3. **Commit**: Claude stages the right files, runs `git commit -m`, deletes the row, updates the phase doc to **done**.
+1. **Review**: read the diff. If happy, mark the item **tested** after testing it next.
+2. **Test**: launch SBC, do the listed steps. If happy, tell Claude to commit.
+3. **Commit**: Claude stages the right files, runs `git commit -m`, deletes the item, updates the phase doc to **done**.
 
-If something fails at any gate: leave a note in the row, kick back to Claude (re-mark the phase-doc entry as **in progress**).
+If something fails at any gate: leave a note on the item, kick back to Claude (re-mark the phase-doc entry as **in progress**).
 
 ## Dependency rule
 

docs/porting/todo.md

@@ -12,7 +12,39 @@ stable mid-review.
 
 ---
 
-## 1. Unit-struct commands are a registration gotcha
+## 1. Objects commands: make them concrete and fully typed
+
+**What.** The three object commands are generic over `objType` and still carry
+`serde_json::Value` (`AddObjectCommand.params`, `SetObjectParamCommand.key`/`value`).
+Split them into concrete per-kind commands whose struct fields *are* the object's
+fields — typed, deserialized straight from the wire, no `serde_json::Value`:
+
+- Add:    `AddAreaCommand`, `AddFeatureCommand`, `AddUnitCommand`
+- Remove: `RemoveAreaCommand`, `RemoveFeatureCommand`, `RemoveUnitCommand`
+- Set:    `SetAreaParamCommand`, `SetFeatureParamCommand`, `SetUnitParamCommand`
+
+Each `Add*`/`Set*` lists every field of its kind explicitly (`Set*` all
+`Option<_>`; `Add*` requires the create params). Field names/types/ranges live in
+`native/src/sbc/objects/model/<kind>_s11n/fields.rs` — keep in sync.
+
+- **Area:** pos, size
+- **Feature:** defName, pos, rot, vel, dir, mass, midAimPos, blocking,
+  radiusHeight, collision, team, health, resources, rules
+- **Unit:** defName, pos, rot, vel, dir, mass, midAimPos, maxRange, blocking,
+  radiusHeight, collision, team, health, maxHealth, paralyze, capture, build,
+  tooltip, stockpile, experience, neutral, fuel, movectrl, gravity,
+  harvestStorage, resources, armored, crashing, rules, states, commands
+
+**Why.** Compile-time field validation, no per-command JSON parse, undo state
+already typed. Removes the last JSON from the objects command path, so the codec's
+`json_to_object`/`parse_named_field` fall away.
+
+**How.** One concrete command per kind via `register_command!`; serde derives do
+the parsing. `ObjectManager`'s `ObjectKind` dispatch stays as-is.
+
+---
+
+## 2. Unit-struct commands are a registration gotcha
 
 **What.** A control command with no payload is written as a braced struct with an
 empty body specifically so it deserializes:
@@ -79,7 +111,7 @@ in place and commented at each site
 
 ---
 
-## 2. In-engine tests bypass the Lua command_manager (dispatch bugs invisible to CI)
+## 3. In-engine tests bypass the Lua command_manager (dispatch bugs invisible to CI)
 
 **What.** The in-engine integration tests
 ([native/src/sbc/tests/](../../native/src/sbc/tests/)) drive Rust **directly**
@@ -110,7 +142,7 @@ and only caught by hand.
 
 ---
 
-## 3. Brush data structures: ditch nested HashMaps for vectors
+## 4. Brush data structures: ditch nested HashMaps for vectors
 
 [brush_filter_generator.rs](../../native/src/sbc/commands/heightmap/brush_filter_generator.rs)
 ports SpringBoard's brush-filter cache verbatim as
@@ -120,9 +152,17 @@ around) is a mechanical port of the Lua; vectors / a flat keyed struct would be
 far more efficient and readable. Refactor the whole brush system off nested
 HashMaps once the behavior is locked in.
 
-## 4. Load the brush greyscale directly in Rust
+## 5. Load the brush greyscale directly in Rust
 
 [set_heightmap_brush_command.rs](../../native/src/sbc/commands/set_heightmap_brush_command.rs)
 receives the greyscale shape as a large array marshalled from Lua on every
 `SetHeightmapBrushCommand`. Load the brush image directly in Rust so the big
 array doesn't cross the bridge (ties into the async-IO image work in slice 2).
+
+## 6. Object field descriptors allocate on every lookup
+
+`ObjectHandler::descriptors()` currently returns `Vec<ObjectFieldDescriptor>` by
+collecting each kind's `inventory` entries. This keeps field registration simple,
+but `ObjectManager::descriptor()` and command parsing can allocate repeatedly for
+what is effectively static metadata. Cache per-kind descriptor slices/maps once,
+or generate static descriptor arrays alongside the field registry.