chore: client/tools standards micro-nits (from code review) #51

New issue

Open

opened 2026-07-19 00:47:05 +02:00 by marco · 1 comment

marco commented

2026-07-19 00:47:05 +02:00

Owner

Low-risk cosmetic cleanups deferred from the standards batch to keep that PR focused (server-side done there). A focused client/tools pass:

One-type-per-file: AssetPack.cs (split public records FrameData/AnimationData), AssetPackLoader.cs (HumanArtSet → own file, and rename to AnimationSet since it is reused for creatures/corpses).
Dedup: the ARGB1555→RGBA ToRgba (identical in AnimMulReader/ArtMulReader/GumpMulReader → one helper); the numerics→XNA Vector2 bridging copied ~5× (Player/Mobile/Corpse/GridRenderer/GameScreen → one ToXnaScreen).
Client config single source: ClientConfig vs the mutable static Session duplicate IsConfigured/URLs — make ClientConfig the single source, hang session fields off ClientContext.
Idioms: remaining plain-ctor→primary-ctor (WebSocketConnection, JwtTokenService, AssetPack); StartsWith without StringComparison (AssetExtractor Program.cs:590); redundant usings in AssetEditor.

Definition of Done

No public multi-type file remains in client/tools beyond the accepted closed-family exception.
ToRgba and the Vector2 bridge each live once.
Client URL/config defaults live in exactly one place.
Whole solution builds green, 0 warnings, csharpier clean.

Progress (2026-07-22)

Re-audited against main:

✅ One-type-per-file: FrameData/AnimationData are now their own files (src/IsoMmo.Assets/), AssetPack.cs holds only AssetPack, and HumanArtSet was renamed and moved to client/IsoMmo.Client/Art/AnimationSet.cs.
✅ ToRgba dedup: now lives once in tools/IsoMmo.AssetExtractor/UoColor.cs; the three *MulReaders all call UoColor.ToRgba.
⬜ Vector2 bridge (ToXnaScreen) — not done (no such helper exists yet).
⬜ Client config single source — not done (IsConfigured still duplicated across ClientConfig and Net/Session).
⬜ Remaining idiom nits — unverified.

So the DoD's multi-type-file and ToRgba-once parts are met; the Vector2-once and config-single-source parts remain.

Low-risk cosmetic cleanups deferred from the standards batch to keep that PR focused (server-side done there). A focused client/tools pass: - **One-type-per-file**: `AssetPack.cs` (split public records `FrameData`/`AnimationData`), `AssetPackLoader.cs` (`HumanArtSet` → own file, and rename to `AnimationSet` since it is reused for creatures/corpses). - **Dedup**: the ARGB1555→RGBA `ToRgba` (identical in AnimMulReader/ArtMulReader/GumpMulReader → one helper); the numerics→XNA `Vector2` bridging copied ~5× (Player/Mobile/Corpse/GridRenderer/GameScreen → one `ToXnaScreen`). - **Client config single source**: `ClientConfig` vs the mutable static `Session` duplicate `IsConfigured`/URLs — make `ClientConfig` the single source, hang session fields off `ClientContext`. - **Idioms**: remaining plain-ctor→primary-ctor (`WebSocketConnection`, `JwtTokenService`, `AssetPack`); `StartsWith` without `StringComparison` (AssetExtractor Program.cs:590); redundant `using`s in AssetEditor. ## Definition of Done - [ ] No public multi-type file remains in client/tools beyond the accepted closed-family exception. - [ ] `ToRgba` and the Vector2 bridge each live once. - [ ] Client URL/config defaults live in exactly one place. - [ ] Whole solution builds green, 0 warnings, csharpier clean. ## Progress (2026-07-22) Re-audited against `main`: - ✅ **One-type-per-file**: `FrameData`/`AnimationData` are now their own files (`src/IsoMmo.Assets/`), `AssetPack.cs` holds only `AssetPack`, and `HumanArtSet` was renamed and moved to `client/IsoMmo.Client/Art/AnimationSet.cs`. - ✅ **`ToRgba` dedup**: now lives once in `tools/IsoMmo.AssetExtractor/UoColor.cs`; the three `*MulReader`s all call `UoColor.ToRgba`. - ⬜ **Vector2 bridge** (`ToXnaScreen`) — not done (no such helper exists yet). - ⬜ **Client config single source** — not done (`IsConfigured` still duplicated across `ClientConfig` and `Net/Session`). - ⬜ Remaining idiom nits — unverified. So the DoD's *multi-type-file* and *`ToRgba`-once* parts are met; the *Vector2-once* and *config-single-source* parts remain.

marco referenced this issue

2026-07-20 18:37:32 +02:00

Alpha — epic: PvP arena for friends #107

marco added the

post-alpha

label

2026-07-20 18:42:07 +02:00

marco referenced this issue from a commit

2026-07-22 06:22:47 +02:00

chore: client/tools standards nits — dedup, splits, idioms (#51)

marco referenced this issue

2026-07-22 06:23:03 +02:00

chore: client/tools standards nits — dedup, splits, idioms (#51) #125

marco commented

2026-07-22 06:23:40 +02:00

Author

Owner

PR #125 covers most of this: ToRgba and the XNA Vector2 bridge each live once; AssetPack records split into own files; HumanArtSet moved to its own file and renamed AnimationSet; primary ctors (AssetPack/WebSocketConnection/JwtTokenService); StartsWith StringComparison. The AssetEditor redundant-usings item was already clean (unused usings fail the build here).

Residual — keep this issue open for it: the ClientConfig/Session single-source (URLs + IsConfigured duplicated; make ClientConfig the source and hang session fields off ClientContext). Deferred from #125 because it touches session wiring across login/game/connection and warrants its own careful pass.

PR #125 covers most of this: `ToRgba` and the XNA `Vector2` bridge each live once; `AssetPack` records split into own files; `HumanArtSet` moved to its own file and renamed `AnimationSet`; primary ctors (`AssetPack`/`WebSocketConnection`/`JwtTokenService`); `StartsWith` StringComparison. The AssetEditor redundant-usings item was already clean (unused usings fail the build here). **Residual — keep this issue open for it:** the `ClientConfig`/`Session` single-source (URLs + `IsConfigured` duplicated; make `ClientConfig` the source and hang session fields off `ClientContext`). Deferred from #125 because it touches session wiring across login/game/connection and warrants its own careful pass.

marco referenced this issue from a commit

2026-07-22 06:25:34 +02:00

chore: client/tools standards nits — dedup, splits, idioms (#51) (#125)