From a7881a0eea83950142ae26fe97a95f50403ea25d Mon Sep 17 00:00:00 2001 From: Serhii Snitsaruk Date: Sun, 21 Jul 2024 13:28:13 +0200 Subject: [PATCH] Rework HSM transitions bookkeeping Fixes a potential rare hash collision situation, and allows additional attributes to be stored with the transition data in future. --- hsm/limbo_hsm.cpp | 32 +++++++++++++++++++++++--------- hsm/limbo_hsm.h | 35 ++++++++++++++++++++++++++--------- 2 files changed, 49 insertions(+), 18 deletions(-) diff --git a/hsm/limbo_hsm.cpp b/hsm/limbo_hsm.cpp index 857f930..76a806f 100644 --- a/hsm/limbo_hsm.cpp +++ b/hsm/limbo_hsm.cpp @@ -104,19 +104,31 @@ void LimboHSM::add_transition(LimboState *p_from_state, LimboState *p_to_state, ERR_FAIL_COND_MSG(p_to_state->get_parent() != this, "LimboHSM: Unable to add a transition to a state that is not an immediate child of mine."); ERR_FAIL_COND_MSG(p_event == StringName(), "LimboHSM: Failed to add transition due to empty event string."); - uint64_t key = _get_transition_key(p_from_state, p_event); - transitions[key] = Object::cast_to(p_to_state); + TransitionKey key = Transition::make_key(p_from_state, p_event); + ERR_FAIL_COND_MSG(transitions.has(key), "LimboHSM: Unable to add another transition with the same event and origin."); + // Note: Explicit casting needed for GDExtension. + transitions[key] = { p_from_state != nullptr ? ObjectID(p_from_state->get_instance_id()) : ObjectID(), ObjectID(p_to_state->get_instance_id()), p_event }; } void LimboHSM::remove_transition(LimboState *p_from_state, const StringName &p_event) { ERR_FAIL_COND_MSG(p_from_state != nullptr && p_from_state->get_parent() != this, "LimboHSM: Unable to remove a transition from a state that is not an immediate child of mine."); ERR_FAIL_COND_MSG(p_event == StringName(), "LimboHSM: Unable to remove a transition due to empty event string."); - uint64_t key = _get_transition_key(p_from_state, p_event); + TransitionKey key = Transition::make_key(p_from_state, p_event); ERR_FAIL_COND_MSG(!transitions.has(key), "LimboHSM: Unable to remove a transition that does not exist."); transitions.erase(key); } +void LimboHSM::get_transition(LimboState *p_from_state, const StringName &p_event, Transition &r_transition) const { + ERR_FAIL_COND_MSG(p_from_state != nullptr && p_from_state->get_parent() != this, "LimboHSM: Unable to get a transition from a state that is not an immediate child of this HSM."); + ERR_FAIL_COND_MSG(p_event == StringName(), "LimboHSM: Unable to get a transition with an empty event string."); + + TransitionKey key = Transition::make_key(p_from_state, p_event); + if (transitions.has(key)) { + r_transition = transitions[key]; + } +} + LimboState *LimboHSM::get_leaf_state() const { LimboHSM *hsm = const_cast(this); while (hsm->active_state != nullptr && hsm->active_state->is_class("LimboHSM")) { @@ -148,16 +160,18 @@ bool LimboHSM::_dispatch(const StringName &p_event, const Variant &p_cargo) { } if (!event_consumed && active_state) { - uint64_t key = _get_transition_key(active_state, p_event); LimboState *to_state = nullptr; - if (transitions.has(key)) { - to_state = transitions[key]; + + Transition transition; + get_transition(active_state, p_event, transition); + if (transition.is_valid()) { + to_state = Object::cast_to(ObjectDB::get_instance(transition.to_state)); } if (to_state == nullptr) { // Get ANYSTATE transition. - key = _get_transition_key(nullptr, p_event); - if (transitions.has(key)) { - to_state = transitions[key]; + get_transition(nullptr, p_event, transition); + if (transition.is_valid()) { + to_state = Object::cast_to(ObjectDB::get_instance(transition.to_state)); if (to_state == active_state) { // Transitions to self are not allowed with ANYSTATE. to_state = nullptr; diff --git a/hsm/limbo_hsm.h b/hsm/limbo_hsm.h index b046a1f..cfad779 100644 --- a/hsm/limbo_hsm.h +++ b/hsm/limbo_hsm.h @@ -14,6 +14,16 @@ #include "limbo_state.h" +#define TransitionKey Pair + +struct TransitionKeyHasher { + static uint32_t hash(const TransitionKey &P) { + uint64_t h1 = HashMapHasherDefault::hash(P.first); + uint64_t h2 = HashMapHasherDefault::hash(P.second); + return hash_one_uint64((h1 << 32) | h2); + } +}; + class LimboHSM : public LimboState { GDCLASS(LimboHSM, LimboState); @@ -25,22 +35,28 @@ public: }; private: + struct Transition { + ObjectID from_state; + ObjectID to_state; + StringName event; + + inline bool is_valid() const { return to_state != ObjectID(); } + + static _FORCE_INLINE_ TransitionKey make_key(LimboState *p_from_state, const StringName &p_event) { + return TransitionKey( + p_from_state != nullptr ? uint64_t(p_from_state->get_instance_id()) : 0, + p_event); + } + }; + UpdateMode update_mode; LimboState *initial_state; LimboState *active_state; LimboState *previous_active; LimboState *next_active; - HashMap transitions; bool updating = false; - _FORCE_INLINE_ uint64_t _get_transition_key(LimboState *p_from_state, const StringName &p_event) { - uint64_t key = hash_djb2_one_64(Variant::OBJECT); - if (p_from_state != nullptr) { - key = hash_djb2_one_64(hash_one_uint64(hash_make_uint64_t(p_from_state)), key); - } - key = hash_djb2_one_64(p_event.hash(), key); - return key; - } + HashMap transitions; protected: static void _bind_methods(); @@ -75,6 +91,7 @@ public: void add_transition(LimboState *p_from_state, LimboState *p_to_state, const StringName &p_event); void remove_transition(LimboState *p_from_state, const StringName &p_event); + void get_transition(LimboState *p_from_state, const StringName &p_event, Transition &r_transition) const; LimboState *anystate() const { return nullptr; }