fix: nested artboards advancing

Attempt nr 2. Resolves: #338 With this change we will only settle if nested artboards (state machines) are also settled. Two tests "broke" as a result of this change. But with this change, these tests behave the same as our other runtimes, meaning it is the expected behavior. This PR went through a few iterations, and this approach seems to be the best trade-off between the desired implementation and nr. of "failing" tests. Diffs= c65e93fee2 fix: nested artboards advancing (#8295) Co-authored-by: Gordon <[email protected]>
rive-app · Oct 14, 2024 · c379190 · c379190
1 parent 492c7ba
commit c379190
Show file tree

Hide file tree

Showing 6 changed files with 47 additions and 5 deletions.
diff --git a/.rive_head b/.rive_head
@@ -1 +1 @@
-0f805654cfd333d978a98a4375a6d99d0d5c8107
+c65e93fee25dd3beea59ff09c5be48a0061c6605
diff --git a/kotlin/src/androidTest/java/app/rive/runtime/kotlin/core/RiveListenerTest.kt b/kotlin/src/androidTest/java/app/rive/runtime/kotlin/core/RiveListenerTest.kt
@@ -287,6 +287,7 @@ class RiveListenerTest {
 
             // animation came to an end inside this time period, this still means no state change
             renderer.advance(1.0f)
+            renderer.advance(0.01f)
             assertEquals(false, view.isPlaying)
             assertEquals(0, observer.states.size)
 

diff --git a/kotlin/src/androidTest/java/app/rive/runtime/kotlin/core/RiveViewStateMachineTest.kt b/kotlin/src/androidTest/java/app/rive/runtime/kotlin/core/RiveViewStateMachineTest.kt
@@ -89,6 +89,7 @@ class RiveViewStateMachineTest {
             assert(mockView.artboardRenderer != null)
             // Let the state machine animation run its course.
             mockView.artboardRenderer!!.advance(1.01f)
+            mockView.artboardRenderer!!.advance(0.0f)
             assert(!mockView.isPlaying)
         }
     }
@@ -113,6 +114,27 @@ class RiveViewStateMachineTest {
         }
     }
 
+    @Test
+    fun nestedStateMachinesContinuePlaying() {
+        UiThreadStatement.runOnUiThread {
+            // The main state machine's controller is not advancing, however, nested artboards
+            // need to continue playing
+            val mView = mockView as TestUtils.MockRiveAnimationView
+            val controller = mView.controller
+            mView.setRiveResource(R.raw.nested_settle)
+            mView.play("State Machine 1", isStateMachine = true)
+            assertEquals(1, controller.stateMachines.size)
+
+            mView.artboardRenderer?.advance(0.5f);
+            assert(mView.isPlaying)
+            mView.artboardRenderer?.advance(0.5f);
+            assert(mView.isPlaying)
+            // The nested artboard should now be settled
+            mView.artboardRenderer?.advance(0.01f);
+            assert(!mView.isPlaying) // Playing is false
+        }
+    }
+
     @Test
     @Ignore("We're not stopping state machines when all layers are stopped atm.")
     fun viewStateMachinesInstancesRemoveOnStop() {

diff --git a/kotlin/src/androidTest/res/raw/nested_settle.riv b/kotlin/src/androidTest/res/raw/nested_settle.riv
diff --git a/kotlin/src/main/cpp/src/bindings/bindings_file.cpp b/kotlin/src/main/cpp/src/bindings/bindings_file.cpp
@@ -39,7 +39,12 @@ extern "C"
     {
         auto file = reinterpret_cast<rive::File*>(ref);
         // Creates a new Artboard instance.
-        return (jlong)file->artboardNamed(JStringToString(env, name)).release();
+        auto artboard = file->artboardNamed(JStringToString(env, name));
+        if (artboard != nullptr)
+        {
+            artboard->advance(0.0);
+        }
+        return (jlong)artboard.release();
     }
 
     JNIEXPORT void JNICALL
@@ -70,7 +75,12 @@ extern "C"
     {
         auto file = reinterpret_cast<rive::File*>(ref);
         // Creates a new Artboard instance.
-        return (jlong)file->artboardAt(index).release();
+        auto artboard = file->artboardAt(index);
+        if (artboard != nullptr)
+        {
+            artboard->advance(0.0);
+        }
+        return (jlong)artboard.release();
     }
 
     JNIEXPORT

diff --git a/kotlin/src/main/java/app/rive/runtime/kotlin/controllers/RiveFileController.kt b/kotlin/src/main/java/app/rive/runtime/kotlin/controllers/RiveFileController.kt
@@ -279,17 +279,26 @@ class RiveFileController(
                     }
                 }
 
+                val stateMachinesToPause = mutableListOf<StateMachineInstance>()
                 stateMachines.forEach { stateMachineInstance ->
                     if (playingStateMachines.contains(stateMachineInstance)) {
                         val stillPlaying =
                             resolveStateMachineAdvance(stateMachineInstance, elapsed)
 
                         if (!stillPlaying) {
-                            pause(stateMachine = stateMachineInstance)
+                            stateMachinesToPause.add(stateMachineInstance)
                         }
                     }
                 }
-                ab.advance(elapsed)
+
+                // Only pause the state machines once the artboard has also settled, as nested
+                // artboards may still be advancing. This is to tie into the current logic of
+                // pausing only when current animations/machines are empty. In the future
+                // this can be simplified.
+                // Resolves: https://github.com/rive-app/rive-android/issues/338
+                if (!ab.advance(elapsed)) {
+                    stateMachinesToPause.forEach { pause(stateMachine = it) }
+                }
                 notifyAdvance(elapsed)
             }
         }