check whether state can change before evaluating conditions

this is a regression from the new implementation to use triggers per state machine layer.
Since canChangeState was being called after evaluating the conditions, the trigger would be marked as used but the state wouldn't change.
This makes sure canChangeState is called before.

Diffs=
7f3314f4f9 check whether state can change before evaluating conditions (#8917)

Co-authored-by: hernan <hernan@rive.app>

.rive_head

@@ -1 +1 @@
-d8d42c0f130ec1f7d70a9e4ca3cffb1f6e3b9860
+7f3314f4f98b54580758aa48a435a8a202ed9fcd

include/rive/animation/state_machine_input_instance.hpp

@@ -79,15 +79,22 @@ class Triggerable
 {
 
 public:
-    bool useInLayer(StateMachineLayerInstance* layer) const
+    bool isUsedInLayer(StateMachineLayerInstance* layer) const
+    {
+        auto it = std::find(m_usedLayers.begin(), m_usedLayers.end(), layer);
+        if (it == m_usedLayers.end())
+        {
+            return false;
+        }
+        return true;
+    }
+    void useInLayer(StateMachineLayerInstance* layer) const
     {
         auto it = std::find(m_usedLayers.begin(), m_usedLayers.end(), layer);
         if (it == m_usedLayers.end())
         {
             m_usedLayers.push_back(layer);
-            return true;
         }
-        return false;
     }
 
 protected:

include/rive/animation/transition_trigger_condition.hpp

@@ -9,6 +9,8 @@ class TransitionTriggerCondition : public TransitionTriggerConditionBase
 public:
     bool evaluate(const StateMachineInstance* stateMachineInstance,
                   StateMachineLayerInstance* layerInstance) const override;
+    void useInLayer(const StateMachineInstance* stateMachineInstance,
+                    StateMachineLayerInstance* layerInstance) const;
 
 protected:
     bool validateInputType(const StateMachineInput* input) const override;

src/animation/state_machine_instance.cpp

@@ -217,21 +217,25 @@ class StateMachineLayerInstance
              i++)
         {
             auto transition = stateFrom->transition(i);
-            auto allowed = transition->allowed(stateFromInstance,
-                                               m_stateMachineInstance,
-                                               this);
-            if (allowed == AllowTransition::yes &&
-                canChangeState(transition->stateTo()))
+            if (canChangeState(transition->stateTo()))
             {
-                transition->evaluatedRandomWeight(transition->randomWeight());
-                totalWeight += transition->randomWeight();
-            }
-            else
-            {
-                transition->evaluatedRandomWeight(0);
-                if (allowed == AllowTransition::waitingForExit)
+
+                auto allowed = transition->allowed(stateFromInstance,
+                                                   m_stateMachineInstance,
+                                                   this);
+                if (allowed == AllowTransition::yes)
+                {
+                    transition->evaluatedRandomWeight(
+                        transition->randomWeight());
+                    totalWeight += transition->randomWeight();
+                }
+                else
                 {
-                    m_waitingForExit = true;
+                    transition->evaluatedRandomWeight(0);
+                    if (allowed == AllowTransition::waitingForExit)
+                    {
+                        m_waitingForExit = true;
+                    }
                 }
             }
         }
@@ -270,21 +274,25 @@ class StateMachineLayerInstance
              i++)
         {
             auto transition = stateFrom->transition(i);
-            auto allowed = transition->allowed(stateFromInstance,
-                                               m_stateMachineInstance,
-                                               this);
-            if (allowed == AllowTransition::yes &&
-                canChangeState(transition->stateTo()))
+            if (canChangeState(transition->stateTo()))
             {
-                transition->evaluatedRandomWeight(transition->randomWeight());
-                return transition;
-            }
-            else
-            {
-                transition->evaluatedRandomWeight(0);
-                if (allowed == AllowTransition::waitingForExit)
+
+                auto allowed = transition->allowed(stateFromInstance,
+                                                   m_stateMachineInstance,
+                                                   this);
+                if (allowed == AllowTransition::yes)
                 {
-                    m_waitingForExit = true;
+                    transition->evaluatedRandomWeight(
+                        transition->randomWeight());
+                    return transition;
+                }
+                else
+                {
+                    transition->evaluatedRandomWeight(0);
+                    if (allowed == AllowTransition::waitingForExit)
+                    {
+                        m_waitingForExit = true;
+                    }
                 }
             }
         }

src/animation/state_transition.cpp

@@ -202,6 +202,15 @@ AllowTransition StateTransition::allowed(
             }
         }
     }
+    for (auto condition : m_Conditions)
+    {
+        if (condition->is<TransitionTriggerCondition>())
+        {
+            condition->as<TransitionTriggerCondition>()->useInLayer(
+                stateMachineInstance,
+                layerInstance);
+        }
+    }
     return AllowTransition::yes;
 }
 

src/animation/transition_property_viewmodel_comparator.cpp

@@ -177,8 +177,8 @@ bool TransitionPropertyViewModelComparator::compare(
                     if (source != nullptr &&
                         source->is<ViewModelInstanceTrigger>())
                     {
-                        if (!source->as<ViewModelInstanceTrigger>()->useInLayer(
-                                layerInstance))
+                        if (source->as<ViewModelInstanceTrigger>()
+                                ->isUsedInLayer(layerInstance))
                         {
 
                             return false;

src/animation/transition_trigger_condition.cpp

@@ -26,9 +26,23 @@ bool TransitionTriggerCondition::evaluate(
     }
     auto triggerInput = static_cast<const SMITrigger*>(inputInstance);
 
-    if (triggerInput->m_fired && triggerInput->useInLayer(layerInstance))
+    if (triggerInput->m_fired && !triggerInput->isUsedInLayer(layerInstance))
     {
         return true;
     }
     return false;
 }
+
+void TransitionTriggerCondition::useInLayer(
+    const StateMachineInstance* stateMachineInstance,
+    StateMachineLayerInstance* layerInstance) const
+{
+
+    auto inputInstance = stateMachineInstance->input(inputId());
+    if (inputInstance == nullptr)
+    {
+        return;
+    }
+    auto triggerInput = static_cast<const SMITrigger*>(inputInstance);
+    triggerInput->useInLayer(layerInstance);
+}

tests/unit_tests/runtime/state_machine_test.cpp

@@ -1,5 +1,6 @@
 #include <rive/file.hpp>
 #include <rive/animation/state_machine_bool.hpp>
+#include <rive/animation/state_machine_trigger.hpp>
 #include <rive/animation/state_machine_layer.hpp>
 #include <rive/animation/animation_state.hpp>
 #include <rive/animation/entry_state.hpp>
@@ -423,3 +424,81 @@ TEST_CASE("Transitions with reset applied to them.", "[file]")
 
     delete stateMachineInstance;
 }
+
+TEST_CASE("Triggers will only be used on allowed state changes.", "[file]")
+{
+    auto file = ReadRiveFile("assets/state_machine_triggers.riv");
+
+    auto artboard = file->artboard("main");
+
+    // We empty all factory reset resources to start the test clean
+    rive::AnimationResetFactory::releaseResources();
+    REQUIRE(rive::AnimationResetFactory::resourcesCount() == 0);
+
+    REQUIRE(artboard != nullptr);
+    REQUIRE(artboard->animationCount() == 1);
+    REQUIRE(artboard->stateMachineCount() == 1);
+
+    auto stateMachine = artboard->stateMachine("State Machine 1");
+    REQUIRE(stateMachine->layerCount() == 1);
+    REQUIRE(stateMachine->inputCount() == 1);
+
+    auto abi = artboard->instance();
+    rive::StateMachineInstance* stateMachineInstance =
+        new rive::StateMachineInstance(stateMachine, abi.get());
+    stateMachineInstance->advanceAndApply(0.1f);
+    abi->advance(0.1f);
+
+    auto trigger = stateMachine->input("Trigger 1");
+    REQUIRE(trigger != nullptr);
+    REQUIRE(trigger->is<rive::StateMachineTrigger>());
+
+    auto layer = stateMachine->layer(0);
+    REQUIRE(layer->stateCount() == 5);
+
+    REQUIRE(layer->anyState() != nullptr);
+    REQUIRE(layer->entryState() != nullptr);
+    REQUIRE(layer->exitState() != nullptr);
+
+    rive::AnimationState* animationState1 = nullptr;
+    rive::AnimationState* animationState2 = nullptr;
+
+    int foundAnimationStates = 0;
+    for (int i = 0; i < layer->stateCount(); i++)
+    {
+        auto state = layer->state(i);
+        if (state->is<rive::AnimationState>())
+        {
+            foundAnimationStates++;
+            REQUIRE(state->as<rive::AnimationState>()->animation() != nullptr);
+            if (foundAnimationStates == 1)
+            {
+                animationState1 = state->as<rive::AnimationState>();
+            }
+            else
+            {
+                animationState2 = state->as<rive::AnimationState>();
+            }
+        }
+    }
+    REQUIRE(foundAnimationStates == 2);
+    REQUIRE(animationState1 != nullptr);
+    REQUIRE(animationState2 != nullptr);
+
+    auto triggerInstance = stateMachineInstance->getTrigger("Trigger 1");
+    REQUIRE(triggerInstance != nullptr);
+
+    triggerInstance->fire();
+    stateMachineInstance->advanceAndApply(0.1f);
+    auto currentLayerState = stateMachineInstance->layerState(0);
+
+    REQUIRE(currentLayerState == animationState2);
+
+    triggerInstance->fire();
+    stateMachineInstance->advanceAndApply(0.1f);
+    currentLayerState = stateMachineInstance->layerState(0);
+
+    REQUIRE(currentLayerState == animationState1);
+
+    delete stateMachineInstance;
+}