From a03e7e3530c1a99e73666ec74657f6a49ba337ab Mon Sep 17 00:00:00 2001 From: xezon <4720891+xezon@users.noreply.github.com> Date: Tue, 17 Jun 2025 18:41:19 +0200 Subject: [PATCH 1/6] [ZH] Change reference counter implementation for StateMachine to avoid mismatch with Retail --- .../Libraries/Source/WWVegas/WWLib/refcount.h | 47 ++++++++++++++++++- .../GameEngine/Include/Common/StateMachine.h | 14 +++--- 2 files changed, 52 insertions(+), 9 deletions(-) diff --git a/Core/Libraries/Source/WWVegas/WWLib/refcount.h b/Core/Libraries/Source/WWVegas/WWLib/refcount.h index 41a5789a739..d9c42b0f7ca 100644 --- a/Core/Libraries/Source/WWVegas/WWLib/refcount.h +++ b/Core/Libraries/Source/WWVegas/WWLib/refcount.h @@ -249,10 +249,53 @@ class RefCountClass */ static bool Validate_Active_Ref(RefCountClass * obj); -#endif +#endif // NDEBUG }; +// This template class is meant to be used as a class member for compact reference counter placements. +// A 1 byte reference counter can be alright if the counter is not reaching the boundaries. +template +class RefCountValue +{ +public: -#endif + RefCountValue(void) + : NumRefs(1) + { + } + + ~RefCountValue(void) + { + WWASSERT(NumRefs == IntegerType(0)); + } + + void Add_Ref(void) const + { + WWASSERT(NumRefs < IntegerType(0)-1); + ++NumRefs; + } + + // Can pass static function or 'operator delete'. + template + void Release_Ref(void(*deleteFunction)(DeleteType*), ObjectType* objectToDelete) const + { + WWASSERT(NumRefs != IntegerType(0)); + if (--NumRefs == IntegerType(0)) + { + deleteFunction(objectToDelete); + } + } + + IntegerType Num_Refs(void) const + { + return NumRefs; + } + +private: + mutable IntegerType NumRefs; +}; + + +#endif // REFCOUNT_H diff --git a/GeneralsMD/Code/GameEngine/Include/Common/StateMachine.h b/GeneralsMD/Code/GameEngine/Include/Common/StateMachine.h index 7478a97dd62..bedb8a390c5 100644 --- a/GeneralsMD/Code/GameEngine/Include/Common/StateMachine.h +++ b/GeneralsMD/Code/GameEngine/Include/Common/StateMachine.h @@ -239,7 +239,7 @@ inline State::~State() { } /** * A finite state machine. */ -class StateMachine : public MemoryPoolObject, public Snapshot, public RefCountClass +class StateMachine : public MemoryPoolObject, public Snapshot { MEMORY_POOL_GLUE_WITH_USERLOOKUP_CREATE( StateMachine, "StateMachinePool" ); @@ -265,6 +265,10 @@ class StateMachine : public MemoryPoolObject, public Snapshot, public RefCountCl virtual StateReturnType setState( StateID newStateID ); ///< change the current state of the machine (which may cause further state changes, due to onEnter) + void Add_Ref() const { m_refCount.Add_Ref(); } + void Release_Ref() const { m_refCount.Release_Ref(MemoryPoolObject::deleteInstanceInternal, const_cast(this)); } + void Num_Refs() const { m_refCount.Num_Refs(); } + StateID getCurrentStateID() const { return m_currentState ? m_currentState->getID() : INVALID_STATE_ID; } ///< return the id of the current state of the machine Bool isInIdleState() const { return m_currentState ? m_currentState->isIdle() : true; } // stateless things are considered 'idle' Bool isInAttackState() const { return m_currentState ? m_currentState->isAttack() : true; } // stateless things are considered 'idle' @@ -344,12 +348,6 @@ class StateMachine : public MemoryPoolObject, public Snapshot, public RefCountCl virtual void xfer( Xfer *xfer ); virtual void loadPostProcess(); - // RefCountClass interface - virtual void Delete_This() - { - MemoryPoolObject::deleteInstanceInternal(this); - } - protected: /** @@ -386,6 +384,8 @@ class StateMachine : public MemoryPoolObject, public Snapshot, public RefCountCl Bool m_locked; ///< whether this machine is locked or not Bool m_defaultStateInited; ///< if initDefaultState has been called + RefCountValue m_refCount; + #ifdef STATE_MACHINE_DEBUG Bool m_debugOutput; AsciiString m_name; ///< Human readable name of this state - for debugging. jba. From 3843801c139bd4648b4e27d113916f60ca34b934 Mon Sep 17 00:00:00 2001 From: xezon <4720891+xezon@users.noreply.github.com> Date: Tue, 17 Jun 2025 18:47:48 +0200 Subject: [PATCH 2/6] Simplify constness for caller of RefCountValue::Release_Ref --- Core/Libraries/Source/WWVegas/WWLib/refcount.h | 4 ++-- GeneralsMD/Code/GameEngine/Include/Common/StateMachine.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Core/Libraries/Source/WWVegas/WWLib/refcount.h b/Core/Libraries/Source/WWVegas/WWLib/refcount.h index d9c42b0f7ca..28ab38e8dce 100644 --- a/Core/Libraries/Source/WWVegas/WWLib/refcount.h +++ b/Core/Libraries/Source/WWVegas/WWLib/refcount.h @@ -279,12 +279,12 @@ class RefCountValue // Can pass static function or 'operator delete'. template - void Release_Ref(void(*deleteFunction)(DeleteType*), ObjectType* objectToDelete) const + void Release_Ref(void(*deleteFunction)(DeleteType*), const ObjectType* objectToDelete) const { WWASSERT(NumRefs != IntegerType(0)); if (--NumRefs == IntegerType(0)) { - deleteFunction(objectToDelete); + deleteFunction(const_cast(objectToDelete)); } } diff --git a/GeneralsMD/Code/GameEngine/Include/Common/StateMachine.h b/GeneralsMD/Code/GameEngine/Include/Common/StateMachine.h index bedb8a390c5..d99abfdfa5b 100644 --- a/GeneralsMD/Code/GameEngine/Include/Common/StateMachine.h +++ b/GeneralsMD/Code/GameEngine/Include/Common/StateMachine.h @@ -266,7 +266,7 @@ class StateMachine : public MemoryPoolObject, public Snapshot virtual StateReturnType setState( StateID newStateID ); ///< change the current state of the machine (which may cause further state changes, due to onEnter) void Add_Ref() const { m_refCount.Add_Ref(); } - void Release_Ref() const { m_refCount.Release_Ref(MemoryPoolObject::deleteInstanceInternal, const_cast(this)); } + void Release_Ref() const { m_refCount.Release_Ref(MemoryPoolObject::deleteInstanceInternal, this); } void Num_Refs() const { m_refCount.Num_Refs(); } StateID getCurrentStateID() const { return m_currentState ? m_currentState->getID() : INVALID_STATE_ID; } ///< return the id of the current state of the machine From 1fe749e107635ba15fbf9826b22b660782ba881b Mon Sep 17 00:00:00 2001 From: xezon <4720891+xezon@users.noreply.github.com> Date: Tue, 17 Jun 2025 19:17:06 +0200 Subject: [PATCH 3/6] Fix VC6 compile error --- Core/Libraries/Source/WWVegas/WWLib/refcount.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Core/Libraries/Source/WWVegas/WWLib/refcount.h b/Core/Libraries/Source/WWVegas/WWLib/refcount.h index 28ab38e8dce..f891cf8e540 100644 --- a/Core/Libraries/Source/WWVegas/WWLib/refcount.h +++ b/Core/Libraries/Source/WWVegas/WWLib/refcount.h @@ -277,9 +277,9 @@ class RefCountValue ++NumRefs; } - // Can pass static function or 'operator delete'. - template - void Release_Ref(void(*deleteFunction)(DeleteType*), const ObjectType* objectToDelete) const + // Can pass static function of type void(*)(DeleteType*) or 'operator delete'. + template + void Release_Ref(DeleteFunction deleteFunction, const ObjectType* objectToDelete) const { WWASSERT(NumRefs != IntegerType(0)); if (--NumRefs == IntegerType(0)) From d4459c9f6c9914d52d39ff0ddf96767a354b7b70 Mon Sep 17 00:00:00 2001 From: xezon <4720891+xezon@users.noreply.github.com> Date: Tue, 17 Jun 2025 19:39:13 +0200 Subject: [PATCH 4/6] Fix wrong assert condition --- Core/Libraries/Source/WWVegas/WWLib/refcount.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Core/Libraries/Source/WWVegas/WWLib/refcount.h b/Core/Libraries/Source/WWVegas/WWLib/refcount.h index f891cf8e540..21d8f8c2e35 100644 --- a/Core/Libraries/Source/WWVegas/WWLib/refcount.h +++ b/Core/Libraries/Source/WWVegas/WWLib/refcount.h @@ -273,7 +273,7 @@ class RefCountValue void Add_Ref(void) const { - WWASSERT(NumRefs < IntegerType(0)-1); + WWASSERT(NumRefs != ~IntegerType(0)); ++NumRefs; } @@ -294,6 +294,7 @@ class RefCountValue } private: + mutable IntegerType NumRefs; }; From 6e69c4f21b66958632a23ecc116e88e4189070e5 Mon Sep 17 00:00:00 2001 From: xezon <4720891+xezon@users.noreply.github.com> Date: Tue, 17 Jun 2025 19:41:27 +0200 Subject: [PATCH 5/6] Replicate in Generals --- .../Code/GameEngine/Include/Common/StateMachine.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Generals/Code/GameEngine/Include/Common/StateMachine.h b/Generals/Code/GameEngine/Include/Common/StateMachine.h index 3d1dd9e5521..534f9c12a92 100644 --- a/Generals/Code/GameEngine/Include/Common/StateMachine.h +++ b/Generals/Code/GameEngine/Include/Common/StateMachine.h @@ -238,7 +238,7 @@ inline State::~State() { } /** * A finite state machine. */ -class StateMachine : public MemoryPoolObject, public Snapshot, public RefCountClass +class StateMachine : public MemoryPoolObject, public Snapshot { MEMORY_POOL_GLUE_WITH_USERLOOKUP_CREATE( StateMachine, "StateMachinePool" ); @@ -264,6 +264,10 @@ class StateMachine : public MemoryPoolObject, public Snapshot, public RefCountCl virtual StateReturnType setState( StateID newStateID ); ///< change the current state of the machine (which may cause further state changes, due to onEnter) + void Add_Ref() const { m_refCount.Add_Ref(); } + void Release_Ref() const { m_refCount.Release_Ref(MemoryPoolObject::deleteInstanceInternal, this); } + void Num_Refs() const { m_refCount.Num_Refs(); } + StateID getCurrentStateID() const { return m_currentState ? m_currentState->getID() : INVALID_STATE_ID; } ///< return the id of the current state of the machine Bool isInIdleState() const { return m_currentState ? m_currentState->isIdle() : true; } // stateless things are considered 'idle' Bool isInAttackState() const { return m_currentState ? m_currentState->isAttack() : true; } // stateless things are considered 'idle' @@ -342,12 +346,6 @@ class StateMachine : public MemoryPoolObject, public Snapshot, public RefCountCl virtual void xfer( Xfer *xfer ); virtual void loadPostProcess(); - // RefCountClass interface - virtual void Delete_This() - { - MemoryPoolObject::deleteInstanceInternal(this); - } - protected: /** @@ -384,6 +382,8 @@ class StateMachine : public MemoryPoolObject, public Snapshot, public RefCountCl Bool m_locked; ///< whether this machine is locked or not Bool m_defaultStateInited; ///< if initDefaultState has been called + RefCountValue m_refCount; + #ifdef STATE_MACHINE_DEBUG Bool m_debugOutput; AsciiString m_name; ///< Human readable name of this state - for debugging. jba. From 1885c3df8d5bf6af5da9fdab14567dfa457cfe9f Mon Sep 17 00:00:00 2001 From: xezon <4720891+xezon@users.noreply.github.com> Date: Tue, 17 Jun 2025 23:27:48 +0200 Subject: [PATCH 6/6] Add a bunch of comments --- .../Libraries/Source/WWVegas/WWLib/refcount.h | 27 ++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/Core/Libraries/Source/WWVegas/WWLib/refcount.h b/Core/Libraries/Source/WWVegas/WWLib/refcount.h index 21d8f8c2e35..c7d1a702936 100644 --- a/Core/Libraries/Source/WWVegas/WWLib/refcount.h +++ b/Core/Libraries/Source/WWVegas/WWLib/refcount.h @@ -103,6 +103,10 @@ struct ActiveRefStruct typedef DataNode RefCountNodeClass; typedef List RefCountListClass; +/* +** Note that Add_Ref and Release_Ref are always const, because copying, destroying and reference +** counting const objects is meant to work. +*/ class RefCountClass { public: @@ -254,8 +258,13 @@ class RefCountClass }; -// This template class is meant to be used as a class member for compact reference counter placements. -// A 1 byte reference counter can be alright if the counter is not reaching the boundaries. +/* +** This template class is meant to be used as a class member for compact reference counter placements. +** A 1 byte reference counter can be alright if the counter is not reaching the value limits. +* +** Note that Add_Ref and Release_Ref are always const, because copying, destroying and reference +** counting const objects is meant to work. +*/ template class RefCountValue { @@ -271,13 +280,22 @@ class RefCountValue WWASSERT(NumRefs == IntegerType(0)); } + /* + ** Add_Ref, call this function if you are going to keep a pointer to this object. + */ void Add_Ref(void) const { WWASSERT(NumRefs != ~IntegerType(0)); ++NumRefs; } - // Can pass static function of type void(*)(DeleteType*) or 'operator delete'. + /* + ** Release_Ref, call this function when you no longer need the pointer to this object. + ** You can pass a static function of type void(*)(DeleteType*) or 'operator delete'. + ** + ** Note that this function takes a const ObjectType*, because this function is expected + ** to be called from within a const function as well. + */ template void Release_Ref(DeleteFunction deleteFunction, const ObjectType* objectToDelete) const { @@ -288,6 +306,9 @@ class RefCountValue } } + /* + ** Check the number of references to this object. + */ IntegerType Num_Refs(void) const { return NumRefs;