From c643d4388fdbb0a9827d8619deea8844ad832261 Mon Sep 17 00:00:00 2001 From: Sergey Lapin Date: Wed, 28 Oct 2026 13:38:24 +0300 Subject: [PATCH] Phase 6.7 complete checkpoint - checked --- src/features/editScene/AGENTS.md | 45 ++- .../editScene/demos/demo-taxi/PLAN.md | 44 ++- .../editScene/demos/demo-taxi/demo_main.cpp | 326 ++++++++++++++++-- .../editScene/systems/AnimationTreeSystem.cpp | 30 +- .../editScene/tests/animation_travel_test.cpp | 97 ++++++ 5 files changed, 490 insertions(+), 52 deletions(-) diff --git a/src/features/editScene/AGENTS.md b/src/features/editScene/AGENTS.md index 2fc182e..f5f5a21 100644 --- a/src/features/editScene/AGENTS.md +++ b/src/features/editScene/AGENTS.md @@ -342,9 +342,12 @@ cd demos/demo-taxi # ...or headless animation-travel probe (phase 6.7): on the default # interior scene, a synthetic registry tree probes the animation state # machine travel — startState boot marker, travelTo hop-through (no -# insta-switch, each hop completes the current clip), the setState -# cancel, the unreachable-target degrade to the plain switch and the -# nested endState hold (see "Animation state machine travel" below): +# insta-switch, each atEnd hop completes the current clip), the +# setState cancel and mid-flight replacement, the unreachable-target +# degrade to the plain switch, the nested endState hold, per-link +# immediate/atEnd wait modes, the forced first hop, unknown +# machine/target no-ops, the disabled-tree no-op and the deleted-link +# cancel (see "Animation state machine travel" below): ./demoTaxi --headless --test-anim-travel # ...or probe natural (non-forced) high-detail spawns from a given player # position; prints how many population NPCs are spawned plus per-NPC @@ -2120,8 +2123,11 @@ each hop's clip before cross-fading to the next: - **Robustness (rule 7):** deleted states/links, a lost machine entry or a vanished target cancel the travel and warn ONCE per (entity, reason, state) — the warning key is - `"smName|reason|stateName"`, never per frame. A disabled tree - degrades quietly (the disabled branch drops pendings anyway). + `"smName|reason|stateName"`, never per frame. A connect link + deleted mid-travel (registry edit + `markModified`) cancels the + pending hop instead of firing along the dead link + (`travelLinkLost`). A disabled tree degrades quietly (the + disabled branch drops pendings anyway). - **Nested machines (rule 6):** `AnimationTreeNode::startState` / `endState` mark the entry/exit states of a machine used as a state of an upper machine (both optional, empty = legacy behavior). @@ -2158,14 +2164,27 @@ unknown state names and duplicate links ARE errors). UI: the node editor's Travel section (ui/AnimationTreeNodeEditor.cpp) edits links (with the per-link wait mode), the start/end dropdowns per machine; the registry editor validates on save. Coverage: -`tests/animation_travel_test.cpp` (CTest `animationTravelTest` — BFS -ties/cycles/unreachable, JSON round trip with backward compatibility, -link waitMode serialization, validation, degrade/cancel) and the -demo probe `demoTaxi --headless --test-anim-travel` (synthetic -registry tree `test_travel_nested` registered at runtime and removed -on PASS; asserts startState boot, hop order with no insta-switch, -cancel, degrade, the nested hold, immediate-vs-atEnd link timing and -the forced first hop). +`tests/animation_travel_test.cpp` (CTest `animationTravelTest`, 11 +cases — BFS shortest/ties/cycles/self-links/empty endpoints, graph +validation incl. duplicates across differing wait modes, JSON round +trip + malformed-link tolerance + backward compatibility, link +waitMode serialization) and the demo probe +`demoTaxi --headless --test-anim-travel` (synthetic registry tree +`test_travel_nested` registered at runtime and removed on PASS; 11 +phases: startState boot, hop order with no insta-switch, setState +cancel, unreachable degrade, the nested hold, immediate-vs-atEnd link +timing, forced first hop, mid-flight replacement, unknown +machine/target + self-travel no-ops, disabled-tree no-op, deleted-link +cancel). + +Registry auto-save caveat for runtime probes: +`AnimationTreeRegistry::registerTree` / `removeTree` / `markModified` +all call `autoSave()`, which rewrites `animation_tree.json` in the +current working directory (indent 4, unordered-map tree order). A +probe that registers a synthetic tree therefore mutates the staged +file while it runs — always `removeTree` on PASS, and re-stage the +file from the source tree after test runs (a killed run otherwise +leaves the synthetic tree behind). ### Character facing convention (frontAxis) diff --git a/src/features/editScene/demos/demo-taxi/PLAN.md b/src/features/editScene/demos/demo-taxi/PLAN.md index 1fe40ff..250f9c7 100644 --- a/src/features/editScene/demos/demo-taxi/PLAN.md +++ b/src/features/editScene/demos/demo-taxi/PLAN.md @@ -4209,16 +4209,18 @@ DONE.** ### Phase 6.7 — Animation state machine travel -**Status: DONE (2026-10-28) — 6.7.1 through 6.7.6 complete: connect +**Status: DONE (2026-10-28) — 6.7.1 through 6.7.7 complete: connect links + start/end markers (data model, JSON round trip, registry/node editors with save validation), the `travelTo` runtime (BFS over connect links, per-hop clip completion, rule-2 degrade, rule-4 cancel), nested rule-6 start/end gating with the hold-abort re-entry, the controller migration (locomotion, vehicle enter/exit, taxi -board/alight, smart objects, path following), the stock-tree links and -the per-link wait modes with the `travelTo` first-hop escape hatch, -covered by `animation_travel_test` (8 cases), the demoTaxi -`--test-anim-travel` probe and the full demo battery.** +board/alight, smart objects, path following), the stock-tree links, +the per-link wait modes with the `travelTo` first-hop escape hatch and +the review-hardening pass (rule-7 deleted-link cancel plus the +expanded test matrix), covered by `animation_travel_test` (11 cases), +the demoTaxi `--test-anim-travel` probe (11 phases) and the full demo +battery.** #### The idea (plain words) @@ -4485,6 +4487,35 @@ feet sliding. frame 4 / runs at frame 6 (was 608 / 683); quick battery (switch / vehicle-drive / taxi / parking) PASS exit 0. +**Task 6.7.7 — Review hardening + full coverage audit. DONE +(2026-10-28).** + +- Re-read the whole travel implementation against rules 1-7 and the + wait-mode contract, close any gap found, and make every rule + observable in a test. +- Done when: every contract rule has at least one discriminating + unit-test or probe assertion, the deleted-link anomaly cancels + instead of hopping, and docs match the code. +- **Implemented:** rule-7 gap fix — `processTravelCheck` now resolves + the hop's link up front and CANCELS with a `travelLinkLost` + warning when the link vanished mid-travel (previously a missing + link silently fell back to atEnd and the hop fired along the dead + link). Unit tests 8 -> 11: BFS cycles/self-links/empty endpoints, + duplicate detection across differing wait modes, malformed-link + JSON tolerance (missing endpoints, non-object entries, non-array + connectLinks, old files). Runtime probe 7 -> 11 phases, adding + rule-4 mid-flight replacement (new target wins, the machine never + continues to the old one), unknown machine/target + self-travel + no-ops (rule 7 warn-once paths), disabled-tree quiet degrade + + re-enable resume, and the deleted-link cancel (the machine must + stay put for >2 running loops). Docs: this section, the + `--test-anim-travel` flag help + listener header (tree layout and + all 11 phases) and editScene/AGENTS.md (rule-7 link loss, the + coverage matrix and the registry auto-save caveat for runtime + probes). Verified 2026-10-28: `animation_travel_test` 11/11; + `--test-anim-travel` PASS (16 assertions across 11 phases, ~9 s); + battery switch/vehicle-drive/taxi/parking PASS exit 0; ctest 6/6. + #### Delivery order 1. 6.7.1 data model (runtime can already read links). @@ -4495,6 +4526,8 @@ feet sliding. 5. 6.7.5 final coverage + docs. 6. 6.7.6 per-link wait modes + the first-hop escape hatch (stock trees immediate). +7. 6.7.7 review hardening: rule-7 link-loss cancel + the expanded + test matrix. #### Explicit non-goals (for later phases) @@ -4555,6 +4588,7 @@ taxi-specific): - Headless: multi-floor grid with a stairwell — character walks from ground floor to the top floor (physics), navmesh query finds a ground->top path; grids without stair flags are unchanged. +- from DSL and scene components point of view, the easiest way to create stairwell would be to create a room with type of stairwell. However sometimes for lobby rooms we want a large stair which leads to next floor, like in some hotels, theatres, administrative buildings... Needs research. ### Phase 8 — Enterable buildings (on-the-fly interiors) diff --git a/src/features/editScene/demos/demo-taxi/demo_main.cpp b/src/features/editScene/demos/demo-taxi/demo_main.cpp index 009f47d..6bb7482 100644 --- a/src/features/editScene/demos/demo-taxi/demo_main.cpp +++ b/src/features/editScene/demos/demo-taxi/demo_main.cpp @@ -7472,14 +7472,17 @@ struct DriveBenchmarkListener : public Ogre::FrameListener, * +- state "running" -> running * +- state "stand" -> nested stateMachine "inner" * (startState "idle-soft", endState "walk-slow"; - * children run-fast, walk-slow, idle-soft; - * links idle-soft->walk-slow, walk-slow->run-fast) + * children run-fast, walk-slow, idle-soft plus the + * wait-mode states wm-a/wm-b/wm-c; + * links idle-soft->walk-slow, walk-slow->run-fast, + * idle-soft->wm-a IMMEDIATE, wm-a->wm-b, wm-b->wm-c, + * wm-c->wm-a - all atEnd unless noted) * * crouch and stand carry no connect links, so they are reachable * only through the rule-2 degrade or a plain setState. * - * Phases (assert ORDER, never exact frames; each hop waits for the - * current clip's loop boundary, so timings follow the clip lengths): + * Phases (assert ORDER, never exact frames; each atEnd hop waits for + * the current clip's loop boundary, so timings follow clip lengths): * 0. boot: locomotion enters the startState "idle", not the first * child "crouch". * 1. travelTo("running") from idle: still "idle" immediately (the @@ -7496,12 +7499,28 @@ struct DriveBenchmarkListener : public Ogre::FrameListener, * endState (still "idle-soft" a few frames after the switch, * then "walk-slow"); re-activating "stand" re-enters * "idle-soft". + * 5. wait modes: the immediate idle-soft->wm-a hop lands within a + * few frames; the atEnd wm-a->wm-b hop then waits out the idle + * loop (hundreds of frames, never early). + * 6. forced first hop: travelTo(..., immediateFirstHop=true) lands + * wm-c at once over an atEnd link, and the following hop still + * waits for its clip (the flag is consumed). + * 7. replacement: travelTo("wm-c") re-issued as travelTo("wm-b") + * mid-wait settles at "wm-b" and never continues to "wm-c". + * 8. robust targets: unknown machine/state and self-travel leave + * the machine untouched. + * 9. disabled tree: travelTo is a quiet no-op; re-enabling resumes + * and the next request travels normally. + * 10. link loss: deleting the pending hop's link from the registry + * mid-travel cancels it - the machine must NOT hop along the + * deleted link. */ struct AnimTravelTestListener : public Ogre::FrameListener { EditorApp *app; int frame = 0; /* 0 spawn+boot, 1 travel, 2 cancel, 3 degrade, 4 nested, - * 5 wait modes, 6 forced first hop, 7 done */ + * 5 wait modes, 6 forced first hop, 7 replacement, 8 robust + * targets, 9 disabled tree, 10 link loss, 11 done */ int phase = 0; /* Substep within the current phase. */ int step = 0; @@ -7683,7 +7702,7 @@ struct AnimTravelTestListener : public Ogre::FrameListener { bool frameRenderingQueued(const Ogre::FrameEvent &) override { frame++; - if (failed || phase == 7) { + if (failed || phase == 11) { app->getRoot()->queueEndRendering(); return true; } @@ -7714,6 +7733,18 @@ struct AnimTravelTestListener : public Ogre::FrameListener { case 6: phaseForcedFirst(); break; + case 7: + phaseReplace(); + break; + case 8: + phaseRobustTargets(); + break; + case 9: + phaseDisabled(); + break; + case 10: + phaseLinkLoss(); + break; } return true; } @@ -8075,16 +8106,7 @@ struct AnimTravelTestListener : public Ogre::FrameListener { std::cout << "[test] post-force hop waited " << waited << " frames -> wm-a" << std::endl; - AnimationTreeRegistry::getSingleton() - .removeTree("test_travel_nested"); - std::cout << "[test] PASS: animation travel " - "(startState boot, hop-through, " - "cancel, degrade, nested " - "endState hold, wait modes, " - "forced first hop)" - << std::endl; enterStep(7, 0); - app->getRoot()->queueEndRendering(); return; } if (s != "wm-c") { @@ -8097,6 +8119,258 @@ struct AnimTravelTestListener : public Ogre::FrameListener { return; } } + + /* Rule 4 replacement: re-issuing travelTo with a DIFFERENT + * target while a travel waits must replace the pending path, + * not queue behind it. The machine must settle at the NEW + * target and never continue to the old one. */ + void phaseReplace() + { + AnimationTreeSystem *animSys = app->getAnimationTreeSystem(); + Ogre::String s = stateOf("inner"); + if (step == 0) { + if (s != "wm-a") { + fail("replacement expected 'wm-a', got '" + + s + "'"); + return; + } + /* Long atEnd wait ahead (wm-a plays the idle + * clip), plenty of time to re-issue. */ + animSys->travelTo(character, "inner", "wm-c"); + enterStep(7, 1); + return; + } + if (step == 1) { + animSys->travelTo(character, "inner", "wm-b"); + if (s != "wm-a") { + fail("replacement switched early (got '" + + s + "')"); + return; + } + enterStep(7, 2); + return; + } + if (step == 2) { + if (s == "wm-c") { + fail("replaced travel continued to the old " + "target"); + return; + } + if (s == "wm-b") { + std::cout << "[test] replaced travel arrived " + "at the new target 'wm-b'" + << std::endl; + enterStep(7, 3); + return; + } + if (s != "wm-a") { + fail("unexpected state '" + s + + "' during replacement"); + return; + } + if (stepExpired()) + fail("replaced travel never arrived"); + return; + } + if (step == 3) { + /* wm-b plays walking (~75 frames per loop): a + * stale pending to wm-c would hop within one + * loop. Hold 90 frames to prove it settled. */ + if (s != "wm-b") { + fail("stale travel continued past the " + "replacement target (state '" + s + "')"); + return; + } + if (frame - stepFrame >= 90) { + enterStep(8, 0); + } + return; + } + } + + /* Rule 7 robust targets: unknown machines/states warn once + * and change nothing; travelTo to the CURRENT state is a + * no-op. */ + void phaseRobustTargets() + { + AnimationTreeSystem *animSys = app->getAnimationTreeSystem(); + Ogre::String s = stateOf("inner"); + if (step == 0) { + if (s != "wm-b") { + fail("robust-target phase expected 'wm-b', " + "got '" + s + "'"); + return; + } + for (int i = 0; i < 2; ++i) { + animSys->travelTo(character, "no-such-machine", + "idle"); + animSys->travelTo(character, "inner", + "no-such-state"); + } + if (stateOf("inner") != "wm-b" || + stateOf("locomotion") != "stand") { + fail("unknown machine/target changed state"); + return; + } + /* Travel to the state we are already in: a + * no-op, never a restart or a hop. */ + animSys->travelTo(character, "inner", "wm-b"); + if (stateOf("inner") != "wm-b") { + fail("travelTo(current) changed state"); + return; + } + std::cout << "[test] unknown machine/target and " + "self-travel left the machine alone" + << std::endl; + enterStep(8, 1); + return; + } + if (step == 1) { + if (frame - stepFrame < 10) + return; + if (s != "wm-b") { + fail("state drifted after robust-target " + "calls ('" + s + "')"); + return; + } + enterStep(9, 0); + return; + } + } + + /* A disabled tree degrades quietly: travelTo is a no-op and + * re-enabling resumes without a surprise hop. */ + void phaseDisabled() + { + AnimationTreeSystem *animSys = app->getAnimationTreeSystem(); + Ogre::String s = stateOf("inner"); + if (step == 0) { + auto &at = character.get_mut(); + at.enabled = false; + animSys->travelTo(character, "inner", "wm-c"); + enterStep(9, 1); + return; + } + if (step == 1) { + if (frame - stepFrame < 30) + return; + if (s != "wm-b") { + fail("travelTo ran on a disabled tree ('" + + s + "')"); + return; + } + auto &at = character.get_mut(); + at.enabled = true; + std::cout << "[test] disabled tree ignored travelTo" + << std::endl; + enterStep(9, 2); + return; + } + if (step == 2) { + if (s == "wm-c") { + std::cout << "[test] re-enabled tree travelled " + "on the next request" + << std::endl; + enterStep(10, 0); + return; + } + if (s != "wm-b") { + fail("unexpected state after re-enable ('" + + s + "')"); + return; + } + animSys->travelTo(character, "inner", "wm-c"); + if (stepExpired()) + fail("re-enabled tree never travelled"); + return; + } + } + + /* Rule 7 link loss: deleting the hop's connect link from the + * registry mid-travel must cancel the travel (one warning), + * never hop along the deleted link. */ + static bool eraseLink(AnimationTreeNode &node, const char *machine, + const char *from, const char *to) + { + if (node.type == "stateMachine" && node.name == machine) { + for (size_t i = 0; i < node.connectLinks.size(); ++i) { + if (node.connectLinks[i].from == from && + node.connectLinks[i].to == to) { + node.connectLinks.erase( + node.connectLinks.begin() + i); + return true; + } + } + return false; + } + for (auto &child : node.children) + if (eraseLink(child, machine, from, to)) + return true; + return false; + } + + void phaseLinkLoss() + { + AnimationTreeSystem *animSys = app->getAnimationTreeSystem(); + Ogre::String s = stateOf("inner"); + if (step == 0) { + if (s != "wm-c") { + fail("link-loss phase expected 'wm-c', got '" + + s + "'"); + return; + } + animSys->travelTo(character, "inner", "wm-a"); + enterStep(10, 1); + return; + } + if (step == 1) { + auto *def = AnimationTreeRegistry::getSingleton() + .findTree("test_travel_nested"); + if (!def) { + fail("synthetic tree vanished"); + return; + } + if (!eraseLink(def->root, "inner", "wm-c", "wm-a")) { + fail("could not find the link to delete"); + return; + } + AnimationTreeRegistry::getSingleton().markModified( + "test_travel_nested"); + enterStep(10, 2); + return; + } + if (step == 2) { + /* The running loop is ~50 frames: without the + * cancel the hop to wm-a fires well inside this + * window. */ + if (s == "wm-a") { + fail("travel hopped along a deleted link"); + return; + } + if (s != "wm-c") { + fail("unexpected state after link loss ('" + + s + "')"); + return; + } + if (frame - stepFrame < 120) + return; + std::cout << "[test] deleted-link travel cancelled, " + "machine stayed at 'wm-c'" + << std::endl; + AnimationTreeRegistry::getSingleton().removeTree( + "test_travel_nested"); + std::cout << "[test] PASS: animation travel " + "(startState boot, hop-through, cancel, " + "degrade, nested endState hold, wait " + "modes, forced first hop, replacement, " + "robust targets, disabled tree, link " + "loss)" + << std::endl; + enterStep(11, 0); + app->getRoot()->queueEndRendering(); + return; + } + } }; /* @@ -8254,15 +8528,19 @@ struct AnimTravelTestListener : public Ogre::FrameListener { * state machine travel, on the default interior * scene: a synthetic registry tree probes the * startState boot marker, travelTo hop-through - * (pending anchor, no insta-switch, each hop + * (pending anchor, no insta-switch, each atEnd hop * completes the current clip, arrival), the setState - * cancel (rule 4), the unreachable-target degrade to - * the plain switch (rule 2) and the nested rule-6 - * contract (latch enters the nested startState, - * leaving the host state holds the deactivation - * until the nested machine travelled to its - * endState, re-activation re-enters the startState); - * exits 0 on PASS. + * cancel and mid-flight replacement (rule 4), the + * unreachable-target degrade to the plain switch + * (rule 2), the nested rule-6 contract (latch enters + * the nested startState, leaving the host state + * holds the deactivation until the nested machine + * travelled to its endState, re-activation re-enters + * the startState), per-link wait modes (immediate + * hop vs atEnd loop wait), the forced-first-hop + * escape hatch, robust unknown machine/target calls, + * the disabled-tree no-op and the deleted-link + * cancel (rule 7); exits 0 on PASS. */ int main(int argc, char *argv[]) { @@ -9087,7 +9365,7 @@ int main(int argc, char *argv[]) app.closeApp(); return 1; } - if (animTravelListener.phase != 7) { + if (animTravelListener.phase != 11) { std::cerr << "[test] FAIL: incomplete (anim " "travel phase " << animTravelListener.phase << ")" diff --git a/src/features/editScene/systems/AnimationTreeSystem.cpp b/src/features/editScene/systems/AnimationTreeSystem.cpp index 6e94c7d..1632aab 100644 --- a/src/features/editScene/systems/AnimationTreeSystem.cpp +++ b/src/features/editScene/systems/AnimationTreeSystem.cpp @@ -1301,21 +1301,31 @@ void AnimationTreeSystem::processTravelCheck( /* The hop's link decides when it may fire: the link * current -> next is "immediate" (hop on the next update) or * "atEnd" (wait for the current clip: one-shot end or a - * looping loop boundary, t >= len * 0.99). The one-shot - * travelTo escape hatch forces the first hop regardless of - * the link's mode; it is consumed by the hop that fires. */ + * looping loop boundary, observed within one frame step). The + * one-shot travelTo escape hatch forces the first hop + * regardless of the link's mode; it is consumed by the hop + * that fires. */ const Ogre::String next = itPending->second.path.empty() ? itPending->second.target : itPending->second.path.front(); - bool hopNow = itPending->second.forceFirst; - if (!hopNow) { - for (const auto &link : smNode.connectLinks) { - if (link.from == current && link.to == next) { - hopNow = link.isImmediate(); - break; - } + const AnimationTreeLink *hopLink = nullptr; + for (const auto &link : smNode.connectLinks) { + if (link.from == current && link.to == next) { + hopLink = &link; + break; } } + if (!hopLink) { + /* Rule 7: the link this hop was anchored on vanished + * (the tree was edited mid-travel). Never hop along a + * deleted link — cancel with one log line. */ + state.pendingTravels.erase(itPending); + warnTravelOnce(state, "travelLinkLost", smNode.name, next, + "travel cancelled: connect link vanished " + "mid-travel"); + return; + } + bool hopNow = itPending->second.forceFirst || hopLink->isImmediate(); if (!hopNow) { const AnimationTreeNode *stNode = findStateNode(smNode, current); diff --git a/src/features/editScene/tests/animation_travel_test.cpp b/src/features/editScene/tests/animation_travel_test.cpp index 670929e..968a017 100644 --- a/src/features/editScene/tests/animation_travel_test.cpp +++ b/src/features/editScene/tests/animation_travel_test.cpp @@ -233,6 +233,100 @@ int testValidationWarnings() return 0; } +int testBfsCycleAndSelfLinks() +{ + TEST("BFS cycles, self links and empty endpoints"); + + /* A<->B cycle with no route to C: must terminate empty, not + * loop forever. */ + std::vector cycle = { { "idle", "walking" }, + { "walking", "idle" } }; + if (!paths(cycle, "idle", "running").empty()) + FAIL("a cycle without an exit must yield no path"); + + /* Same cycle with an exit: the path is still found. */ + cycle.push_back({ "walking", "running" }); + std::vector p = paths(cycle, "idle", "running"); + if (p.size() != 2 || p[0] != "walking" || p[1] != "running") + FAIL("BFS must escape a cycle via the exit link"); + + /* Self links never create a hop. */ + std::vector self = { { "idle", "idle" }, + { "idle", "walking" } }; + p = paths(self, "idle", "walking"); + if (p.size() != 1 || p[0] != "walking") + FAIL("self links must be ignored"); + + /* Empty-endpoint links never create a hop either. */ + std::vector empty = { { "", "walking" }, + { "idle", "" } }; + if (!paths(empty, "idle", "walking").empty()) + FAIL("empty-endpoint links must be ignored"); + + PASS(); + return 0; +} + +int testValidationDuplicateModes() +{ + TEST("duplicate links error regardless of waitMode"); + + std::vector states = { "idle", "walking", "running" }; + /* The duplicate check is (from, to)-only: the same edge twice + * with different wait modes is still a duplicate. */ + std::vector dup = { { "idle", "walking" }, + { "idle", "walking", "immediate" } }; + auto issues = validateStateMachine(states, dup, "idle", ""); + bool duplicateError = false; + for (const auto &i : issues) + if (i.error && + i.message.find("duplicate") != std::string::npos) + duplicateError = true; + if (!duplicateError) + FAIL("same edge with a different waitMode must error"); + + PASS(); + return 0; +} + +int testJsonSkipsBadLinks() +{ + TEST("JSON tolerates malformed links and old files"); + + /* Links with a missing/empty endpoint and non-object entries + * are skipped on load; a non-array connectLinks is tolerated. */ + nlohmann::json nodeJson = nlohmann::json::object(); + nodeJson["type"] = "stateMachine"; + nodeJson["name"] = "locomotion"; + nodeJson["connectLinks"] = nlohmann::json::array(); + nodeJson["connectLinks"].push_back( + { { "from", "idle" }, { "to", "walking" } }); + nodeJson["connectLinks"].push_back({ { "from", "walking" } }); + nodeJson["connectLinks"].push_back({ { "to", "running" } }); + nodeJson["connectLinks"].push_back("garbage"); + + AnimationTreeNode back; + animationTreeNodeFromJson(back, nodeJson); + if (back.connectLinks.size() != 1) + FAIL("malformed links must be skipped on load"); + if (back.connectLinks[0].from != "idle" || + back.connectLinks[0].to != "walking") + FAIL("the valid link must survive"); + if (!back.startState.empty() || !back.endState.empty()) + FAIL("missing markers must default to empty (old files)"); + if (back.connectLinks[0].waitMode != "atEnd") + FAIL("missing waitMode must default to atEnd"); + + /* A non-array connectLinks must not throw or fabricate links. */ + nodeJson["connectLinks"] = nlohmann::json::object(); + animationTreeNodeFromJson(back, nodeJson); + if (!back.connectLinks.empty()) + FAIL("non-array connectLinks must yield no links"); + + PASS(); + return 0; +} + int testJsonRoundTrip() { TEST("connectLinks/startState/endState JSON round trip"); @@ -352,8 +446,11 @@ int main() failures += testBfsTieBreaks(); failures += testBfsDegenerate(); failures += testBfsShortestWins(); + failures += testBfsCycleAndSelfLinks(); failures += testValidationErrors(); failures += testValidationWarnings(); + failures += testValidationDuplicateModes(); + failures += testJsonSkipsBadLinks(); failures += testJsonRoundTrip(); failures += testWaitModeRoundTrip(); std::printf("%d/%d tests passed\n", passCount, testCount);