fix(content): isfinite-guard float reads in Quest/Item/Dialogue deser…

[drsnuggles8]

…ializers

Summary by CodeRabbit

• Bug Fixes
  • Improved handling of malformed numeric values in dialogue, item, and quest data so invalid values no longer break loading or create bad in-game/editor behavior.
  • Added safeguards for editor positions, item weights, attribute modifiers, quest time limits, and cooldowns, with sensible fallback values when needed.
  • Prevented negative or non-finite values from causing unstable or unexpected results in loaded content.

[coderabbitai]

📝 Walkthrough

Walkthrough

Dialogue, item, and quest YAML loaders now sanitize non-finite float inputs. Dialogue editor positions and float properties, item weights and attribute modifiers, and quest time limit and cooldown fields use finite fallbacks or clamping. New tests and CMake entries cover the updated parsing paths.

Changes

Finite float validation

Layer / File(s)	Summary
Dialogue float sanitization OloEngine/src/OloEngine/Dialogue/DialogueTreeSerializer.cpp, OloEngine/tests/DialogueTreeFloatValidationTest.cpp, OloEngine/tests/CMakeLists.txt	DialogueTreeSerializer rejects non-finite float properties, sanitizes EditorPosition.x/y, and adds tests for non-finite and finite YAML values.
Gameplay database float sanitization OloEngine/src/OloEngine/Gameplay/Inventory/ItemDatabase.cpp, OloEngine/src/OloEngine/Gameplay/Quest/QuestDatabase.cpp, OloEngine/tests/ItemDatabaseFloatValidationTest.cpp, OloEngine/tests/QuestDatabaseFloatValidationTest.cpp, OloEngine/tests/CMakeLists.txt	ItemDatabase and QuestDatabase sanitize non-finite float fields, clamp negative weight and cooldown values, and add tests for NaN/inf, negative, and valid inputs.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: finite-value guards for float deserialization in Quest, Item, and Dialogue content.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@OloEngine/src/OloEngine/Gameplay/Quest/QuestDatabase.cpp`:
- Around line 16-24: The SanitizeFinite helper is duplicated across multiple
serializers/databases, with the QuestDatabase version matching the ItemDatabase
one except for the log prefix. Consolidate the shared 4-argument SanitizeFinite
logic into a common utility used by QuestDatabase, ItemDatabase, and the
DialogueTreeSerializer wrapper, and pass the prefix as a parameter so only the
wrapper-specific behavior remains in each file.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 149de6ba-2765-4596-8997-906a481684d5

📥 Commits

Reviewing files that changed from the base of the PR and between 0c56611 and 7e9aa08.

📒 Files selected for processing (7)

• OloEngine/src/OloEngine/Dialogue/DialogueTreeSerializer.cpp
• OloEngine/src/OloEngine/Gameplay/Inventory/ItemDatabase.cpp
• OloEngine/src/OloEngine/Gameplay/Quest/QuestDatabase.cpp
• OloEngine/tests/CMakeLists.txt
• OloEngine/tests/DialogueTreeFloatValidationTest.cpp
• OloEngine/tests/ItemDatabaseFloatValidationTest.cpp
• OloEngine/tests/QuestDatabaseFloatValidationTest.cpp

[drsnuggles8]

🤖 Self-review (finish-pr) @ 7e9aa08a6df0507d2dfd8706f9e6bf34226a3a08

Reviewed the full PR diff (origin/master...HEAD, 7 files / +411) at high effort — 2 finder passes (correctness; cleanup+conventions), findings verified against the code.

• Correctness: 0 bugs. Notably checked the one real risk — a "float"-typed dialogue property falling through to a std::string variant when non-finite — and confirmed it's safe: every runtime consumer of DialogueNodeData::Properties (DialogueSystem.cpp, DialogueVariables.cpp) uses std::get_if (returns nullptr, never throws bad_variant_access). Quest/Item consumers (QuestJournal > 0.0f timers, Inventory::GetTotalWeight) are exactly what the guards protect.
• Conventions (cpp-coding-quality.md): PASS — no raw float ==/!= in the new tests (EXPECT_FLOAT_EQ/std::isfinite used); <cmath> included everywhere std::isfinite is called; tests classified // OLO_TEST_LAYER: unit and registered.
• Tests genuinely exercise the guards: .nan/.inf decode to real non-finite values via yaml-cpp as<f32>() and std::stof (not the .as<f32>(default) fallback), so each test would FAIL if its guard were removed; valid-value controls confirm no over-rejection.
• Findings: 2 — both dismissed:
  • Reuse Math::IsFinite for the inner check — dismissed: the repo's serialization-boundary convention is bare std::isfinite per-site (SaveGameComponentSerializer + Animation IK/Procedural solvers); the reusable unit is the fallback+warn wrapper, not the predicate.
  • Fixed temp-dir name in Quest/Item tests — dismissed: per-process isolation is already correct (gtest single-threaded; SetUp recreates; unique per-test filenames; Quest/Item use distinct dirs); cross-process collision needs two concurrent runs sharing a machine temp dir, which isolated per-job CI runners don't do.

Local build (OloEngine-Tests Debug) clean; the 11 new Quest/Item/Dialogue float-validation tests pass.

[sonarqubecloud]

Quality Gate passed

Issues
9 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud