diff --git a/CLAUDE.md b/CLAUDE.md index a71145295..ea2481f14 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -148,7 +148,7 @@ Cross-cutting patterns: - the ECS `AllComponents` type list (from every `struct *Component` *definition*, minus a small runtime-only exclusion set) → `OloEngine/src/OloEngine/Scene/Generated/AllComponents.Generated.inl`, `#include`d by `Scene/Components.h`. This collapses one of the formerly-hand-maintained ECS component touch-points into codegen (issue #380, first slice). - the SaveGame `SAVE_COMPONENT(...)` / `TRY_LOAD_COMPONENT(...)` enumeration lists (same `struct *Component` scan, minus a save-game-specific exclusion set `kComponentsNotInSaveGame` — components with no save serializer) → `OloEngine/src/OloEngine/SaveGame/Generated/SaveGameComponent{Capture,Restore}.Generated.inl`, `#include`d by `SaveGame/SaveGameSerializer.cpp`. This collapses the two most dangerous unguarded ECS touch-points — a component missing from either list was *silently* dropped from every save-game (issue #380, second slice). - the Scene `OnComponentAdded` / `OnComponentRemoved` no-op specialization lists (same `struct *Component` scan, minus two *distinct* custom-handler exclusion sets `kComponentsCustomOnAdd` / `kComponentsCustomOnRemove` — the components whose add/remove callback is hand-written because it does real init/teardown) → `OloEngine/src/OloEngine/Scene/Generated/OnComponent{Added,Removed}.Generated.inl` (same `Scene/Generated` dir as the tuple, no extra CLI arg), `#include`d by `Scene/Scene.cpp` inside the `OLO_ON_COMPONENT_{ADDED,REMOVED}_NOOP` macros. This collapses the `OnComponentAdded`/`OnComponentRemoved` touch-point (issue #380, third slice); the declaration-only primary templates mean a mis-excluded handler is a loud build error (duplicate-definition or unresolved-symbol), never silent. -- the Scene serializer per-component serialize/deserialize blocks → `OloEngine/src/OloEngine/Scene/Generated/Scene{Serialize,Deserialize}Components.Generated.inl` (same `Scene/Generated` dir, no extra CLI arg), `#include`d by `Scene/SceneSerializer.cpp`. Driven by a **separate full data-member scan** (`CollectComponentFields`), **not** the `OLO_PROPERTY` scan — the serializer persists *every* field, not just script-exposed ones (e.g. `DirectionalLightComponent` serializes 9 fields but annotates only 3). A component is emitted iff **every** data member is a primitive / small-int (`u8`/`u16`/`i8`/`i16`) / `glm::vec*`/`ivec*`/`quat`/`mat3`/`mat4` / `std::string` / `AssetHandle` / `UUID` / `enum`, a `std::vector` of one of those, a `std::unordered_set` of a sortable trivial scalar, or a `std::string`-keyed `std::unordered_map` (`SceneSerType` plus a `CollectEnumTypes` leaf-name check, the `std::vector` element-type parse, and the `std::unordered_set`/`std::unordered_map` parse added by issue #451's unordered_map/set slice — see the cross-binding section above for the exact eligibility rules and the sort-before-emit determinism reasoning; `AssetHandle`/`UUID` `u64`-wrapper types added by issue #451's first slice — `static_cast` on write, `.as` + implicit `UUID(u64)` on read; `enum`s added by #451's enum slice — `static_cast` on write, `.as` on read, cast back via `static_cast`; glm-math / small-int / `std::vector` added by #451's glm/small-int/vector slice — glm via the `Core/YAMLConverters.h` Encode/Decode helpers, small ints widened to `u32`/`i32` on emit to dodge yaml-cpp's raw-char encode then read via `.as`, vectors as YAML sequences; **an all-trivial nested `struct` and a `std::vector` added by #451's nested-struct slice** — a leaf-name→body registry of every struct (`CollectStructBodies`) fed to a recursive `ClassifyStruct`/`ParseComponentFields` pair with a `visited` cycle-guard, emitted as a nested sub-map / sequence-of-sub-maps by the recursive `EmitSerializeFields`/`EmitDeserializeFields`; that slice also hardened the statement-splitter to break at the brace-depth-0 `}` so an inline method body no longer swallows a following member or a `private:` label — the fix that makes `ColliderMaterial`'s private members correctly reject the six 3D colliders) **and** it is not in `kComponentsCustomSerialize` (the fourth, distinct exclusion set — trivial components kept hand-written for range-clamp/`Sanitize`/runtime-field/non-`m_`/entity-identity reasons; e.g. `IDComponent` is excluded because the test suite can't catch its mis-generation — see the cross-binding section). Anything with a still-unhandled non-trivial field (`Ref` of a non-Asset-derived type, `std::array`, a non-`std::string`-keyed `std::unordered_map`, a `std::unordered_set` of a non-scalar/non-sortable element (`float`, a glm vector/matrix/quat, a struct), a `std::vector` of a *non-trivial* struct, **or any non-public member** — the parser tracks `private:`/`protected:` and fails such a component safe to non-trivial) is classified non-trivial and skipped automatically (no exclusion entry needed). **The enum slice flipped several components that needed exclusion** — `Rigidbody3DComponent` (its enum is keyed `BodyType` not the `m_`-stripped `Type`, plus a runtime `m_RuntimeBodyToken` + many fields the hand-written serializer omits), `StreamingVolumeComponent` (runtime `IsLoaded` + radius clamps), `FogVolumeComponent` (Shape/Priority clamps), `UIButtonComponent` (runtime `m_State`), `UISliderComponent` (runtime `m_IsDragging`); **the glm/vector slice flipped 3 more** — `LightProbeVolumeComponent` (runtime `m_Dirty`/`m_ShowDebugProbes` + clamps), `NavAgentComponent` (runtime path/target fields), `PhysicsJoint3DComponent` (runtime `m_RuntimeConstraintToken` + dozens of clamps); **the nested-struct slice flipped 5 more, all needing exclusion** — `LODGroupComponent` (hand-written flattens the `LODGroup` sub-struct to top-level keys + omits runtime `m_GeneratedLODHandles`), `NavMeshBoundsComponent` (`SanitizeVec3` + Min≤Max ordering + drops non-finite links), and the runtime `DialogueStateComponent`/`NoiseAnimationStateComponent`/`SpringBoneStateComponent` (per-tick state that must not be serialized — the `IDComponent` no-hand-written-block blind spot) — a textbook reminder to diff the regenerated `.inl` and exclude every newly-flipped component that omits a field, clamps on load, or is runtime-only. This collapses the last big *unguarded* ECS touch-point (issue #380, fourth slice) — a forgotten field was **silent scene-data loss**, which the old name-only coverage test couldn't catch. Guarded by `ComponentSerializerCoverageTest`: existence (every component handled by `SceneSerializer.cpp` ∪ the generated `.inl`) **and** disjointness (`NoComponentIsBothHandWrittenAndGenerated` — no double-emit). **Per-field `OLO_SERIALIZE(Clamp, Min=…, Max=…)` (issue #451 Clamp slice)** later un-flipped 4 of the above: `FogVolumeComponent`, `SnowDeformerComponent`, `SpringBoneComponent`, and `NavAgentComponent` (whose remaining runtime fields now carry `OLO_SERIALIZE(Skip)`) all migrated fully generated once their clamp-only fields were annotated — see the field-level detail in the cross-binding section above. `LightProbeVolumeComponent`/`NavMeshBoundsComponent`/`StreamingVolumeComponent`/`PhysicsJoint3DComponent` stay excluded — each needs a cross-field invariant or a scale of complexity beyond a per-field range clamp. **The unordered_map/set slice migrated `PrefabComponent`** (three `std::unordered_set` override-tracking fields) fully off hand-written code — it needed no `kComponentsCustomSerialize` entry, before or after, since it was hand-written solely because `std::unordered_set` wasn't a recognised type yet. `MorphTargetComponent`'s `std::unordered_map Weights` field is also now individually trivial but the component stays hand-written for unrelated reasons (a `Ref` field where `MorphTargetSet` isn't `Asset`-derived, plus a per-value `[0,1]` clamp on load) — see the cross-binding section above for the full detail. +- the Scene serializer per-component serialize/deserialize blocks → `OloEngine/src/OloEngine/Scene/Generated/Scene{Serialize,Deserialize}Components.Generated.inl` (same `Scene/Generated` dir, no extra CLI arg), `#include`d by `Scene/SceneSerializer.cpp`. Driven by a **separate full data-member scan** (`CollectComponentFields`), **not** the `OLO_PROPERTY` scan — the serializer persists *every* field, not just script-exposed ones (e.g. `DirectionalLightComponent` serializes 9 fields but annotates only 3). A component is emitted iff **every** data member is a primitive / small-int (`u8`/`u16`/`i8`/`i16`) / `glm::vec*`/`ivec*`/`quat`/`mat3`/`mat4` / `std::string` / `AssetHandle` / `UUID` / `enum`, a `std::vector` of one of those, a `std::unordered_set` of a sortable trivial scalar, or a `std::string`-keyed `std::unordered_map` (`SceneSerType` plus a `CollectEnumTypes` leaf-name check, the `std::vector` element-type parse, and the `std::unordered_set`/`std::unordered_map` parse added by issue #451's unordered_map/set slice — see the cross-binding section above for the exact eligibility rules and the sort-before-emit determinism reasoning; `AssetHandle`/`UUID` `u64`-wrapper types added by issue #451's first slice — `static_cast` on write, `.as` + implicit `UUID(u64)` on read; `enum`s added by #451's enum slice — `static_cast` on write, `.as` on read, cast back via `static_cast`; glm-math / small-int / `std::vector` added by #451's glm/small-int/vector slice — glm via the `Core/YAMLConverters.h` Encode/Decode helpers, small ints widened to `u32`/`i32` on emit to dodge yaml-cpp's raw-char encode then read via `.as`, vectors as YAML sequences; **an all-trivial nested `struct` and a `std::vector` added by #451's nested-struct slice** — a leaf-name→body registry of every struct (`CollectStructBodies`) fed to a recursive `ClassifyStruct`/`ParseComponentFields` pair with a `visited` cycle-guard, emitted as a nested sub-map / sequence-of-sub-maps by the recursive `EmitSerializeFields`/`EmitDeserializeFields`; that slice also hardened the statement-splitter to break at the brace-depth-0 `}` so an inline method body no longer swallows a following member or a `private:` label — the fix that makes `ColliderMaterial`'s private members correctly reject the six 3D colliders) **and** it is not in `kComponentsCustomSerialize` (the fourth, distinct exclusion set — trivial components kept hand-written for range-clamp/`Sanitize`/runtime-field/non-`m_`/entity-identity reasons; e.g. `IDComponent` is excluded because the test suite can't catch its mis-generation — see the cross-binding section). Anything with a still-unhandled non-trivial field (`Ref` of a non-Asset-derived type, `std::array`, a non-`std::string`-keyed `std::unordered_map`, a `std::unordered_set` of a non-scalar/non-sortable element (`float`, a glm vector/matrix/quat, a struct), a `std::vector` of a *non-trivial* struct, **or any non-public member** — the parser tracks `private:`/`protected:` and fails such a component safe to non-trivial) is classified non-trivial and skipped automatically (no exclusion entry needed). **The enum slice flipped several components that needed exclusion** — `Rigidbody3DComponent` (its enum is keyed `BodyType` not the `m_`-stripped `Type`, plus a runtime `m_RuntimeBodyToken` + many fields the hand-written serializer omits), `StreamingVolumeComponent` (runtime `IsLoaded` + radius clamps), `FogVolumeComponent` (Shape/Priority clamps), `UIButtonComponent` (runtime `m_State`), `UISliderComponent` (runtime `m_IsDragging`); **the glm/vector slice flipped 3 more** — `LightProbeVolumeComponent` (runtime `m_Dirty`/`m_ShowDebugProbes` + clamps), `NavAgentComponent` (runtime path/target fields), `PhysicsJoint3DComponent` (runtime `m_RuntimeConstraintToken` + dozens of clamps); **the nested-struct slice flipped 5 more, all needing exclusion** — `LODGroupComponent` (hand-written flattens the `LODGroup` sub-struct to top-level keys + omits runtime `m_GeneratedLODHandles`), `NavMeshBoundsComponent` (`SanitizeVec3` + Min≤Max ordering + drops non-finite links), and the runtime `DialogueStateComponent`/`NoiseAnimationStateComponent`/`SpringBoneStateComponent` (per-tick state that must not be serialized — the `IDComponent` no-hand-written-block blind spot) — a textbook reminder to diff the regenerated `.inl` and exclude every newly-flipped component that omits a field, clamps on load, or is runtime-only. This collapses the last big *unguarded* ECS touch-point (issue #380, fourth slice) — a forgotten field was **silent scene-data loss**, which the old name-only coverage test couldn't catch. Guarded by `ComponentSerializerCoverageTest`: existence (every component handled by `SceneSerializer.cpp` ∪ the generated `.inl`) **and** disjointness (`NoComponentIsBothHandWrittenAndGenerated` — no double-emit). **Per-field `OLO_SERIALIZE(Clamp, Min=…, Max=…)` (issue #451 Clamp slice)** later un-flipped 4 of the above: `FogVolumeComponent`, `SnowDeformerComponent`, `SpringBoneComponent`, and `NavAgentComponent` (whose remaining runtime fields now carry `OLO_SERIALIZE(Skip)`) all migrated fully generated once their clamp-only fields were annotated — see the field-level detail in the cross-binding section above. `LightProbeVolumeComponent`/`NavMeshBoundsComponent`/`StreamingVolumeComponent`/`PhysicsJoint3DComponent` stay excluded — each needs a cross-field invariant or a scale of complexity beyond a per-field range clamp. **The unordered_map/set slice migrated `PrefabComponent`** (three `std::unordered_set` override-tracking fields) fully off hand-written code — it needed no `kComponentsCustomSerialize` entry, before or after, since it was hand-written solely because `std::unordered_set` wasn't a recognised type yet. `MorphTargetComponent`'s `std::unordered_map Weights` field is also now individually trivial but the component stays hand-written for unrelated reasons (a `Ref` field where `MorphTargetSet` isn't `Asset`-derived, plus a per-value `[0,1]` clamp on load) — see the cross-binding section above for the full detail. **The vec3-Clamp slice (issue #451 follow-up) extended `Clamp` to `glm::vec3` fields** — `kClampEligible` in `ParseComponentFields` now accepts `PropType::Vec3` alongside the scalar types, and `EmitDeserializeFields`'s `PropType::Vec3` case appends a component-wise `glm::clamp`/`glm::max`/`glm::min` step (via a new `ApplyVec3Clamp` helper, the vec3 sibling of `ApplyClamp`) after the plain `.as(lhs)` finite-validated read — mirroring the hand-written `SanitizeVec3Clamped` idiom, except the finite-fallback is still whole-vector (from `DecodeVec3`), not per-component; the annotation only adds the range step on top. This migrated `BuoyancyComponent` (`m_ProbeExtents`, plus its five scalar float fields onto the pre-existing scalar `Clamp`) and `NoiseAnimationComponent` (`RotationAmplitude`/`TranslationAmplitude`, plus `ChainLength`/`Octaves`/`Frequency`/`Lacunarity`/`Gain`/`Weight` onto scalar `Clamp`) fully off `kComponentsCustomSerialize` — both were excluded *solely* for this reason, so no new exclusion was needed. `SphereAreaLightComponent` remains excluded (reject-not-clamp semantics, per the Clamp slice's own note above) and is the only component still needing a *dedicated* vec3 mechanism beyond this slice. Wired as the `GenerateBindings` custom target; `OloEngine` and `OloEngine-ScriptCore` depend on it, so it runs automatically on configure/build. If you change an annotated property, add/rename a component, and the generated `.inl` / `.cs` look stale, build `GenerateBindings` directly (then re-stage the regenerated files — they're tracked, not git-ignored). **Gotcha for future codegen slices:** the coverage tests that guard these touch-points parse generated/source files as *text* (e.g. `ComponentTupleCoverageTest::CollectTupleMembers` reads the `AllComponents = ComponentGroup<…>` marker) — when you move a touch-point from a hand-written location into a generated file, repoint the test's parser at the generated path or it fails on an empty parse. diff --git a/OloEngine/src/OloEngine/Animation/NoiseAnimationComponent.h b/OloEngine/src/OloEngine/Animation/NoiseAnimationComponent.h index 3ede8d271..7feb6768a 100644 --- a/OloEngine/src/OloEngine/Animation/NoiseAnimationComponent.h +++ b/OloEngine/src/OloEngine/Animation/NoiseAnimationComponent.h @@ -23,22 +23,30 @@ namespace OloEngine OLO_PROPERTY() u32 EndBoneIndex = 0; // chain tip bone (e.g. head or chest) OLO_PROPERTY() + OLO_SERIALIZE(Clamp, Min = 1u) u32 ChainLength = 1; // bones in the chain including the tip (>= 1) OLO_PROPERTY() + OLO_SERIALIZE(Clamp, Min = 0.0f, Max = 1e4f) f32 Frequency = 1.0f; // noise evolution speed — higher = faster wobble OLO_PROPERTY() + OLO_SERIALIZE(Clamp, Min = -6.2832f, Max = 6.2832f) glm::vec3 RotationAmplitude{ 0.08f, 0.08f, 0.08f }; // peak rotation offset per local axis (radians) OLO_PROPERTY() + OLO_SERIALIZE(Clamp, Min = -1e4f, Max = 1e4f) glm::vec3 TranslationAmplitude{ 0.0f, 0.0f, 0.0f }; // peak translation offset per local axis (model units) OLO_PROPERTY() + OLO_SERIALIZE(Clamp, Min = 1u, Max = 8u) u32 Octaves = 2; // fractal octaves (clamped 1..8) OLO_PROPERTY() + OLO_SERIALIZE(Clamp, Min = 1.0f, Max = 8.0f) f32 Lacunarity = 2.0f; // frequency multiplier per octave (clamped 1..8) OLO_PROPERTY() + OLO_SERIALIZE(Clamp, Min = 0.0f, Max = 1.0f) f32 Gain = 0.5f; // amplitude multiplier per octave (clamped 0..1) OLO_PROPERTY() u32 Seed = 0; // de-correlates entities/instances sharing a clip OLO_PROPERTY() + OLO_SERIALIZE(Clamp, Min = 0.0f, Max = 1.0f) f32 Weight = 1.0f; // 0 = animated pose, 1 = full noise offset // Trivially-copyable POD component: a single whole-struct bitwise compare diff --git a/OloEngine/src/OloEngine/Scene/ComponentReflection.h b/OloEngine/src/OloEngine/Scene/ComponentReflection.h index 276d77060..0b2258919 100644 --- a/OloEngine/src/OloEngine/Scene/ComponentReflection.h +++ b/OloEngine/src/OloEngine/Scene/ComponentReflection.h @@ -52,16 +52,18 @@ // [Min, Max] (both given), or applies a one-sided std::max/std::min (only // one given). Mirrors the SanitizeFloat/std::clamp idiom every hand-written // clamp-only component already used, so the generated block reproduces the -// same result for valid data. Only scalar Float/Int/UInt/SmallInt/ -// SmallUInt/Enum fields support Clamp today (Vec*/vector-of-T clamping is a -// follow-up) — requesting it on any other type marks the whole component -// non-trivial (fail-safe) rather than silently dropping the annotation. +// same result for valid data. Scalar Float/Int/UInt/SmallInt/SmallUInt/Enum +// fields and glm::vec3 (clamped per-component) support Clamp today +// (Vec2/Vec4/vector-of-T clamping remain a follow-up) — requesting it on +// any other type marks the whole component non-trivial (fail-safe) rather +// than silently dropping the annotation. // Note this is CLAMP-to-range, not REJECT-out-of-range: a component whose // hand-written deserialize instead *rejects* an out-of-bounds value (keeps // the constructor default rather than clamping to the bound) is a different // semantic and must stay hand-written. // Min — Clamp lower bound, e.g. Min = 0.0f. Emitted as a static_cast to the -// field's own type, so an int literal is fine for a float field. +// field's own type, so an int literal is fine for a float field. For a +// glm::vec3 field, broadcast to all three components via glm::vec3(Min). // Max — Clamp upper bound, e.g. Max = 100.0f. // // Usage: @@ -69,6 +71,9 @@ // { // OLO_SERIALIZE(Clamp, Min = 0.0f, Max = 100.0f) // f32 m_Density = 0.5f; // deserialize: m_Density = std::clamp(v, 0.0f, 100.0f) +// OLO_SERIALIZE(Clamp, Min = 0.0f, Max = 1000.0f) +// glm::vec3 m_Extents{ 0.5f, 0.5f, 0.5f }; +// // deserialize: m_Extents = glm::clamp(v, glm::vec3(0.0f), glm::vec3(1000.0f)) // }; // // May co-exist with OLO_PROPERTY on the same field (e.g. a runtime field exposed to diff --git a/OloEngine/src/OloEngine/Scene/Components.h b/OloEngine/src/OloEngine/Scene/Components.h index 005a23312..1d36da54c 100644 --- a/OloEngine/src/OloEngine/Scene/Components.h +++ b/OloEngine/src/OloEngine/Scene/Components.h @@ -3083,12 +3083,18 @@ namespace OloEngine // Local-space half-extents of the buoyancy box (the 8 corners become the // submersion probes). Default matches a 1 m cube; pick values close to the // body's collider so the displaced-volume estimate is sensible. + OLO_SERIALIZE(Clamp, Min = 0.01f, Max = 1000.0f) glm::vec3 m_ProbeExtents = { 0.5f, 0.5f, 0.5f }; - f32 m_FluidDensity = 1000.0f; ///< kg/m^3 (fresh water ~1000). Body floats when its mass < density * boxVolume. - f32 m_BuoyancyScale = 1.0f; ///< global multiplier on the Archimedes force (tuning / artistic control) - f32 m_LinearDrag = 0.8f; ///< submerged linear drag coefficient (damps vertical bobbing) - f32 m_AngularDrag = 0.5f; ///< submerged angular drag coefficient (damps rocking) + OLO_SERIALIZE(Clamp, Min = 1.0f, Max = 100000.0f) + f32 m_FluidDensity = 1000.0f; ///< kg/m^3 (fresh water ~1000). Body floats when its mass < density * boxVolume. + OLO_SERIALIZE(Clamp, Min = 0.0f, Max = 1000.0f) + f32 m_BuoyancyScale = 1.0f; ///< global multiplier on the Archimedes force (tuning / artistic control) + OLO_SERIALIZE(Clamp, Min = 0.0f, Max = 1000.0f) + f32 m_LinearDrag = 0.8f; ///< submerged linear drag coefficient (damps vertical bobbing) + OLO_SERIALIZE(Clamp, Min = 0.0f, Max = 1000.0f) + f32 m_AngularDrag = 0.5f; ///< submerged angular drag coefficient (damps rocking) + OLO_SERIALIZE(Clamp, Min = 0.001f, Max = 100.0f) f32 m_SubmergenceRamp = 0.25f; ///< metres over which a probe ramps dry -> fully wet (smooths the waterline) }; diff --git a/OloEngine/src/OloEngine/Scene/Generated/SceneDeserializeComponents.Generated.inl b/OloEngine/src/OloEngine/Scene/Generated/SceneDeserializeComponents.Generated.inl index 42b9ef6a9..6350f0dfc 100644 --- a/OloEngine/src/OloEngine/Scene/Generated/SceneDeserializeComponents.Generated.inl +++ b/OloEngine/src/OloEngine/Scene/Generated/SceneDeserializeComponents.Generated.inl @@ -25,6 +25,24 @@ // double-emit, so a new trivial component is auto-serialized and a new complex // one fails the coverage test until hand-written. +if (auto node = entity["BuoyancyComponent"]; node) +{ + auto& comp = deserializedEntity.AddComponent(); + comp.m_Enabled = node["Enabled"].as(comp.m_Enabled); + comp.m_ProbeExtents = node["ProbeExtents"].as(comp.m_ProbeExtents); + comp.m_ProbeExtents = glm::clamp(comp.m_ProbeExtents, glm::vec3(0.01f), glm::vec3(1000.0f)); + if (f32 v; ::OloEngine::YAMLUtils::TryReadFiniteF32(node["FluidDensity"], v)) + comp.m_FluidDensity = std::clamp(v, static_cast(1.0f), static_cast(100000.0f)); + if (f32 v; ::OloEngine::YAMLUtils::TryReadFiniteF32(node["BuoyancyScale"], v)) + comp.m_BuoyancyScale = std::clamp(v, static_cast(0.0f), static_cast(1000.0f)); + if (f32 v; ::OloEngine::YAMLUtils::TryReadFiniteF32(node["LinearDrag"], v)) + comp.m_LinearDrag = std::clamp(v, static_cast(0.0f), static_cast(1000.0f)); + if (f32 v; ::OloEngine::YAMLUtils::TryReadFiniteF32(node["AngularDrag"], v)) + comp.m_AngularDrag = std::clamp(v, static_cast(0.0f), static_cast(1000.0f)); + if (f32 v; ::OloEngine::YAMLUtils::TryReadFiniteF32(node["SubmergenceRamp"], v)) + comp.m_SubmergenceRamp = std::clamp(v, static_cast(0.001f), static_cast(100.0f)); +} + if (auto node = entity["CharacterController3DComponent"]; node) { auto& comp = deserializedEntity.AddComponent(); @@ -206,6 +224,28 @@ if (auto node = entity["NetworkLODComponent"]; node) comp.Level = static_cast(node["Level"].as(static_cast(comp.Level))); } +if (auto node = entity["NoiseAnimationComponent"]; node) +{ + auto& comp = deserializedEntity.AddComponent(); + comp.Enabled = node["Enabled"].as(comp.Enabled); + comp.EndBoneIndex = node["EndBoneIndex"].as(comp.EndBoneIndex); + comp.ChainLength = std::max(node["ChainLength"].as(comp.ChainLength), static_cast(1u)); + if (f32 v; ::OloEngine::YAMLUtils::TryReadFiniteF32(node["Frequency"], v)) + comp.Frequency = std::clamp(v, static_cast(0.0f), static_cast(1e4f)); + comp.RotationAmplitude = node["RotationAmplitude"].as(comp.RotationAmplitude); + comp.RotationAmplitude = glm::clamp(comp.RotationAmplitude, glm::vec3(-6.2832f), glm::vec3(6.2832f)); + comp.TranslationAmplitude = node["TranslationAmplitude"].as(comp.TranslationAmplitude); + comp.TranslationAmplitude = glm::clamp(comp.TranslationAmplitude, glm::vec3(-1e4f), glm::vec3(1e4f)); + comp.Octaves = std::clamp(node["Octaves"].as(comp.Octaves), static_cast(1u), static_cast(8u)); + if (f32 v; ::OloEngine::YAMLUtils::TryReadFiniteF32(node["Lacunarity"], v)) + comp.Lacunarity = std::clamp(v, static_cast(1.0f), static_cast(8.0f)); + if (f32 v; ::OloEngine::YAMLUtils::TryReadFiniteF32(node["Gain"], v)) + comp.Gain = std::clamp(v, static_cast(0.0f), static_cast(1.0f)); + comp.Seed = node["Seed"].as(comp.Seed); + if (f32 v; ::OloEngine::YAMLUtils::TryReadFiniteF32(node["Weight"], v)) + comp.Weight = std::clamp(v, static_cast(0.0f), static_cast(1.0f)); +} + if (auto node = entity["PerceptibleComponent"]; node) { auto& comp = deserializedEntity.AddComponent(); diff --git a/OloEngine/src/OloEngine/Scene/Generated/SceneSerializeComponents.Generated.inl b/OloEngine/src/OloEngine/Scene/Generated/SceneSerializeComponents.Generated.inl index 8614bd6bc..17bb8fec9 100644 --- a/OloEngine/src/OloEngine/Scene/Generated/SceneSerializeComponents.Generated.inl +++ b/OloEngine/src/OloEngine/Scene/Generated/SceneSerializeComponents.Generated.inl @@ -25,6 +25,21 @@ // double-emit, so a new trivial component is auto-serialized and a new complex // one fails the coverage test until hand-written. +if (entity.HasComponent()) +{ + out << YAML::Key << "BuoyancyComponent"; + out << YAML::BeginMap; // BuoyancyComponent + auto const& comp = entity.GetComponent(); + out << YAML::Key << "Enabled" << YAML::Value << comp.m_Enabled; + out << YAML::Key << "ProbeExtents" << YAML::Value << comp.m_ProbeExtents; + out << YAML::Key << "FluidDensity" << YAML::Value << comp.m_FluidDensity; + out << YAML::Key << "BuoyancyScale" << YAML::Value << comp.m_BuoyancyScale; + out << YAML::Key << "LinearDrag" << YAML::Value << comp.m_LinearDrag; + out << YAML::Key << "AngularDrag" << YAML::Value << comp.m_AngularDrag; + out << YAML::Key << "SubmergenceRamp" << YAML::Value << comp.m_SubmergenceRamp; + out << YAML::EndMap; // BuoyancyComponent +} + if (entity.HasComponent()) { out << YAML::Key << "CharacterController3DComponent"; @@ -217,6 +232,25 @@ if (entity.HasComponent()) out << YAML::EndMap; // NetworkLODComponent } +if (entity.HasComponent()) +{ + out << YAML::Key << "NoiseAnimationComponent"; + out << YAML::BeginMap; // NoiseAnimationComponent + auto const& comp = entity.GetComponent(); + out << YAML::Key << "Enabled" << YAML::Value << comp.Enabled; + out << YAML::Key << "EndBoneIndex" << YAML::Value << comp.EndBoneIndex; + out << YAML::Key << "ChainLength" << YAML::Value << comp.ChainLength; + out << YAML::Key << "Frequency" << YAML::Value << comp.Frequency; + out << YAML::Key << "RotationAmplitude" << YAML::Value << comp.RotationAmplitude; + out << YAML::Key << "TranslationAmplitude" << YAML::Value << comp.TranslationAmplitude; + out << YAML::Key << "Octaves" << YAML::Value << comp.Octaves; + out << YAML::Key << "Lacunarity" << YAML::Value << comp.Lacunarity; + out << YAML::Key << "Gain" << YAML::Value << comp.Gain; + out << YAML::Key << "Seed" << YAML::Value << comp.Seed; + out << YAML::Key << "Weight" << YAML::Value << comp.Weight; + out << YAML::EndMap; // NoiseAnimationComponent +} + if (entity.HasComponent()) { out << YAML::Key << "PerceptibleComponent"; diff --git a/OloEngine/src/OloEngine/Scene/SceneSerializer.cpp b/OloEngine/src/OloEngine/Scene/SceneSerializer.cpp index d8c0d8d0e..8a3a7b515 100644 --- a/OloEngine/src/OloEngine/Scene/SceneSerializer.cpp +++ b/OloEngine/src/OloEngine/Scene/SceneSerializer.cpp @@ -2808,26 +2808,6 @@ namespace OloEngine DeserializeWaterComponent(water, waterComponent); } - if (auto buoyancyComponent = entity["BuoyancyComponent"]; buoyancyComponent) - { - auto& buoyancy = deserializedEntity.AddComponent(); - buoyancy.m_Enabled = buoyancyComponent["Enabled"].as(buoyancy.m_Enabled); - buoyancy.m_ProbeExtents = buoyancyComponent["ProbeExtents"].as(buoyancy.m_ProbeExtents); - buoyancy.m_FluidDensity = buoyancyComponent["FluidDensity"].as(buoyancy.m_FluidDensity); - buoyancy.m_BuoyancyScale = buoyancyComponent["BuoyancyScale"].as(buoyancy.m_BuoyancyScale); - buoyancy.m_LinearDrag = buoyancyComponent["LinearDrag"].as(buoyancy.m_LinearDrag); - buoyancy.m_AngularDrag = buoyancyComponent["AngularDrag"].as(buoyancy.m_AngularDrag); - buoyancy.m_SubmergenceRamp = buoyancyComponent["SubmergenceRamp"].as(buoyancy.m_SubmergenceRamp); - - // Validate every float read from YAML (project rule). - SanitizeVec3Clamped(buoyancy.m_ProbeExtents, 0.01f, 1000.0f, { 0.5f, 0.5f, 0.5f }); - SanitizeFloat(buoyancy.m_FluidDensity, 1.0f, 100000.0f, 1000.0f); - SanitizeFloat(buoyancy.m_BuoyancyScale, 0.0f, 1000.0f, 1.0f); - SanitizeFloat(buoyancy.m_LinearDrag, 0.0f, 1000.0f, 0.8f); - SanitizeFloat(buoyancy.m_AngularDrag, 0.0f, 1000.0f, 0.5f); - SanitizeFloat(buoyancy.m_SubmergenceRamp, 0.001f, 100.0f, 0.25f); - } - if (auto decalComponent = entity["DecalComponent"]; decalComponent) { DeserializeDecalComponent(deserializedEntity, decalComponent); @@ -3627,32 +3607,6 @@ namespace OloEngine SanitizeFloat(ik.ChainWeight, 0.0f, 1.0f, 1.0f); } - if (auto noiseNode = entity["NoiseAnimationComponent"]; noiseNode) - { - auto& noise = deserializedEntity.AddComponent(); - TrySet(noise.Enabled, noiseNode["Enabled"]); - TrySet(noise.EndBoneIndex, noiseNode["EndBoneIndex"]); - TrySet(noise.ChainLength, noiseNode["ChainLength"]); - TrySet(noise.Frequency, noiseNode["Frequency"]); - TrySet(noise.RotationAmplitude, noiseNode["RotationAmplitude"]); - TrySet(noise.TranslationAmplitude, noiseNode["TranslationAmplitude"]); - TrySet(noise.Octaves, noiseNode["Octaves"]); - TrySet(noise.Lacunarity, noiseNode["Lacunarity"]); - TrySet(noise.Gain, noiseNode["Gain"]); - TrySet(noise.Seed, noiseNode["Seed"]); - TrySet(noise.Weight, noiseNode["Weight"]); - - // Sanitize float/vector fields — replace NaN/Inf with defaults and clamp to valid ranges - noise.ChainLength = std::max(1u, noise.ChainLength); - noise.Octaves = std::clamp(noise.Octaves, 1u, 8u); - SanitizeFloat(noise.Frequency, 0.0f, 1e4f, 1.0f); - SanitizeVec3Clamped(noise.RotationAmplitude, -6.2832f, 6.2832f, glm::vec3(0.08f)); - SanitizeVec3Clamped(noise.TranslationAmplitude, -1e4f, 1e4f, glm::vec3(0.0f)); - SanitizeFloat(noise.Lacunarity, 1.0f, 8.0f, 2.0f); - SanitizeFloat(noise.Gain, 0.0f, 1.0f, 0.5f); - SanitizeFloat(noise.Weight, 0.0f, 1.0f, 1.0f); - } - // Auto-generated trivial-component deserialize blocks (OloHeaderTool, issue // #380) — the read-side complement of SceneSerializeComponents.Generated.inl. // Floats are validated with std::isfinite; a missing key keeps the default. @@ -4941,23 +4895,6 @@ namespace OloEngine out << YAML::EndMap; // WaterComponent } - if (entity.HasComponent()) - { - out << YAML::Key << "BuoyancyComponent"; - out << YAML::BeginMap; - - auto const& buoyancy = entity.GetComponent(); - out << YAML::Key << "Enabled" << YAML::Value << buoyancy.m_Enabled; - out << YAML::Key << "ProbeExtents" << YAML::Value << buoyancy.m_ProbeExtents; - out << YAML::Key << "FluidDensity" << YAML::Value << buoyancy.m_FluidDensity; - out << YAML::Key << "BuoyancyScale" << YAML::Value << buoyancy.m_BuoyancyScale; - out << YAML::Key << "LinearDrag" << YAML::Value << buoyancy.m_LinearDrag; - out << YAML::Key << "AngularDrag" << YAML::Value << buoyancy.m_AngularDrag; - out << YAML::Key << "SubmergenceRamp" << YAML::Value << buoyancy.m_SubmergenceRamp; - - out << YAML::EndMap; // BuoyancyComponent - } - if (entity.HasComponent()) { out << YAML::Key << "DecalComponent"; @@ -5744,27 +5681,6 @@ namespace OloEngine out << YAML::EndMap; // IKTargetComponent } - if (entity.HasComponent()) - { - out << YAML::Key << "NoiseAnimationComponent"; - out << YAML::BeginMap; - - auto const& noise = entity.GetComponent(); - out << YAML::Key << "Enabled" << YAML::Value << noise.Enabled; - out << YAML::Key << "EndBoneIndex" << YAML::Value << noise.EndBoneIndex; - out << YAML::Key << "ChainLength" << YAML::Value << noise.ChainLength; - out << YAML::Key << "Frequency" << YAML::Value << noise.Frequency; - out << YAML::Key << "RotationAmplitude" << YAML::Value << noise.RotationAmplitude; - out << YAML::Key << "TranslationAmplitude" << YAML::Value << noise.TranslationAmplitude; - out << YAML::Key << "Octaves" << YAML::Value << noise.Octaves; - out << YAML::Key << "Lacunarity" << YAML::Value << noise.Lacunarity; - out << YAML::Key << "Gain" << YAML::Value << noise.Gain; - out << YAML::Key << "Seed" << YAML::Value << noise.Seed; - out << YAML::Key << "Weight" << YAML::Value << noise.Weight; - - out << YAML::EndMap; // NoiseAnimationComponent - } - // Auto-generated trivial-component serialize blocks (OloHeaderTool, issue #380). // One `if (entity.HasComponent()) { … }` per all-trivial component not in // the generator's kComponentsCustomSerialize set. Editing the touch-point out diff --git a/OloEngine/tests/CMakeLists.txt b/OloEngine/tests/CMakeLists.txt index a8ec6c04d..a0a50e8eb 100644 --- a/OloEngine/tests/CMakeLists.txt +++ b/OloEngine/tests/CMakeLists.txt @@ -276,6 +276,7 @@ add_executable(OloEngine-Tests Serialization/FontAssetPackSerializerTest.cpp Serialization/NestedStructSerializerCodegenTest.cpp Serialization/ContainerSerializerCodegenTest.cpp + Serialization/Vec3ClampSerializerCodegenTest.cpp # SaveGame Tests SaveGame/SaveGameFileTest.cpp SaveGame/SaveGameManagerTest.cpp diff --git a/OloEngine/tests/Serialization/Vec3ClampSerializerCodegenTest.cpp b/OloEngine/tests/Serialization/Vec3ClampSerializerCodegenTest.cpp new file mode 100644 index 000000000..3d7c7ee14 --- /dev/null +++ b/OloEngine/tests/Serialization/Vec3ClampSerializerCodegenTest.cpp @@ -0,0 +1,161 @@ +#include "OloEnginePCH.h" +#include + +// OLO_TEST_LAYER: unit +// ============================================================================= +// Vec3ClampSerializerCodegenTest +// +// Pins the OloHeaderTool scene-serializer codegen for OLO_SERIALIZE(Clamp, Min=…, +// Max=…) on a glm::vec3 field (issue #451's vec3-Clamp slice — the follow-up to +// the scalar-only Clamp slice, #536). tools/OloHeaderTool/main.cpp's +// ParseComponentFields now accepts PropType::Vec3 in its Clamp-eligible set, and +// EmitDeserializeFields emits the plain `.as(lhs)` finite-validated +// read followed by a per-component glm::clamp/glm::max/glm::min step — mirroring +// the hand-written SanitizeVec3Clamped idiom used by BuoyancyComponent:: +// m_ProbeExtents and NoiseAnimationComponent::RotationAmplitude/ +// TranslationAmplitude before this slice migrated them onto the generated path +// (see ComponentSerializerCoverageTest for their live coverage). +// +// The mirror function below accesses the vec3 through a member of a small local +// "holder" struct (`holder.Field`), matching the real generator's `comp.m_Field` +// access shape used throughout this codegen (see ContainerSerializerCodegenTest +// / NestedStructSerializerCodegenTest for the same convention). +// +// If you change EmitDeserializeFields' PropType::Vec3 branch or ApplyVec3Clamp, +// update the mirrored Deserialize* helper below to match. +// ============================================================================= + +#include "OloEngine/Core/Base.h" +#include "OloEngine/Core/YAMLConverters.h" + +#include +#include + +#include + +namespace OloEngine::Tests +{ + namespace + { + struct ExtentsHolder + { + glm::vec3 Extents{ 0.5f, 0.5f, 0.5f }; + }; + + // --- Mirror of EmitDeserializeFields' PropType::Vec3 branch with both + // Min and Max given (issue #451 vec3-Clamp slice) --- + void DeserializeExtentsBothBounds(const YAML::Node& node, ExtentsHolder& comp) + { + comp.Extents = node["Extents"].as(comp.Extents); + comp.Extents = glm::clamp(comp.Extents, glm::vec3(0.01f), glm::vec3(1000.0f)); + } + + // --- Mirror of EmitDeserializeFields' PropType::Vec3 branch, Min-only --- + void DeserializeExtentsMinOnly(const YAML::Node& node, ExtentsHolder& comp) + { + comp.Extents = node["Extents"].as(comp.Extents); + comp.Extents = glm::max(comp.Extents, glm::vec3(0.0f)); + } + + // --- Mirror of EmitDeserializeFields' PropType::Vec3 branch, Max-only --- + void DeserializeExtentsMaxOnly(const YAML::Node& node, ExtentsHolder& comp) + { + comp.Extents = node["Extents"].as(comp.Extents); + comp.Extents = glm::min(comp.Extents, glm::vec3(10.0f)); + } + + void SerializeExtents(YAML::Emitter& out, const ExtentsHolder& comp) + { + out << YAML::Key << "Extents" << YAML::Value << comp.Extents; + } + + std::string EmitToString(const ExtentsHolder& comp) + { + YAML::Emitter out; + out << YAML::BeginMap; + SerializeExtents(out, comp); + out << YAML::EndMap; + return out.c_str(); + } + } // namespace + + // An in-range vec3 round-trips unchanged. + TEST(Vec3ClampSerializerCodegen, InRangeValueRoundTrips) + { + ExtentsHolder src; + src.Extents = { 2.0f, 3.0f, 4.0f }; + const std::string yaml = EmitToString(src); + + ExtentsHolder roundtripped; + DeserializeExtentsBothBounds(YAML::Load(yaml), roundtripped); + EXPECT_FLOAT_EQ(roundtripped.Extents.x, 2.0f); + EXPECT_FLOAT_EQ(roundtripped.Extents.y, 3.0f); + EXPECT_FLOAT_EQ(roundtripped.Extents.z, 4.0f); + } + + // Each component is clamped independently into [Min, Max] — mirrors + // SanitizeVec3Clamped's per-component behavior. + TEST(Vec3ClampSerializerCodegen, OutOfRangeComponentsClampIndependently) + { + const char* yaml = R"(Extents: [-5.0, 2000.0, 500.0])"; + + ExtentsHolder dest; + DeserializeExtentsBothBounds(YAML::Load(yaml), dest); + + EXPECT_FLOAT_EQ(dest.Extents.x, 0.01f); // -5.0 clamped up to Min + EXPECT_FLOAT_EQ(dest.Extents.y, 1000.0f); // 2000.0 clamped down to Max + EXPECT_FLOAT_EQ(dest.Extents.z, 500.0f); // already in range + } + + // A missing key keeps the pre-existing (constructor-default) value, same as + // the plain (non-Clamp) glm::vec3 path — the clamp step never runs on an + // absent node because DecodeVec3 already left the fallback in place. + TEST(Vec3ClampSerializerCodegen, MissingKeyKeepsDefault) + { + ExtentsHolder dest; + DeserializeExtentsBothBounds(YAML::Load("{ Unrelated: 1 }"), dest); + EXPECT_FLOAT_EQ(dest.Extents.x, 0.5f); + EXPECT_FLOAT_EQ(dest.Extents.y, 0.5f); + EXPECT_FLOAT_EQ(dest.Extents.z, 0.5f); + } + + // Min-only clamps up but never down (glm::max broadcast). + TEST(Vec3ClampSerializerCodegen, MinOnlyClampsUpNotDown) + { + const char* yaml = R"(Extents: [-1.0, 5.0, -0.001])"; + ExtentsHolder dest; + DeserializeExtentsMinOnly(YAML::Load(yaml), dest); + EXPECT_FLOAT_EQ(dest.Extents.x, 0.0f); + EXPECT_FLOAT_EQ(dest.Extents.y, 5.0f); + EXPECT_FLOAT_EQ(dest.Extents.z, 0.0f); + } + + // Max-only clamps down but never up (glm::min broadcast). + TEST(Vec3ClampSerializerCodegen, MaxOnlyClampsDownNotUp) + { + const char* yaml = R"(Extents: [20.0, 5.0, 9.999])"; + ExtentsHolder dest; + DeserializeExtentsMaxOnly(YAML::Load(yaml), dest); + EXPECT_FLOAT_EQ(dest.Extents.x, 10.0f); + EXPECT_FLOAT_EQ(dest.Extents.y, 5.0f); + EXPECT_FLOAT_EQ(dest.Extents.z, 9.999f); + } + + // A non-finite component falls back to the WHOLE pre-existing vector (not a + // per-component fallback) before the clamp step runs — DecodeVec3 rejects the + // whole node if any component is non-finite, matching every other plain + // glm::vec3 field's behavior; the Clamp annotation only adds a range step on + // top, it does not change the finite-fallback semantics. + TEST(Vec3ClampSerializerCodegen, NonFiniteComponentFallsBackToWholeDefault) + { + const char* yaml = R"(Extents: [.nan, 5.0, 3.0])"; + ExtentsHolder dest; + dest.Extents = { 7.0f, 7.0f, 7.0f }; + DeserializeExtentsBothBounds(YAML::Load(yaml), dest); + + // Whole-vector fallback kept (7,7,7), then clamped into [0.01, 1000]. + EXPECT_FLOAT_EQ(dest.Extents.x, 7.0f); + EXPECT_FLOAT_EQ(dest.Extents.y, 7.0f); + EXPECT_FLOAT_EQ(dest.Extents.z, 7.0f); + } +} // namespace OloEngine::Tests diff --git a/tools/OloHeaderTool/main.cpp b/tools/OloHeaderTool/main.cpp index a6646970a..d944efff8 100644 --- a/tools/OloHeaderTool/main.cpp +++ b/tools/OloHeaderTool/main.cpp @@ -1013,14 +1013,20 @@ static const std::set kComponentsCustomOnRemove = { // // Each entry is a trivial component whose hand-written block does something the // plain round-trip generator must NOT silently drop: -// * BuoyancyComponent / SphereAreaLightComponent / SpringBoneComponent / -// NoiseAnimationComponent / DialogueComponent / PerceptionComponent / -// IKTargetComponent — deserialize clamps / Sanitize()s float (or u32 chain- -// length / vector) ranges; auto-generating would relax those guards. +// * SphereAreaLightComponent — REJECT-not-clamp semantics (keeps the constructor +// default rather than clamping an out-of-bounds value), a different semantic +// from OLO_SERIALIZE(Clamp) — see ComponentReflection.h's Clamp doc comment. +// * DialogueComponent / PerceptionComponent / IKTargetComponent — deserialize +// clamps / Sanitize()s float (or vector) ranges beyond what a per-field Clamp +// annotation alone expresses; auto-generating would relax those guards. // (PerceptionComponent also intentionally does NOT restore its runtime-derived // fields — HasVisibleTarget / VisibleTarget / LastKnownPosition / … — on load.) -// * SnowDeformerComponent — serialize/deserialize delegate to a hand-written -// helper (Serialize/DeserializeSnowDeformerComponent), not a flat field list. +// BuoyancyComponent / NoiseAnimationComponent / SpringBoneComponent / +// SnowDeformerComponent / FogVolumeComponent / NavAgentComponent MIGRATED off +// this set onto OLO_SERIALIZE(Clamp) — the last two, BuoyancyComponent and +// NoiseAnimationComponent, by the vec3-Clamp follow-up slice (their +// SanitizeVec3Clamped-equivalent bounds are now per-field +// OLO_SERIALIZE(Clamp, Min=…, Max=…) annotations on the glm::vec3 members). // * ScriptComponent — serializes the C# ScriptField map owned by ScriptEngine, // not just its ClassName member (the parser only sees ClassName). // * VehicleComponent — has a runtime-only RuntimeVehicleToken field the @@ -1147,7 +1153,6 @@ static const std::set kComponentsCustomOnRemove = { // listing one here while ALSO leaving (or removing) its hand-written block is a // loud test failure, never a silent double-emit / drop. static const std::set kComponentsCustomSerialize = { - "BuoyancyComponent", "CinematicComponent", "DecalComponent", "DialogueComponent", @@ -1159,7 +1164,6 @@ static const std::set kComponentsCustomSerialize = { "MeshComponent", "ModelComponent", "NavMeshBoundsComponent", - "NoiseAnimationComponent", "NoiseAnimationStateComponent", "PerceptionComponent", "PhaseComponent", @@ -1293,10 +1297,12 @@ struct SerField // OLO_SERIALIZE(Clamp, Min=…, Max=…) — issue #451's Clamp slice. When set, the // generated deserialize ranges the read value into [clampMin, clampMax] (both - // set) or applies a one-sided std::max/std::min (only one set). Only ever set - // for a scalar (non-vector) Float/Int/UInt/SmallInt/SmallUInt/Enum field — - // ParseComponentFields fails the whole component non-trivial if Clamp is - // requested on any other type rather than silently dropping it. + // set) or applies a one-sided std::max/std::min (only one set). Set for a + // scalar Float/Int/UInt/SmallInt/SmallUInt/Enum field, or (the vec3-Clamp + // follow-up slice) a glm::vec3 field — clamped per-component via glm::clamp/ + // glm::max/glm::min instead of std::clamp. ParseComponentFields fails the + // whole component non-trivial if Clamp is requested on any other type rather + // than silently dropping it. bool hasClamp{ false }; std::optional clampMin; std::optional clampMax; @@ -2124,14 +2130,16 @@ static ComponentSerInfo ParseComponentFields(std::string body, ambiguous = true; continue; } - // Clamp (issue #451) is only supported on scalar Float/Int/UInt/SmallInt/ - // SmallUInt/Enum fields — requesting it on any other type (Vec*/Struct/…), - // or with neither Min nor Max given, marks the whole component non-trivial - // rather than silently dropping the annotation (ambiguity fails safe, same + // Clamp (issue #451) is supported on scalar Float/Int/UInt/SmallInt/ + // SmallUInt/Enum fields, and — the vec3-Clamp follow-up slice — glm::vec3 + // (clamped per-component, mirroring the hand-written SanitizeVec3Clamped + // idiom). Requesting it on any other type (Vec2/Vec4/Struct/…), or with + // neither Min nor Max given, marks the whole component non-trivial rather + // than silently dropping the annotation (ambiguity fails safe, same // discipline as every other unsupported construct in this parser). static const std::set kClampEligible = { PropType::Float, PropType::Int, PropType::UInt, - PropType::SmallInt, PropType::SmallUInt, PropType::Enum + PropType::SmallInt, PropType::SmallUInt, PropType::Enum, PropType::Vec3 }; if (fieldHasClamp && (!kClampEligible.contains(pt) || (!fieldClampMin && !fieldClampMax))) { @@ -2428,23 +2436,57 @@ static std::string SceneWriteExpr(PropType t, const std::string& expr) } } +// Shared clamp-expression assembler behind ApplyClamp/ApplyVec3Clamp: given the +// clamp-function namespace prefix ("std::" or "glm::") and the ALREADY-WRAPPED +// Min/Max bound expressions (a scalar static_cast for ApplyClamp, a glm::vec3(...) +// broadcast for ApplyVec3Clamp), assembles the same both-bounds -> clamp / +// one-sided -> max/min / neither -> unchanged branching (issue #451's Clamp +// annotation: both bounds given -> clamp, one bound given -> max/min, neither +// given -> returns `expr` unchanged — ParseComponentFields never produces this +// last case since Clamp requires at least one of Min/Max, but the no-op fallback +// keeps the callers safe to call unconditionally once they've checked f.hasClamp). +static std::string AssembleClampExpr(const std::string& expr, const std::string& ns, + const std::optional& wrappedMin, + const std::optional& wrappedMax) +{ + if (wrappedMin && wrappedMax) + return ns + "clamp(" + expr + ", " + *wrappedMin + ", " + *wrappedMax + ")"; + if (wrappedMin) + return ns + "max(" + expr + ", " + *wrappedMin + ")"; + if (wrappedMax) + return ns + "min(" + expr + ", " + *wrappedMax + ")"; + return expr; +} + // Wrap a deserialize value expression `expr` (already of type `castType`, e.g. // "f32" / "i32" / "decltype(lhs)") in a range clamp per the field's Clamp -// annotation (issue #451): both bounds given -> std::clamp, one bound given -> -// std::max/std::min, neither given -> returns `expr` unchanged (ParseComponentFields -// never produces this last case — Clamp requires at least one of Min/Max — but the -// no-op fallback keeps this helper safe to call unconditionally by a caller that -// already checked f.hasClamp). +// annotation (issue #451): each bound is cast to the field's own type, so +// `Min = 0` is fine on a float field. static std::string ApplyClamp(const std::string& expr, const std::string& castType, const SerField& f) { - if (f.clampMin && f.clampMax) - return "std::clamp(" + expr + ", static_cast<" + castType + ">(" + *f.clampMin + "), static_cast<" + - castType + ">(" + *f.clampMax + "))"; + std::optional wrappedMin, wrappedMax; if (f.clampMin) - return "std::max(" + expr + ", static_cast<" + castType + ">(" + *f.clampMin + "))"; + wrappedMin = "static_cast<" + castType + ">(" + *f.clampMin + ")"; if (f.clampMax) - return "std::min(" + expr + ", static_cast<" + castType + ">(" + *f.clampMax + "))"; - return expr; + wrappedMax = "static_cast<" + castType + ">(" + *f.clampMax + ")"; + return AssembleClampExpr(expr, "std::", wrappedMin, wrappedMax); +} + +// Wrap a glm::vec3 deserialize expression `expr` in a PER-COMPONENT range clamp +// per the field's Clamp annotation (issue #451's vec3-Clamp slice) — mirrors the +// hand-written SanitizeVec3Clamped idiom, applied AFTER the plain `.as` +// read (which already finiteness-validates the whole vector, falling back to the +// pre-existing value on any non-finite component — see DecodeVec3). glm::clamp / +// glm::max / glm::min have vector-vs-scalar overloads that broadcast the scalar +// bound across all three components, matching std::clamp/max/min per component. +static std::string ApplyVec3Clamp(const std::string& expr, const SerField& f) +{ + std::optional wrappedMin, wrappedMax; + if (f.clampMin) + wrappedMin = "glm::vec3(" + *f.clampMin + ")"; + if (f.clampMax) + wrappedMax = "glm::vec3(" + *f.clampMax + ")"; + return AssembleClampExpr(expr, "glm::", wrappedMin, wrappedMax); } // The element loop / temp variable base name for depth `d`. Depth 0 keeps the @@ -2759,7 +2801,14 @@ static void EmitDeserializeFields(std::ostream& out, const std::vector out << indent << lhs << " = " << key << ".as(" << lhs << ");\n"; break; case PropType::Vec3: + // A Clamp annotation (issue #451's vec3-Clamp slice) ranges each + // component of the decoded value into [Min, Max] AFTER the finite- + // validated read — mirrors the hand-written SanitizeVec3Clamped + // idiom (component-wise fallback-then-clamp), applied here as a + // separate clamp step on top of DecodeVec3's whole-vector fallback. out << indent << lhs << " = " << key << ".as(" << lhs << ");\n"; + if (f.hasClamp) + out << indent << lhs << " = " << ApplyVec3Clamp(lhs, f) << ";\n"; break; case PropType::Vec4: out << indent << lhs << " = " << key << ".as(" << lhs << ");\n";