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>
diff --git a/.rive_head b/.rive_head index 62d82cc..e69de29 100644 --- a/.rive_head +++ b/.rive_head
@@ -1 +0,0 @@ -24d9162103a73712ba3c80cfc909fee6948a0089
diff --git a/include/rive/animation/state_machine_input_instance.hpp b/include/rive/animation/state_machine_input_instance.hpp index 6d947ed..23958f2 100644 --- a/include/rive/animation/state_machine_input_instance.hpp +++ b/include/rive/animation/state_machine_input_instance.hpp
@@ -79,15 +79,22 @@ { 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:
diff --git a/include/rive/animation/transition_trigger_condition.hpp b/include/rive/animation/transition_trigger_condition.hpp index f084793..8186434 100644 --- a/include/rive/animation/transition_trigger_condition.hpp +++ b/include/rive/animation/transition_trigger_condition.hpp
@@ -9,6 +9,8 @@ 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;
diff --git a/src/animation/state_machine_instance.cpp b/src/animation/state_machine_instance.cpp index 8d02ee3..114e9b1 100644 --- a/src/animation/state_machine_instance.cpp +++ b/src/animation/state_machine_instance.cpp
@@ -217,21 +217,25 @@ 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) { - m_waitingForExit = true; + transition->evaluatedRandomWeight( + transition->randomWeight()); + totalWeight += transition->randomWeight(); + } + else + { + transition->evaluatedRandomWeight(0); + if (allowed == AllowTransition::waitingForExit) + { + m_waitingForExit = true; + } } } } @@ -270,21 +274,25 @@ 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; + } } } }
diff --git a/src/animation/state_transition.cpp b/src/animation/state_transition.cpp index 59ef929..8ebb202 100644 --- a/src/animation/state_transition.cpp +++ b/src/animation/state_transition.cpp
@@ -202,6 +202,15 @@ } } } + for (auto condition : m_Conditions) + { + if (condition->is<TransitionTriggerCondition>()) + { + condition->as<TransitionTriggerCondition>()->useInLayer( + stateMachineInstance, + layerInstance); + } + } return AllowTransition::yes; }
diff --git a/src/animation/transition_property_viewmodel_comparator.cpp b/src/animation/transition_property_viewmodel_comparator.cpp index ca95feb..a288601 100644 --- a/src/animation/transition_property_viewmodel_comparator.cpp +++ b/src/animation/transition_property_viewmodel_comparator.cpp
@@ -177,8 +177,8 @@ if (source != nullptr && source->is<ViewModelInstanceTrigger>()) { - if (!source->as<ViewModelInstanceTrigger>()->useInLayer( - layerInstance)) + if (source->as<ViewModelInstanceTrigger>() + ->isUsedInLayer(layerInstance)) { return false;
diff --git a/src/animation/transition_trigger_condition.cpp b/src/animation/transition_trigger_condition.cpp index 2d4a570..c60458c 100644 --- a/src/animation/transition_trigger_condition.cpp +++ b/src/animation/transition_trigger_condition.cpp
@@ -26,9 +26,23 @@ } 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); +}
diff --git a/tests/unit_tests/assets/state_machine_triggers.riv b/tests/unit_tests/assets/state_machine_triggers.riv new file mode 100644 index 0000000..42ec964 --- /dev/null +++ b/tests/unit_tests/assets/state_machine_triggers.riv Binary files differ
diff --git a/tests/unit_tests/runtime/state_machine_test.cpp b/tests/unit_tests/runtime/state_machine_test.cpp index 7247ea1..61c7724 100644 --- a/tests/unit_tests/runtime/state_machine_test.cpp +++ b/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 @@ 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; +}