refactor(world): read-only World.Mobiles / World.Items views (drop scalar count props) #160

Merged

marco merged 1 commit from refactor/world-entity-views into main

2026-07-22 23:01:17 +02:00

marco commented

2026-07-22 22:58:29 +02:00

Owner

Summary

Unify entity access on the World facade behind two read-only query views — World.Mobiles and World.Items — replacing the scattered scalar count props and FindMobile. ModernUO-style (World.Mobiles/World.Items), but read-only: mutation stays on World's own methods. Closes #158.

World.Mobiles (MobileCollection : IReadOnlyCollection<Mobile>) — unifies the player + creature registries: Find(id) (live instance), Count, and enumeration, so world.Mobiles.Count(m => m.IsPlayer) / Find(id) read cleanly. Added Mobile.IsPlayer (virtual; true on PlayerMobile).
World.Items (ItemCollection : IReadOnlyCollection<Item>) — Find(id), Count, enumeration over ItemRegistry.
Removed: World.FindMobile, World.CreatureCount, World.PlacedItemCount, World.ItemCount — migrated every call-site (GM SetCommand/SetSkillCommand, WorldPersister, WorldTick metrics, tests) to the views.
Item + ItemPlace promoted to public, with Item's mutating setters made internal set and its Serialize/Deserialize internal — so an item handed out by Items.Find is a read/inspect handle; placement/ownership still change only through World/ItemRegistry operations. EquipLayer was already public.

Why read-only (not the raw registry)

Exposing the mutating registries would let callers Spawn/Give/reassign around the facade, breaking the thin-facade + single-thread discipline. The views expose only find/count/enumerate; Find(id) returns the live entity for the act-on-instance pattern (e.g. GM SetStat) — identical to the old FindMobile, now unified. Items can't own Mobile-style guarded mutators (an item's validity is contextual — walkable tile, owner, weight budget — which lives in ItemRegistry), so Items.Find is honestly a read/inspect handle while mutation stays on the facade.

Invariants Check (persisted from the critical-design-review)

Scope ✓ delivers the requested view unification; nothing beyond.
Server-authoritative N/A — no client intent/rule; internal API only.
GM authorization ✓ SetCommand/SetSkillCommand still authorize via CanExecute; only the target lookup changed to Mobiles.Find.
Identity model N/A — no account/claim change.
Protocol versioned N/A — no wire/enum/DTO change.
String catalog N/A — no user-facing text.
Single-threaded sim ✓ views are touched only on the sim thread; no lock added; enumeration is lazy over the registries the sim thread already owns.
World.cs HARD GATE ✓ Mobiles/Items are trivial property gets; find/count/enumerate logic lives in the view components. Net logic in World.cs decreased (removed FindMobile/count bodies).
Screen HARD GATE N/A — server-only.
Client engine-independence N/A — no client code.
Gameplay/Networking separation ✓ views are pure gameplay-domain; networking still reaches them only through World.
Act on the instance ✓ Mobiles.Find(id) returns the live Mobile; no side-collection — the views are projections over the authoritative registries.
Extend by type, not switch ✓ IsPlayer is a virtual override (polymorphism), not a type switch.
Server-paced actions N/A.
Persistence (GameServer) ✓ Item.Serialize/Deserialize became internal but format + version int unchanged; no blob change.
Persistence (Auth) N/A.
Process separation N/A — GameServer-internal.
Typed options N/A — no tunable.
Broadcasts/AoI ✓ MobilesWithin/PlayersWithin untouched; the views add a whole-world query surface, not an AoI change.
Multi-platform ✓ pure C#.
Assets required / Asset naming N/A — no art.
ModernUO reference ✓ follows World.Mobiles/World.Items; diverges by staying read-only and keeping per-player blobs (no global linker, #23).
Docs & DoD same change ✓ docs/architecture.md updated; DoD in #158.

How it was tested

New WorldEntityViewsTests (7): Mobiles.Find returns the live instance (mutation via it is observed), unknown id → null, Count + predicate counts split players/creatures, enumeration covers both; Items.Find/Count, unknown id → null, predicate count separates Ground vs Placed.
All migrated call-sites keep their existing tests green.
dotnet test IsoMmo.slnx — all 7 projects green (GameServer 213). dotnet build IsoMmo.slnx clean. dotnet csharpier check . clean.

Checklist

just lint passes (zero warnings)
just test green (whole solution builds)
Multi-platform preserved (pure C#)
Docs updated (docs/architecture.md) + DoD in #158
No self-merge — owner merges after review

## Summary Unify entity access on the `World` facade behind two **read-only query views** — `World.Mobiles` and `World.Items` — replacing the scattered scalar count props and `FindMobile`. ModernUO-style (`World.Mobiles`/`World.Items`), but read-only: mutation stays on `World`'s own methods. Closes #158. - **`World.Mobiles`** (`MobileCollection : IReadOnlyCollection<Mobile>`) — unifies the player + creature registries: `Find(id)` (live instance), `Count`, and enumeration, so `world.Mobiles.Count(m => m.IsPlayer)` / `Find(id)` read cleanly. Added `Mobile.IsPlayer` (virtual; `true` on `PlayerMobile`). - **`World.Items`** (`ItemCollection : IReadOnlyCollection<Item>`) — `Find(id)`, `Count`, enumeration over `ItemRegistry`. - **Removed**: `World.FindMobile`, `World.CreatureCount`, `World.PlacedItemCount`, `World.ItemCount` — migrated every call-site (GM `SetCommand`/`SetSkillCommand`, `WorldPersister`, `WorldTick` metrics, tests) to the views. - **`Item` + `ItemPlace` promoted to `public`**, with `Item`'s mutating setters made `internal set` and its `Serialize`/`Deserialize` `internal` — so an item handed out by `Items.Find` is a read/inspect handle; placement/ownership still change only through `World`/`ItemRegistry` operations. `EquipLayer` was already public. ## Why read-only (not the raw registry) Exposing the mutating registries would let callers `Spawn`/`Give`/reassign around the facade, breaking the thin-facade + single-thread discipline. The views expose only find/count/enumerate; `Find(id)` returns the live entity for the **act-on-instance** pattern (e.g. GM `SetStat`) — identical to the old `FindMobile`, now unified. Items can't own Mobile-style guarded mutators (an item's validity is contextual — walkable tile, owner, weight budget — which lives in `ItemRegistry`), so `Items.Find` is honestly a read/inspect handle while mutation stays on the facade. ## Invariants Check (persisted from the critical-design-review) - **Scope** ✓ delivers the requested view unification; nothing beyond. - **Server-authoritative** N/A — no client intent/rule; internal API only. - **GM authorization** ✓ `SetCommand`/`SetSkillCommand` still authorize via `CanExecute`; only the target lookup changed to `Mobiles.Find`. - **Identity model** N/A — no account/claim change. - **Protocol versioned** N/A — no wire/enum/DTO change. - **String catalog** N/A — no user-facing text. - **Single-threaded sim** ✓ views are touched only on the sim thread; no lock added; enumeration is lazy over the registries the sim thread already owns. - **`World.cs` HARD GATE** ✓ `Mobiles`/`Items` are trivial property gets; find/count/enumerate logic lives in the view components. Net logic in `World.cs` decreased (removed `FindMobile`/count bodies). - **`Screen` HARD GATE** N/A — server-only. - **Client engine-independence** N/A — no client code. - **Gameplay/Networking separation** ✓ views are pure gameplay-domain; networking still reaches them only through `World`. - **Act on the instance** ✓ `Mobiles.Find(id)` returns the live `Mobile`; no side-collection — the views are projections over the authoritative registries. - **Extend by type, not switch** ✓ `IsPlayer` is a virtual override (polymorphism), not a type switch. - **Server-paced actions** N/A. - **Persistence (GameServer)** ✓ `Item.Serialize/Deserialize` became `internal` but format + version int unchanged; no blob change. - **Persistence (Auth)** N/A. - **Process separation** N/A — GameServer-internal. - **Typed options** N/A — no tunable. - **Broadcasts/AoI** ✓ `MobilesWithin`/`PlayersWithin` untouched; the views add a whole-world query surface, not an AoI change. - **Multi-platform** ✓ pure C#. - **Assets required / Asset naming** N/A — no art. - **ModernUO reference** ✓ follows `World.Mobiles`/`World.Items`; diverges by staying read-only and keeping per-player blobs (no global linker, #23). - **Docs & DoD same change** ✓ `docs/architecture.md` updated; DoD in #158. ## How it was tested - New `WorldEntityViewsTests` (7): `Mobiles.Find` returns the live instance (mutation via it is observed), unknown id → null, `Count` + predicate counts split players/creatures, enumeration covers both; `Items.Find`/`Count`, unknown id → null, predicate count separates Ground vs Placed. - All migrated call-sites keep their existing tests green. - `dotnet test IsoMmo.slnx` — all 7 projects green (GameServer 213). `dotnet build IsoMmo.slnx` clean. `dotnet csharpier check .` clean. ## Checklist - [x] `just lint` passes (zero warnings) - [x] `just test` green (whole solution builds) - [x] Multi-platform preserved (pure C#) - [x] Docs updated (`docs/architecture.md`) + DoD in #158 - [x] No self-merge — owner merges after review

marco added 1 commit

2026-07-22 22:58:29 +02:00

refactor(world): read-only World.Mobiles / World.Items views (drop scalar count props)

ci / Lint & Test (pull_request) Successful in 2m15s

Details

555f26100b

Unify entity access behind two read-only query views (Find/Count/enumerate),
ModernUO-style. Mobiles unifies players + creatures; Items wraps ItemRegistry.
Removes FindMobile/CreatureCount/PlacedItemCount/ItemCount. Item/ItemPlace
promoted to public with internal setters so mutation stays behind World.
Closes #158.

marco merged commit 8c5dd9e2b4 into main

2026-07-22 23:01:17 +02:00

marco deleted branch refactor/world-entity-views

2026-07-22 23:01:17 +02:00

marco referenced this pull request from a commit

2026-07-22 23:01:19 +02:00

refactor(world): read-only World.Mobiles / World.Items views (drop scalar count props) (#160)