From 5eecb2b2b0ebc1612221ed0481f6c61ea749ff3a Mon Sep 17 00:00:00 2001 From: John McCardle Date: Sun, 21 Jun 2026 12:11:37 -0400 Subject: [PATCH] Rewrite stale Animation-ctor unit tests to drawable.animate() test_animation_raii and test_animation_property_locking called the mcrfpy.Animation(...) constructor, which was removed from the module export during the API freeze. The Animation type still exists (returned by drawable.animate()) and every behavior these tests check is intact: - hasValidTarget()/complete()/stop() on the returned handle (weak-target RAII) - conflict_mode 'replace'/'queue'/'error' + invalid-mode ValueError (#120) Ported both to drawable.animate(prop, target, seconds, easing, conflict_mode=). This file is the suite's only conflict_mode coverage, so it was rewritten rather than deleted. Durations converted ms -> s. Suite now 297/297. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01KnywUddaFRhkxo5kijxJnv --- ROADMAP.md | 1 + tests/unit/test_animation_property_locking.py | 78 +++++++------------ tests/unit/test_animation_raii.py | 28 +++---- 3 files changed, 45 insertions(+), 62 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 94ca51f..1ed0c1e 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -57,6 +57,7 @@ The active tier1 queue is empty. The last three findings (#309 Caption float→u - **Phase 5.3** -- documentation regenerated; `tools/generate_stubs_v2.py` rewritten as introspection-based so it can no longer drift from the C++ source. ### Recently Shipped (June 2026) +- **#320** -- `Caption` constructor positional signature now matches its frozen docstring. The docstring advertised `Caption(pos, font, text, ...)` (parallel to `Sprite`/`Entity`, whose 2nd positional is the resource), but the implementation laid its two positional slots out as `(pos, text)` with `font` keyword-only, so `Caption((x,y), None, "text")` raised `TypeError`. Fixed `UICaption::init` to `(pos, font, text)` positional-or-keyword. Audited zero live callers of the old `(pos, text)` 2-positional form. Also rewrote two stale unit tests (`test_animation_raii`, `test_animation_property_locking`) that called the removed `mcrfpy.Animation(...)` constructor to use `drawable.animate(...)` -- preserving the suite's only `conflict_mode` (#120) coverage and the weak-target RAII checks. Suite now 297/297. - **#317 / #318 / #319** -- The three code-level bugs surfaced by the #314 docstring-accuracy verify pass, fixed together. #317: `automation.scroll()` dropped the x of its position argument (the scroll delta now has its own `injectMouseEvent` parameter, so the real x/y is forwarded). #318: `GridView.texture` always returned `None` (a TODO stub) -- it now returns a `Texture` wrapper (and since `mcrfpy.Grid`/`mcrfpy.GridView` are one type post-#252, both names benefit). #319: `Entity.visible_entities(radius=None)` raised `TypeError` (the `i` format code rejects `None`) -- radius is now parsed as an object so `None`/omitted/`-1` mean "grid default". Regression tests for each; api-surface snapshot re-baselined and docs/stubs regenerated. - **#316** -- Sparse (windowed) perspective writeback in `UIEntity::updateVisibility`. The demote+promote passes are now clipped to an AABB sized to `fov_radius` (with a `prev_fov` window cache so a moving entity leaves no trailing "ghost vision"), replacing two full-`W*H` walks per entity. The Phase 5.2 benchmark's flat ~25-36 ms/entity writeback overhead on a 1000x1000 grid collapses to single-digit microseconds (384x-6577x on the cheap algorithms; lost in timing noise on the rest). Adversarial verify caught a regression the happy-path test missed -- externally-assigned maps (the documented `from_bytes` load/resume path) need a one-shot full demote (`perspective_full_demote_pending`) since `prev_fov` only bounds engine-promoted cells; fixed and locked with a 7-section regression test. - **#313** -- `UIEntity::grid` migrated from `shared_ptr` to `shared_ptr` (post-#252 refactor cleanup), adding a new public `entity.texture` read/write property. Merged to master. diff --git a/tests/unit/test_animation_property_locking.py b/tests/unit/test_animation_property_locking.py index ea9e000..6fe2f6a 100644 --- a/tests/unit/test_animation_property_locking.py +++ b/tests/unit/test_animation_property_locking.py @@ -2,6 +2,13 @@ """ Test Animation Property Locking (#120) Verifies that multiple animations on the same property are handled correctly. + +API note: the standalone ``mcrfpy.Animation(...)`` constructor was removed during +the API freeze. Animations are now created via ``drawable.animate(...)``, which +creates+starts the animation and returns the Animation handle. The conflict_mode +semantics ('replace'/'queue'/'error') are unchanged -- they are now a keyword +argument on ``animate()`` instead of on ``Animation.start()``. This file is the +only coverage of conflict_mode in the suite. """ import mcrfpy @@ -34,17 +41,11 @@ def test_1_replace_mode_default(): frame = mcrfpy.Frame(pos=(100, 100), size=(100, 100)) ui.append(frame) - # Start first animation - anim1 = mcrfpy.Animation("x", 500.0, 2.0, "linear") - anim1.start(frame) # Default is replace mode + # Start first animation (default conflict_mode is 'replace') + frame.animate("x", 500.0, 2.0, "linear") - # Immediately start second animation on same property - anim2 = mcrfpy.Animation("x", 200.0, 1.0, "linear") - anim2.start(frame) # Should replace anim1 - - # anim1 should have been completed (jumped to final value) - # and anim2 should now be active - # The frame should be at x=500 (anim1's final value) then animating to 200 + # Immediately start second animation on same property -> replaces first + frame.animate("x", 200.0, 1.0, "linear") # If we got here without error, replace worked test_result("Replace mode (default)", True) @@ -59,11 +60,8 @@ def test_2_replace_mode_explicit(): frame = mcrfpy.Frame(pos=(100, 100), size=(100, 100)) ui.append(frame) - anim1 = mcrfpy.Animation("x", 500.0, 2.0, "linear") - anim1.start(frame, conflict_mode="replace") - - anim2 = mcrfpy.Animation("x", 200.0, 1.0, "linear") - anim2.start(frame, conflict_mode="replace") + frame.animate("x", 500.0, 2.0, "linear", conflict_mode="replace") + frame.animate("x", 200.0, 1.0, "linear", conflict_mode="replace") test_result("Replace mode (explicit)", True) except Exception as e: @@ -78,15 +76,12 @@ def test_3_queue_mode(): ui.append(frame) # Start first animation (short duration for test) - anim1 = mcrfpy.Animation("y", 300.0, 0.5, "linear") - anim1.start(frame) + frame.animate("y", 300.0, 0.5, "linear") - # Queue second animation - anim2 = mcrfpy.Animation("y", 100.0, 0.5, "linear") - anim2.start(frame, conflict_mode="queue") + # Queue second animation -> starts after the first completes + frame.animate("y", 100.0, 0.5, "linear", conflict_mode="queue") # Both should be accepted without error - # anim2 will start after anim1 completes test_result("Queue mode", True) except Exception as e: test_result("Queue mode", False, str(e)) @@ -99,13 +94,11 @@ def test_4_error_mode(): frame = mcrfpy.Frame(pos=(100, 100), size=(100, 100)) ui.append(frame) - anim1 = mcrfpy.Animation("w", 200.0, 2.0, "linear") - anim1.start(frame) + frame.animate("w", 200.0, 2.0, "linear") # Try to start second animation with error mode - anim2 = mcrfpy.Animation("w", 300.0, 1.0, "linear") try: - anim2.start(frame, conflict_mode="error") + frame.animate("w", 300.0, 1.0, "linear", conflict_mode="error") test_result("Error mode", False, "Expected RuntimeError but none was raised") except RuntimeError as e: # This is expected! @@ -124,9 +117,8 @@ def test_5_invalid_conflict_mode(): frame = mcrfpy.Frame(pos=(100, 100), size=(100, 100)) ui.append(frame) - anim = mcrfpy.Animation("h", 200.0, 1.0, "linear") try: - anim.start(frame, conflict_mode="invalid_mode") + frame.animate("h", 200.0, 1.0, "linear", conflict_mode="invalid_mode") test_result("Invalid conflict_mode", False, "Expected ValueError but none raised") except ValueError as e: if "invalid" in str(e).lower(): @@ -144,14 +136,10 @@ def test_6_different_properties_no_conflict(): frame = mcrfpy.Frame(pos=(100, 100), size=(100, 100)) ui.append(frame) - # Animate different properties - should not conflict - anim_x = mcrfpy.Animation("x", 500.0, 1.0, "linear") - anim_y = mcrfpy.Animation("y", 500.0, 1.0, "linear") - anim_w = mcrfpy.Animation("w", 200.0, 1.0, "linear") - - anim_x.start(frame, conflict_mode="error") - anim_y.start(frame, conflict_mode="error") - anim_w.start(frame, conflict_mode="error") + # Animate different properties with error mode - should not conflict + frame.animate("x", 500.0, 1.0, "linear", conflict_mode="error") + frame.animate("y", 500.0, 1.0, "linear", conflict_mode="error") + frame.animate("w", 200.0, 1.0, "linear", conflict_mode="error") # All should succeed without error since they're different properties test_result("Different properties no conflict", True) @@ -171,11 +159,8 @@ def test_7_different_targets_no_conflict(): ui.append(frame2) # Same property, different targets - should not conflict - anim1 = mcrfpy.Animation("x", 500.0, 1.0, "linear") - anim2 = mcrfpy.Animation("x", 600.0, 1.0, "linear") - - anim1.start(frame1, conflict_mode="error") - anim2.start(frame2, conflict_mode="error") + frame1.animate("x", 500.0, 1.0, "linear", conflict_mode="error") + frame2.animate("x", 600.0, 1.0, "linear", conflict_mode="error") test_result("Different targets no conflict", True) except RuntimeError as e: @@ -191,16 +176,13 @@ def test_8_replace_completes_old(): frame = mcrfpy.Frame(pos=(0, 0), size=(100, 100)) ui.append(frame) - # Start animation to move x to 500 - anim1 = mcrfpy.Animation("x", 500.0, 10.0, "linear") # Long duration - anim1.start(frame) + # Start animation to move x to 500 (long duration) + frame.animate("x", 500.0, 10.0, "linear") - # Immediately replace - should complete anim1 (jump to 500) - anim2 = mcrfpy.Animation("x", 200.0, 1.0, "linear") - anim2.start(frame, conflict_mode="replace") + # Immediately replace - should complete the old animation (jump to 500) + frame.animate("x", 200.0, 1.0, "linear", conflict_mode="replace") - # Frame should now be at x=500 (anim1's final) and animating to 200 - # Due to immediate completion, x should equal 500 right now + # Due to immediate completion of the replaced animation, x should be 500 now if frame.x == 500.0: test_result("Replace completes old animation", True) else: diff --git a/tests/unit/test_animation_raii.py b/tests/unit/test_animation_raii.py index d1ae292..315c674 100644 --- a/tests/unit/test_animation_raii.py +++ b/tests/unit/test_animation_raii.py @@ -3,6 +3,12 @@ Test the RAII AnimationManager implementation. This verifies that weak_ptr properly handles all crash scenarios. Uses mcrfpy.step() for synchronous test execution. + +API note: the standalone ``mcrfpy.Animation(...)`` constructor was removed during +the API freeze. Animations are created via ``drawable.animate(prop, target, +duration_seconds, easing)``, which returns the Animation handle. The handle still +exposes ``hasValidTarget()``, ``complete()``, and ``stop()`` -- the weak_ptr +target-lifetime safety this suite checks is unchanged. (Durations are in seconds.) """ import mcrfpy @@ -48,8 +54,7 @@ try: frame = mcrfpy.Frame(pos=(100, 100), size=(100, 100)) ui.append(frame) - anim = mcrfpy.Animation("x", 200.0, 1000, "linear") - anim.start(frame) + anim = frame.animate("x", 200.0, 1.0, "linear") if hasattr(anim, 'hasValidTarget'): valid = anim.hasValidTarget() @@ -59,13 +64,12 @@ try: except Exception as e: test_result("Basic animation", False, str(e)) -# Test 2: Remove animated object - shared_ptr stays alive while Python ref exists +# Test 2: Remove animated object - target invalid once last shared_ptr drops try: frame = mcrfpy.Frame(pos=(100, 100), size=(100, 100)) ui.append(frame) - anim = mcrfpy.Animation("x", 500.0, 2000, "easeInOut") - anim.start(frame) + anim = frame.animate("x", 500.0, 2.0, "easeInOut") ui.remove(frame) # Note: frame still holds a shared_ptr reference, so target is still valid @@ -85,8 +89,7 @@ try: frame = mcrfpy.Frame(pos=(100, 100), size=(100, 100)) ui.append(frame) - anim = mcrfpy.Animation("x", 500.0, 2000, "linear") - anim.start(frame) + anim = frame.animate("x", 500.0, 2.0, "linear") if hasattr(anim, 'complete'): anim.complete() @@ -96,14 +99,13 @@ try: except Exception as e: test_result("Animation complete method", False, str(e)) -# Test 4: Multiple animations rapidly +# Test 4: Multiple animations rapidly (each replaces the prior on 'x') try: frame = mcrfpy.Frame(pos=(200, 200), size=(100, 100)) ui.append(frame) for i in range(10): - anim = mcrfpy.Animation("x", 300.0 + i * 10, 1000, "linear") - anim.start(frame) + frame.animate("x", 300.0 + i * 10, 1.0, "linear") test_result("Multiple animations rapidly", True) except Exception as e: @@ -116,8 +118,7 @@ try: for i in range(5): frame = mcrfpy.Frame(pos=(50 * i, 100), size=(40, 40)) ui.append(frame) - anim = mcrfpy.Animation("y", 300.0, 2000, "easeOutBounce") - anim.start(frame) + frame.animate("y", 300.0, 2.0, "easeOutBounce") test2.activate() mcrfpy.step(0.1) @@ -132,8 +133,7 @@ except Exception as e: try: frame = mcrfpy.Frame(pos=(100, 100), size=(100, 100)) ui.append(frame) - anim = mcrfpy.Animation("w", 200.0, 1500, "easeInOutCubic") - anim.start(frame) + anim = frame.animate("w", 200.0, 1.5, "easeInOutCubic") # Clear all UI except background - iterate in reverse for i in range(len(ui) - 1, 0, -1):