Skip to content

Commit

Permalink
check whether state can change before evaluating conditions
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
bodymovin and bodymovin committed Jan 24, 2025
1 parent 26dd69f commit 276d1a6
Show file tree
Hide file tree
Showing 9 changed files with 151 additions and 33 deletions.
1 change: 0 additions & 1 deletion .rive_head
Original file line number Diff line number Diff line change
@@ -1 +0,0 @@
24d9162103a73712ba3c80cfc909fee6948a0089
13 changes: 10 additions & 3 deletions include/rive/animation/state_machine_input_instance.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions include/rive/animation/transition_trigger_condition.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
60 changes: 34 additions & 26 deletions src/animation/state_machine_instance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}
}
Expand Down Expand Up @@ -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;
}
}
}
}
Expand Down
9 changes: 9 additions & 0 deletions src/animation/state_transition.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
4 changes: 2 additions & 2 deletions src/animation/transition_property_viewmodel_comparator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
16 changes: 15 additions & 1 deletion src/animation/transition_trigger_condition.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Binary file added tests/unit_tests/assets/state_machine_triggers.riv
Binary file not shown.
79 changes: 79 additions & 0 deletions tests/unit_tests/runtime/state_machine_test.cpp
Original file line number Diff line number Diff line change
@@ -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>
Expand Down Expand Up @@ -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;
}

0 comments on commit 276d1a6

Please sign in to comment.