From 14f7a10ae692817174c547a59472dad7c192fb4d Mon Sep 17 00:00:00 2001
From: Boondorl <boondorl@live.com>
Date: Fri, 24 Jan 2025 12:15:48 -0500
Subject: [PATCH] Added global iterator for behaviors

Imported Behaviors to the engine to allow them to properly clean up their level list. Restrict Behaviors from being new'd in ZScript as they need an owner to function.
---
 src/g_levellocals.h                    | 24 +++++++++
 src/p_saveg.cpp                        |  3 +-
 src/playsim/actor.h                    | 18 +++++--
 src/playsim/p_mobj.cpp                 | 71 +++++++++++++++++---------
 src/scripting/backend/codegen_doom.cpp |  8 ++-
 src/scripting/vmiterators.cpp          | 44 ++++++++++++----
 src/serializer_doom.cpp                |  8 +--
 src/serializer_doom.h                  |  3 +-
 wadsrc/static/zscript/actors/actor.zs  |  6 ++-
 9 files changed, 140 insertions(+), 45 deletions(-)

diff --git a/src/g_levellocals.h b/src/g_levellocals.h
index 2002e35b5e..da3bc58c25 100644
--- a/src/g_levellocals.h
+++ b/src/g_levellocals.h
@@ -706,6 +706,7 @@ public:
 	DVisualThinker* VisualThinkerHead = nullptr;
 
 	// links to global game objects
+	TArray<DBehavior*> ActorBehaviors;
 	TArray<TObjPtr<AActor *>> CorpseQueue;
 	TObjPtr<DFraggleThinker *> FraggleScriptThinker = MakeObjPtr<DFraggleThinker*>(nullptr);
 	TObjPtr<DACSThinker*> ACSThinker = MakeObjPtr<DACSThinker*>(nullptr);
@@ -717,6 +718,29 @@ public:
 	//
 	//==========================================================================
 
+	void AddActorBehavior(DBehavior& b)
+	{
+		if (b.Level == nullptr)
+		{
+			b.Level = this;
+			ActorBehaviors.Push(&b);
+		}
+	}
+
+	void RemoveActorBehavior(DBehavior& b)
+	{
+		if (b.Level == this)
+		{
+			b.Level = nullptr;
+			ActorBehaviors.Delete(ActorBehaviors.Find(&b));
+		}
+	}
+
+	//==========================================================================
+	//
+	//
+	//==========================================================================
+
 	bool IsJumpingAllowed() const
 	{
 		if (dmflags & DF_NO_JUMP)
diff --git a/src/p_saveg.cpp b/src/p_saveg.cpp
index ce26ff39c7..d759add727 100644
--- a/src/p_saveg.cpp
+++ b/src/p_saveg.cpp
@@ -991,7 +991,8 @@ void FLevelLocals::Serialize(FSerializer &arc, bool hubload)
 		("automap", automap)
 		("interpolator", interpolator)
 		("frozenstate", frozenstate)
-		("visualthinkerhead", VisualThinkerHead);
+		("visualthinkerhead", VisualThinkerHead)
+		("actorbehaviors", ActorBehaviors);
 
 
 	// Hub transitions must keep the current total time
diff --git a/src/playsim/actor.h b/src/playsim/actor.h
index ffe48fdfd0..b852053736 100644
--- a/src/playsim/actor.h
+++ b/src/playsim/actor.h
@@ -778,6 +778,18 @@ public:
 	void Serialize(FSerializer& arc) override;
 };
 
+class DBehavior : public DObject
+{
+	DECLARE_CLASS(DBehavior, DObject)
+	HAS_OBJECT_POINTERS
+public:
+	TObjPtr<AActor*> Owner;
+	FLevelLocals* Level;
+
+	void Serialize(FSerializer& arc) override;
+	void OnDestroy() override;
+};
+
 const double MinVel = EQUAL_EPSILON;
 
 // Map Object definition.
@@ -1374,7 +1386,7 @@ public:
 	// landing speed from a jump with normal gravity (squats the player's view)
 	// (note: this is put into AActor instead of the PlayerPawn because non-players also use the value)
 	double LandingSpeed;
-	TMap<FName, DObject*> Behaviors;
+	TMap<FName, DBehavior*> Behaviors;
 
 
 	// ThingIDs
@@ -1436,12 +1448,12 @@ public:
 		return GetClass()->FindState(numnames, names, exact);
 	}
 
-	DObject* FindBehavior(const PClass& type) const
+	DBehavior* FindBehavior(const PClass& type) const
 	{
 		auto b = Behaviors.CheckKey(type.TypeName);
 		return b != nullptr && *b != nullptr && !((*b)->ObjectFlags & OF_EuthanizeMe) ? *b : nullptr;
 	}
-	DObject* AddBehavior(PClass& type);
+	DBehavior* AddBehavior(PClass& type);
 	bool RemoveBehavior(const PClass& type);
 	void TickBehaviors();
 	void MoveBehaviors(AActor& from);
diff --git a/src/playsim/p_mobj.cpp b/src/playsim/p_mobj.cpp
index 82f7a181ed..93480d6fa8 100644
--- a/src/playsim/p_mobj.cpp
+++ b/src/playsim/p_mobj.cpp
@@ -180,6 +180,14 @@ IMPLEMENT_POINTERS_START(AActor)
 	IMPLEMENT_POINTER(modelData)
 IMPLEMENT_POINTERS_END
 
+IMPLEMENT_CLASS(DBehavior, false, true)
+IMPLEMENT_POINTERS_START(DBehavior)
+	IMPLEMENT_POINTER(Owner)
+IMPLEMENT_POINTERS_END
+
+DEFINE_FIELD(DBehavior, Owner)
+DEFINE_FIELD(DBehavior, Level)
+
 //==========================================================================
 //
 // Make sure Actors can never have their networking disabled.
@@ -205,8 +213,8 @@ void AActor::EnableNetworking(const bool enable)
 
 size_t AActor::PropagateMark()
 {
-	TMap<FName, DObject*>::Iterator it = { Behaviors };
-	TMap<FName, DObject*>::Pair* pair = nullptr;
+	TMap<FName, DBehavior*>::Iterator it = { Behaviors };
+	TMap<FName, DBehavior*>::Pair* pair = nullptr;
 	while (it.NextPair(pair))
 		GC::Mark(pair->Value);
 
@@ -468,6 +476,21 @@ void AActor::PostSerialize()
 //
 //==========================================================================
 
+void DBehavior::Serialize(FSerializer& arc)
+{
+	Super::Serialize(arc);
+	arc("owner", Owner)
+		("level", Level);
+}
+
+void DBehavior::OnDestroy()
+{
+	if (Level != nullptr)
+		Level->RemoveActorBehavior(*this);
+
+	Super::OnDestroy();
+}
+
 bool AActor::RemoveBehavior(const PClass& type)
 {
 	if (Behaviors.CheckKey(type.TypeName))
@@ -493,11 +516,11 @@ static int RemoveBehavior(AActor* self, PClass* type)
 DEFINE_ACTION_FUNCTION_NATIVE(AActor, RemoveBehavior, RemoveBehavior)
 {
 	PARAM_SELF_PROLOGUE(AActor);
-	PARAM_CLASS_NOT_NULL(type, DObject);
+	PARAM_CLASS_NOT_NULL(type, DBehavior);
 	ACTION_RETURN_BOOL(self->RemoveBehavior(*type));
 }
 
-DObject* AActor::AddBehavior(PClass& type)
+DBehavior* AActor::AddBehavior(PClass& type)
 {
 	if (type.bAbstract || !type.IsDescendantOf(NAME_Behavior))
 		return nullptr;
@@ -505,14 +528,13 @@ DObject* AActor::AddBehavior(PClass& type)
 	auto b = FindBehavior(type);
 	if (b == nullptr)
 	{
-		b = type.CreateNew();
+		b = dyn_cast<DBehavior>(type.CreateNew());
 		if (b == nullptr)
 			return nullptr;
 
-		auto& owner = b->PointerVar<AActor>(NAME_Owner);
-		owner = this;
-
+		b->Owner = this;
 		Behaviors[type.TypeName] = b;
+		Level->AddActorBehavior(*b);
 		IFOVERRIDENVIRTUALPTRNAME(b, NAME_Behavior, Initialize)
 		{
 			VMValue params[] = { b };
@@ -543,7 +565,7 @@ DObject* AActor::AddBehavior(PClass& type)
 	return b;
 }
 
-static DObject* AddBehavior(AActor* self, PClass* type)
+static DBehavior* AddBehavior(AActor* self, PClass* type)
 {
 	return self->AddBehavior(*type);
 }
@@ -551,17 +573,17 @@ static DObject* AddBehavior(AActor* self, PClass* type)
 DEFINE_ACTION_FUNCTION_NATIVE(AActor, AddBehavior, AddBehavior)
 {
 	PARAM_SELF_PROLOGUE(AActor);
-	PARAM_CLASS_NOT_NULL(type, DObject);
+	PARAM_CLASS_NOT_NULL(type, DBehavior);
 	ACTION_RETURN_OBJECT(self->AddBehavior(*type));
 }
 
 void AActor::TickBehaviors()
 {
 	TArray<FName> toRemove = {};
-	TArray<DObject*> toTick = {};
+	TArray<DBehavior*> toTick = {};
 
-	TMap<FName, DObject*>::Iterator it = { Behaviors };
-	TMap<FName, DObject*>::Pair* pair = nullptr;
+	TMap<FName, DBehavior*>::Iterator it = { Behaviors };
+	TMap<FName, DBehavior*>::Pair* pair = nullptr;
 	while (it.NextPair(pair))
 	{
 		auto b = pair->Value;
@@ -576,8 +598,7 @@ void AActor::TickBehaviors()
 
 	for (auto& b : toTick)
 	{
-		auto& owner = b->PointerVar<AActor>(NAME_Owner);
-		if (owner != this)
+		if (b->Owner != this)
 		{
 			toRemove.Push(pair->Key);
 			continue;
@@ -609,7 +630,7 @@ DEFINE_ACTION_FUNCTION_NATIVE(AActor, TickBehaviors, TickBehaviors)
 	return 0;
 }
 
-static DObject* FindBehavior(AActor* self, PClass* type)
+static DBehavior* FindBehavior(AActor* self, PClass* type)
 {
 	return self->FindBehavior(*type);
 }
@@ -617,7 +638,7 @@ static DObject* FindBehavior(AActor* self, PClass* type)
 DEFINE_ACTION_FUNCTION_NATIVE(AActor, FindBehavior, FindBehavior)
 {
 	PARAM_SELF_PROLOGUE(AActor);
-	PARAM_CLASS_NOT_NULL(type, DObject);
+	PARAM_CLASS_NOT_NULL(type, DBehavior);
 	ACTION_RETURN_OBJECT(self->FindBehavior(*type));
 }
 
@@ -632,8 +653,8 @@ void AActor::MoveBehaviors(AActor& from)
 	
 	// Clean up any empty behaviors that remained as well while
 	// changing the owner.
-	TMap<FName, DObject*>::Iterator it = { Behaviors };
-	TMap<FName, DObject*>::Pair* pair = nullptr;
+	TMap<FName, DBehavior*>::Iterator it = { Behaviors };
+	TMap<FName, DBehavior*>::Pair* pair = nullptr;
 	while (it.NextPair(pair))
 	{
 		auto b = pair->Value;
@@ -643,8 +664,12 @@ void AActor::MoveBehaviors(AActor& from)
 			continue;
 		}
 
-		auto& owner = b->PointerVar<AActor>(NAME_Owner);
-		owner = this;
+		b->Owner = this;
+		if (b->Level != b->Owner->Level)
+		{
+			b->Level->RemoveActorBehavior(*b);
+			b->Owner->Level->AddActorBehavior(*b);
+		}
 	}
 
 	for (auto& type : toRemove)
@@ -668,8 +693,8 @@ void AActor::ClearBehaviors()
 {
 	TArray<FName> toRemove = {};
 
-	TMap<FName, DObject*>::Iterator it = { Behaviors };
-	TMap<FName, DObject*>::Pair* pair = nullptr;
+	TMap<FName, DBehavior*>::Iterator it = { Behaviors };
+	TMap<FName, DBehavior*>::Pair* pair = nullptr;
 	while (it.NextPair(pair))
 		toRemove.Push(pair->Key);
 
diff --git a/src/scripting/backend/codegen_doom.cpp b/src/scripting/backend/codegen_doom.cpp
index 0e52790d72..9feab0f9c0 100644
--- a/src/scripting/backend/codegen_doom.cpp
+++ b/src/scripting/backend/codegen_doom.cpp
@@ -936,7 +936,7 @@ static DObject *BuiltinNewDoom(PClass *cls, int outerside, int backwardscompatib
 	// Creating actors here must be outright prohibited,
 	if (cls->IsDescendantOf(NAME_Actor))
 	{
-		ThrowAbortException(X_OTHER, "Cannot create actors with 'new'");
+		ThrowAbortException(X_OTHER, "Cannot create actors with 'new'. Use Actor.Spawn instead.");
 		return nullptr;
 	}
 	if (cls->IsDescendantOf(NAME_VisualThinker)) // Same for VisualThinkers.
@@ -944,6 +944,12 @@ static DObject *BuiltinNewDoom(PClass *cls, int outerside, int backwardscompatib
 		ThrowAbortException(X_OTHER, "Cannot create VisualThinker or inheriting classes with 'new'. Use 'VisualThinker.Spawn' instead.");
 		return nullptr;
 	}
+	// These don't make sense without an owning Actor so don't allow creating them.
+	if (cls->IsDescendantOf(NAME_Behavior))
+	{
+		ThrowAbortException(X_OTHER, "Behaviors must be added to existing Actors and cannot be created with 'new'");
+		return nullptr;
+	}
 	if ((vm_warnthinkercreation || !backwardscompatible) && cls->IsDescendantOf(NAME_Thinker))
 	{
 		// This must output a diagnostic warning
diff --git a/src/scripting/vmiterators.cpp b/src/scripting/vmiterators.cpp
index 1b6eae509e..f6a9c30b77 100644
--- a/src/scripting/vmiterators.cpp
+++ b/src/scripting/vmiterators.cpp
@@ -400,29 +400,41 @@ class DBehaviorIterator : public DObject
 {
 	DECLARE_ABSTRACT_CLASS(DBehaviorIterator, DObject)
 	size_t _index;
-	TArray<DObject*> _behaviors;
+	TArray<DBehavior*> _behaviors;
 
 public:
-	DBehaviorIterator(const AActor& mobj, FName type)
+	DBehaviorIterator(const AActor& mobj, PClass* type)
 	{
-		TMap<FName, DObject*>::ConstIterator it = { mobj.Behaviors };
-		TMap<FName, DObject*>::ConstPair* pair = nullptr;
+		TMap<FName, DBehavior*>::ConstIterator it = { mobj.Behaviors };
+		TMap<FName, DBehavior*>::ConstPair* pair = nullptr;
 		while (it.NextPair(pair))
 		{
 			auto b = pair->Value;
 			if (b == nullptr || (b->ObjectFlags & OF_EuthanizeMe))
 				continue;
 
-			if (type == NAME_None || b->IsKindOf(type))
+			if (type == nullptr || b->IsKindOf(type))
 				_behaviors.Push(b);
 		}
 
 		Reinit();
 	}
 
-	// TODO: List for all behaviors
+	DBehaviorIterator(const FLevelLocals& level, PClass* type)
+	{
+		for (auto& b : level.ActorBehaviors)
+		{
+			if (b == nullptr || (b->ObjectFlags & OF_EuthanizeMe))
+				continue;
 
-	DObject* Next()
+			if (type == nullptr || b->IsKindOf(type))
+				_behaviors.Push(b);
+		}
+
+		Reinit();
+	}
+
+	DBehavior* Next()
 	{
 		if (_index >= _behaviors.Size())
 			return nullptr;
@@ -443,18 +455,30 @@ IMPLEMENT_CLASS(DBehaviorIterator, true, false);
 
 static DBehaviorIterator* CreateBehaviorItFromActor(AActor* mobj, PClass* type)
 {
-	return Create<DBehaviorIterator>(*mobj, type == nullptr ? NAME_None : type->TypeName);
+	return Create<DBehaviorIterator>(*mobj, type);
 }
 
 DEFINE_ACTION_FUNCTION_NATIVE(DBehaviorIterator, CreateFrom, CreateBehaviorItFromActor)
 {
 	PARAM_PROLOGUE;
 	PARAM_OBJECT_NOT_NULL(mobj, AActor);
-	PARAM_CLASS(type, DObject);
+	PARAM_CLASS(type, DBehavior);
 	ACTION_RETURN_OBJECT(CreateBehaviorItFromActor(mobj, type));
 }
 
-static DObject* NextBehavior(DBehaviorIterator* self)
+static DBehaviorIterator* CreateBehaviorIt(PClass* type)
+{
+	return Create<DBehaviorIterator>(*primaryLevel, type);
+}
+
+DEFINE_ACTION_FUNCTION_NATIVE(DBehaviorIterator, Create, CreateBehaviorIt)
+{
+	PARAM_PROLOGUE;
+	PARAM_CLASS(type, DBehavior);
+	ACTION_RETURN_OBJECT(CreateBehaviorIt(type));
+}
+
+static DBehavior* NextBehavior(DBehaviorIterator* self)
 {
 	return self->Next();
 }
diff --git a/src/serializer_doom.cpp b/src/serializer_doom.cpp
index a46d8c3f3b..566aa31647 100644
--- a/src/serializer_doom.cpp
+++ b/src/serializer_doom.cpp
@@ -226,15 +226,15 @@ FSerializer& FDoomSerializer::StatePointer(const char* key, void* ptraddr, bool
 }
 
 
-FSerializer& Serialize(FSerializer& arc, const char* key, TMap<FName, DObject*>& value, TMap<FName, DObject*>* def)
+FSerializer& Serialize(FSerializer& arc, const char* key, TMap<FName, DBehavior*>& value, TMap<FName, DBehavior*>* def)
 {
 	if (!arc.BeginObject(key))
 		return arc;
 
 	if (arc.isWriting())
 	{
-		TMap<FName, DObject*>::Iterator it = { value };
-		TMap<FName, DObject*>::Pair* pair = nullptr;
+		TMap<FName, DBehavior*>::Iterator it = { value };
+		TMap<FName, DBehavior*>::Pair* pair = nullptr;
 		while (it.NextPair(pair))
 			arc(pair->Key.GetChars(), pair->Value);
 	}
@@ -243,7 +243,7 @@ FSerializer& Serialize(FSerializer& arc, const char* key, TMap<FName, DObject*>&
 		const char* k = nullptr;
 		while ((k = arc.GetKey()) != nullptr)
 		{
-			DObject* obj = nullptr;
+			DBehavior* obj = nullptr;
 			arc(k, obj);
 			value[k] = obj;
 		}
diff --git a/src/serializer_doom.h b/src/serializer_doom.h
index ec1a54fb89..4abc9fb196 100644
--- a/src/serializer_doom.h
+++ b/src/serializer_doom.h
@@ -3,6 +3,7 @@
 #include "serializer.h"
 
 class player_t;
+class DBehavior;
 struct sector_t;
 struct line_t;
 struct side_t;
@@ -43,7 +44,7 @@ FSerializer &Serialize(FSerializer &arc, const char *key, ticcmd_t &sid, ticcmd_
 FSerializer &Serialize(FSerializer &arc, const char *key, usercmd_t &cmd, usercmd_t *def);
 FSerializer &Serialize(FSerializer &arc, const char *key, FInterpolator &rs, FInterpolator *def);
 FSerializer& Serialize(FSerializer& arc, const char* key, struct FStandaloneAnimation& value, struct FStandaloneAnimation* defval);
-FSerializer& Serialize(FSerializer& arc, const char* key, TMap<FName, DObject*>& value, TMap<FName, DObject*>* def);
+FSerializer& Serialize(FSerializer& arc, const char* key, TMap<FName, DBehavior*>& value, TMap<FName, DBehavior*>* def);
 
 template<> FSerializer &Serialize(FSerializer &arc, const char *key, FPolyObj *&value, FPolyObj **defval);
 template<> FSerializer &Serialize(FSerializer &arc, const char *key, sector_t *&value, sector_t **defval);
diff --git a/wadsrc/static/zscript/actors/actor.zs b/wadsrc/static/zscript/actors/actor.zs
index ee20183e38..6f85778554 100644
--- a/wadsrc/static/zscript/actors/actor.zs
+++ b/wadsrc/static/zscript/actors/actor.zs
@@ -73,9 +73,10 @@ class ViewPosition native
 	native readonly int Flags;
 }
 
-class Behavior play abstract
+class Behavior native play abstract
 {
-	readonly Actor Owner;
+	native readonly Actor Owner;
+	native readonly LevelLocals Level;
 
 	virtual void Initialize() {}
 	virtual void Reinitialize() {}
@@ -85,6 +86,7 @@ class Behavior play abstract
 class BehaviorIterator native abstract final
 {
 	native static BehaviorIterator CreateFrom(Actor mobj, class<Behavior> type = null);
+	native static BehaviorIterator Create(class<Behavior> type = null);
 
 	native Behavior Next();
 	native void Reinit();