docs: correct per-property conversion-status markers

[claude]

Requested by mwfarb · Slack thread

Before: The // ARENA <name> component unity conversion status: header block at the top of each component is the package's own inventory of which ARENA properties Unity actually implements. It was wrong in both directions. Some properties were marked // DONE: even though their DTO member is never read anywhere in the package, so the component silently ignores them on the wire. Others were marked // TODO: even though the code right below the header implements them — ArenaWireArenauiPrompt claimed nothing was done while ApplyRender() implements six of its seven properties. Because the markers were unreliable in both directions, you could not read them as a to-do list: a DONE did not mean done, and a TODO did not mean missing.

After: Every marker in the files touched here matches what the code does. A TODO now means the property is genuinely unimplemented and a DONE means the DTO member is actually read and applied, so the header blocks can be trusted as the starting point for picking up the next property.

This PR corrects the conversion-status comments only and changes no behaviour whatsoever.

How: For each marker, the whole package (Runtime/ + Editor/, excluding the autogenerated Runtime/Schemas/) was grepped for the corresponding DTO member — json.Dest, json.RetainCache, and so on. Zero reads anywhere means the DONE was false; a read means the TODO was stale. A read from a different file still counts as implemented, so those were not touched. Where a fix would have required editing code, the comment was left alone and is listed under "Needs a behavioural change" below instead. git diff is comment lines only — every added and removed line begins with //.

Refs #227

Refs, not Closes: the third item of #227 — stopping the drift mechanically, with a checker that derives each marker from its DTO's [JsonProperty] names and fails CI when a marker and the code disagree — is not addressed here and still needs a home (the Tests~/DotNet suite on the unmerged #183, or a small Editor/ utility). Without it these blocks will drift again.

Markers changed

15 properties had a wrong marker, plus 1 undocumented property and 1 inaccurate prose comment — 17 line-items across 10 files.

Marked DONE, never read → now TODO (6)

file:line	property	evidence
Runtime/Components/ArenaGotoUrl.cs:17	dest	json.Dest appears nowhere; :39 Application.OpenURL(json.Url) ignores it, so newTab/sameTab/popup all behave identically
Runtime/Components/ArenaGltfModelLod.cs:20	retainCache	json.RetainCache appears nowhere; ApplyRender() reads only json.DetailedUrl
Runtime/Components/ArenaSceneEnvPresets.cs:43	shadowSize	json.ShadowSize appears nowhere
Runtime/Components/ArenaWireArenauiCard.cs:26	font	json.Font appears nowhere; :231-236 assigns TMP_Settings.defaultFontAsset unconditionally
Runtime/Components/ArenaWireArenauiButtonPanel.cs:23	font	json.Font appears nowhere; :129-131 and :261 assign TMP_Settings.defaultFontAsset
Runtime/Components/ArenaMaterialExtras.cs:19	colorWrite	json.ColorWrite appears nowhere; _ColorWriteMask is set at :105-106, but gated on json.TransparentOccluder at :101 — a different property. Added by a later commit on this branch, after the first version of this body

Marked TODO, actually implemented → now DONE (8)

file:line	property	where it is applied
Runtime/Components/ArenaWireArenauiPrompt.cs:19	title	json.Title at :130, :131
Runtime/Components/ArenaWireArenauiPrompt.cs:20	description	json.Description at :133, :134
Runtime/Components/ArenaWireArenauiPrompt.cs:21	buttons	json.Buttons at :152, :175
Runtime/Components/ArenaWireArenauiPrompt.cs:22	width	json.Width at :137
Runtime/Components/ArenaWireArenauiPrompt.cs:24	theme	json.Theme at :119
Runtime/Components/ArenaWireArenauiPrompt.cs:25	materialSides	json.MaterialSides at :254
Runtime/Components/ArenaSceneEnvPresets.cs:29	ground	json.Ground at :260
Runtime/Components/ArenaSceneEnvPresets.cs:40	preset	c.json.Preset at Runtime/ArenaUnity.cs:749, in ApplyEnvironmentPresets, which selects the defaults from ArenaSceneEnvPresetsDefaults.Presets

ArenaWireArenauiPrompt.cs:23 // TODO: font is left as TODO — it is the one entry in that header that was already correct.

Qualified, added, and corrected (3)

file:line	change	why
Runtime/Components/ArenaWireLine.cs:18	// DONE: opacity → // DONE: opacity (only applied when color is also set)	json.Opacity is read, at :45 color.a = json.Opacity;, but only inside the if (json.Color != null) branch at :42. To be precise about what that means: the opacity write sits inside the colour branch and is skipped only when colour is explicitly null — it is not a condition that bites in normal use, because ArenaLineJson.Color defaults to "#74BEC1" (Runtime/Schemas/Object/ArenaLineJson.cs:31-35) and ShouldSerializeColor() returns true unconditionally, so the branch is taken in practice. The marker stays DONE because the property is implemented. The parenthetical as committed reads a shade more alarming than the code warrants and is worth tightening to "skipped only when color is explicitly null" the next time the file is touched; making opacity unconditional would be a behavioural change
Runtime/Components/ArenaWirePcdModel.cs:18	added // TODO: opacity	ArenaPcdModelJson declares four properties (Url, PointSize, PointColor, Opacity) but the status block listed only three. Added in DTO order; TODO is correct since ApplyRender() is empty
Runtime/Components/ArenaMaterial.cs:445-446	reworded	The comment said "attempt direct texture assignment with a warning", which reads as though the assignment always logs. It does not: the assignment at :449-452 is silent, and the warning at :454 fires only on the fallback path where the shader exposes no reflection texture property. Now: "attempt direct texture assignment, warning only if the shader exposes no reflection texture property"

On the count

Earlier scan passes disagreed with each other on how many false-DONE properties exist — one said 15, an enumeration listed 16 line-items over 8 sites, and further passes turned up more. Those numbers are not comparable, because they counted different things and included sites this PR deliberately leaves alone. Recounted from this branch's own diff rather than from any earlier tally (git diff --stat main — 10 files changed, 18 insertions, 16 deletions; the two extra insertions are the added ArenaWirePcdModel line and the two-line rewrap of the ArenaMaterial.cs prose comment), the number corrected here is 15 wrong markers (6 false DONE + 8 stale TODO + 1 unconditional DONE qualified), plus 1 property missing from its status block and 1 prose comment that misdescribed its own code — 17 line-items across 10 files. This is one more than the 16-across-9 figure this body first carried: a later commit on this branch added the ArenaMaterialExtras colorWrite correction, which that figure predates. The larger earlier scan figures are reconciled by the exclusions in the next two sections: the ten ArenaSpeParticles randomize*/rotationAxisSpread entries are real false DONEs but belong to a sibling PR, and three more scan hits were false positives.

False positives deliberately NOT changed

These look like false DONEs to a single-file grep, but the property is read from another file, so the marker is correct as it stands:

file:line	marker	read at
Runtime/Components/ArenaSceneOptions.cs:34	// DONE: physics	ArenaClientScene.Instance.sceneOptions.Physics, ArenaSceneOptions.cs:47 and :51
Runtime/Components/ArenaArmarker.cs:20	// DONE: publish	Runtime/ArenaAprilTag.cs:184, :212
Runtime/Components/ArenaArmarker.cs:22	// DONE: dynamic	Runtime/ArenaAprilTag.cs:190

Also left as-is, because the existing marker is arguably the honest one and changing it would be a judgement call about intent rather than a factual correction:

• Runtime/Components/ArenaWireLight.cs:23 // TODO: envMap — json.EnvMap is read at :114, but only to emit Debug.LogWarning(...) at :118. The property is parsed, not applied, so TODO describes the outcome correctly.
• Runtime/Components/ArenaWireObjModel.cs:22 // TODO: mtl — json.Mtl is read at :37, :39, :42, but only inside an #if UNITY_EDITOR block, so it does nothing in a player build. TODO is defensible; flipping it to DONE would overstate the runtime path.

Both are flagged here for your call rather than decided in this PR.

Left to sibling PRs

These files also carry wrong markers, but sibling PRs already own them and a comment change here would conflict, so they are untouched:

file	why it is left alone
Runtime/Components/ArenaSpeParticles.cs	the ten randomize* / rotationAxisSpread DONE markers become true once that PR implements them, so no comment fix is needed
Runtime/Components/ArenaClickListener.cs	owned by a sibling PR
Runtime/Components/ArenaModelUpdate.cs	owned by a sibling PR
Runtime/Components/ArenaGltfMorph.cs	owned by the base-Update() fix PR

Autogenerated files under Runtime/Schemas/ are untouched.

Needs a behavioural change, not a comment change

No marker in scope needed code to make it true, so nothing was deferred on those grounds. The three items above (ArenaWireLine's conditional opacity, ArenaWireLight's envMap, ArenaWireObjModel's editor-only mtl) are the places where the comment is now accurate but the code is the thing worth revisiting.

Testing

No Editor test is needed. Every changed line is a // comment — git diff -U0 filtered to lines not starting with // is empty — so nothing compiles differently and no runtime path is affected. A review read of the table above against the cited line numbers is all this one wants.

---

Generated by Claude Code