fix: re-enable a renderer when visible: true is set

[claude]

Requested by mwfarb · Slack thread

Before: Hiding an object over the wire was one-way. Publishing visible: false disabled the renderers, but publishing visible: true for the same object afterwards did nothing — the object stayed invisible for the rest of the session. The only way it came back was if the same message also carried a remote-render field, which then happened to re-enable it through a different code path.

After: visible: true re-enables the renderers, so visibility can be toggled back and forth over the wire as many times as you like. visible: false behaves exactly as it does today, and a message that does not mention visible at all is still handed to the remote-render branch untouched.

ApplyVisible (Runtime/ArenaUnity.cs:272-284) was always correct — it assigns renderer.enabled = (bool)data.Visible and handles both values. Only the guard that decides whether to call it was wrong.

How: The guard at Runtime/ArenaClientScene.Messaging.cs:249 read data.Visible != null && !(bool)data.Visible, so ApplyVisible was reached only for a false value. The typed data.Visible cannot answer "did this message set visible?" in the first place: ArenaDataObjectJson.Visible is initialized to true (defVisible, Runtime/Schemas/ArenaDataObjectJson.cs:76,79) and the message is deserialized with NullValueHandling.Ignore (Messaging.cs:116), so the member reads true both for an explicit visible: true and for the majority of messages that omit visible entirely. That is why the old guard leaned on !(bool)data.Visible as a proxy for "the message asked to hide" — and it is why simply dropping the negation would have made the guard true for nearly every object update and turned the remote-render branch into dead code.

So the presence test now goes against the raw jData instead, matching the idiom already used a few lines above for rotation.w (Messaging.cs:238), with ApplyVisible left to apply whichever value arrived. The ordering is unchanged: visible is still checked first and still takes precedence over remote-render, which is only consulted when visible is absent.

Closes #221

Scope — this addresses half of #221. The one-way visible guard is fixed. The cross-message priority relationship between visible and remote-render is deliberately left exactly as it is, and a second, pre-existing path can still undo the visibility this change restores: Runtime/Components/ArenaMaterial.cs:109 sets renderer.enabled = json.Visible from the material component's own visible property, which is a different field from the object-level one this PR fixes. It defaults to true (defVisible, Runtime/Schemas/Attribute/ArenaMaterialJson.cs:619-622), and it is applied from the base ArenaComponent.Update() tick (Runtime/Components/ArenaComponent.cs:25-32) rather than inline with the message, so it lands a frame after the message is handled. An object that also carries a material component can therefore have a visible: false quietly reverted by that path. Untouched here. Because neither of those two is resolved, the maintainer may prefer Refs #221 to Closes #221; it is left as Closes for that call.

How to test in the Editor

• Publish visible: false for an object, confirm its renderer switches off, then publish visible: true for the same object and confirm the renderer comes back. Repeat the toggle a couple of times.
• Confirm the remote-render path still behaves as before: with no visible field on the message, remote-render should still enable/disable the renderer exactly as it does on main.
• Confirm normal object updates that carry no visible field (position/rotation/scale moves) do not disturb visibility — this is the case the guard change is most exposed to.
• Publish visible: true in a message of its own, carrying no other key — not alongside a position update and not alongside remote-render. This is the case that did nothing at all before: the guard now sees visible present in the raw jData, so a bare {"visible": true} update should bring the renderer back on its own.
• Worth a glance: an object whose message carries both visible: true and remote-render. On main the remote-render value wins; with this change visible wins, which is what the existing comment on that line ("visible, if set is highest priority") always claimed should happen.

Untested

This is untested. There is no Unity Editor and no Unity licence in the environment this was written in, and no CI runs on this PR (the workflow that adds pull_request triggers only exists on the unmerged #183). The diff has been read adversarially against the schema defaults and the deserializer settings, but nothing here has been compiled or run — please treat it as a patch to try in your Editor rather than a verified fix.

Note on merge order

Two other open drafts add dispatch arms to this same file, Runtime/ArenaClientScene.Messaging.cs (one for the physx joint attributes, one for video-control). This hunk is deliberately kept to the visibility guard alone and does not touch the attribute switch, so it should not conflict textually with either — but the merge order between the three still matters, and whichever lands last will want a quick re-read of that region.

---

Generated by Claude Code