fix: add missing Moon member to ArenaEnvPresetsJson.PresetType

[claude]

Requested by mwfarb · Slack thread

Before: ArenaSceneEnvPresetsDefaults.Presets has a "moon" key, but ArenaEnvPresetsJson.PresetType has no Moon member. An inbound "preset":"moon" therefore fails the StringEnumConverter conversion. It is not silent and it does not lose the look of the scene: the defaults lookup in ApplyEnvironmentPresets keys on the raw JSON string under StringComparer.OrdinalIgnoreCase, so the 25 moon values are populated anyway, and the DTO's [OnError] handler logs a Newtonsoft warning before setting Handled = true. What is left behind is a torn state — the component holds moon values while its enum-typed Preset field still reads Default. Because the preset-change detector compares the incoming string against c.json.Preset.ToString().ToLower(), every later env-presets message sees "moon" != "default", takes the "preset changed" branch, and wipes the accumulated state, discarding any incremental override the scene layered on top. The outbound echo reports preset as default rather than moon.

After: "moon" binds to PresetType.Moon, so the enum field agrees with the values that were loaded. The change detector reads "moon" on the second and every subsequent message, matches the incoming value, and keeps the previous state instead of wiping it. The outbound echo reports the preset the scene actually is.

Adds one enum member, Moon, with [EnumMember(Value = "moon")], in the enum's alphabetical position between Japan and None.

How: the only change is two lines in Runtime/Schemas/Attribute/ArenaEnvPresetsJson.cs (+2 at :371-372), declared exactly like its 18 siblings — attribute line carrying the lowercase wire name, then the PascalCase member. Preset is already annotated [JsonConverter(typeof(StringEnumConverter))], which honours EnumMemberAttribute, so no converter or dictionary change is needed. PresetType.Moon.ToString().ToLower() is "moon", which is what the detector at Runtime/ArenaUnity.cs:749 compares against, so that call site needs no change either. Nothing else in the tree references PresetType (grep -rn "PresetType" --include=*.cs returns only the three lines in this one file).

Refs #226

Refs rather than Closes, because the fix is not durable on its own. A reviewer checked arenaxr/arena-schemas and reports that its schemas/scene-options.json declares preset.enum as exactly the 18 pre-PR values, with no moon — so the next regeneration of this file deletes the member added here and the defect returns. That repo is not reachable from this session, so the enum contents are the reviewer's finding and not something verified here.

The local PresetType.Moon addition is therefore a stopgap. The two durable options are to declare moon in the upstream schema's preset.enum, or to drop the local "moon" key from ArenaSceneEnvPresetsDefaults.Presets so the DTO and the defaults table agree the other way. Note that upstream aframe-environment-component has the same split — it defines a moon preset but omits it from the property's allowed values — so which side is "correct" is a maintainer call, not something either repo's source settles.

⚠️ Generated file — needs mirroring upstream

Runtime/Schemas/Attribute/ArenaEnvPresetsJson.cs carries the header // CAUTION: This file is autogenerated from https://github.com/arenaxr/arena-schemas. Changes made here may be overwritten. The same moon addition has to be made to the arena-schemas source, or this fix is lost on the next regeneration. That repo was not reachable from the session that produced this PR, so nothing was changed there. Worth deciding at the same time whether moon is a genuine aframe-environment-component preset (belongs in the schema) or an addition local to this repo (in which case the dictionary key may be what should change) — that is a maintainer call, not something the source settles.

Relayed second-hand from another session and not verified in this pass: that report states Runtime/Components/ArenaSceneEnvPresetsDefaults.cs is hand-maintained rather than generated — the generator is said to write only Runtime/Schemas/ — so a schema regeneration would delete the PresetType.Moon member this PR adds while leaving the local "moon" defaults entry in place, restoring exactly the torn state described above. That claim was not checked here; treat it as hearsay to confirm, not as a finding of this PR.

What was traced in source vs. inferred

Traced, by reading this tree at a5d27bd:

• Inbound binding. Preset is [JsonConverter(typeof(StringEnumConverter))] with [JsonProperty(PropertyName = "preset")] (ArenaEnvPresetsJson.cs:390-393), and every sibling member uses [EnumMember(Value = "<lowercase>")]. Adding Moon with Value = "moon" is the serialised form the wire already sends, per the dictionary key at ArenaSceneEnvPresetsDefaults.cs:33.
• Change detector. ArenaUnity.cs:748-753 compares (string)incoming["preset"] with c.json.Preset.ToString().ToLower(). With the member present that comparison is "moon" == "moon", so the wipe branch is not taken and baseJson = JObject.FromObject(c.json) keeps state. Stable across repeated messages.
• One correction to the issue. fix: the moon env preset has no PresetType enum member, so an inbound preset: moon cannot round-trip #226 says preset drops out of the outbound echo because ShouldSerializePreset returns Preset != defPreset. That is not what this tree contains — ShouldSerializePreset() at :394-397 is return true; // required in json schema, unconditional. So preset is never omitted; the defect on the outbound side is that it carried the wrong value ("default"), and the InjectNulls / "preset":null concern raised in the issue does not arise. After this change the echo carries "moon".

Inferred, not executed (no Editor available — see below):

• That the warning currently logged is the Newtonsoft conversion message and disappears once the member exists.
• The rendered result — that the moon environment looks the same before and after, since the values were already being applied. This change is about the enum field, not the values.

How to test in the Editor

1. Publish an env-presets scene message with preset: moon. Confirm the scene renders the moon environment and that the Newtonsoft conversion LogWarning no longer appears in the Console.
2. Check the ArenaSceneEnvPresets component in the Inspector: Preset should now read Moon, not Default.
3. Publish the same env-presets message with preset: moon a second time in a row, ideally with an incremental override layered on (e.g. groundColor). Confirm the second message does not wipe scene state — the override survives. Before this change the "preset changed" branch fired on every message.
4. Inspect the outbound message the client publishes and confirm it now carries preset with the value moon (previously default).
5. Sanity-check that no other preset regressed: switch to a neighbouring preset (japan, none) and back.

Untested

This PR is untested. No Unity Editor and no Unity licence were available, so it was never compiled or run, and no CI runs on this PR — the workflow that adds pull_request triggers exists only in the unmerged draft #183. The diff was read adversarially instead; please compile and exercise it in your Editor before this leaves draft.

No tests were added: the Tests~/DotNet/ project only exists on #183's branch. #186 tracks golden-value assertions for these presets and is where a round-trip assertion — every key of ArenaSceneEnvPresetsDefaults.Presets parses to a PresetType member and back — would belong.

Most likely risk in the Editor: the new member shifts the underlying integer values of None and every member after it by one. Any ArenaSceneEnvPresets component already serialized in a scene or prefab asset stores the enum as an int, so a saved None (was 10) would deserialize as Moon (now 10), and so on down the list. Please check any committed scenes/prefabs that pin a preset at or after none alphabetically. Inserting alphabetically matches the file's idiom and how a regeneration from the schema would order it; appending Moon at the end would avoid the renumbering but diverge from that ordering — say the word if you would rather have it appended.

---

Generated by Claude Code