From 1e8064306e33c6cce4dff1a1e0413e8faa1cc274 Mon Sep 17 00:00:00 2001
From: Randy Heit <rheit@zdoom.fake>
Date: Thu, 13 Mar 2008 00:41:16 +0000
Subject: [PATCH] - Changed the sentinels in the thinker lists into a proper
 thinker. The old   way wasn't playing well with the write barriers. - Fixed:
 DFrameBuffer::WriteSavePic needs to fix the canvas in place while   using it.
 - Fixed: The sound system was updated every frame, which is a complete waste 
  of time. Doing it only once each tic is quite sufficient, since nothing  
 really moves between tics, even if the display makes it look otherwise.

SVN r799 (trunk)
---
 docs/rh-log.txt  |   9 ++
 src/d_main.cpp   |   4 +-
 src/d_net.cpp    |  36 ++++---
 src/dobject.cpp  |   2 +-
 src/dobject.h    |   1 +
 src/dthinker.cpp | 271 ++++++++++++++++++++++++++---------------------
 src/dthinker.h   |  38 ++++---
 src/v_video.cpp  |   2 +
 8 files changed, 213 insertions(+), 150 deletions(-)

diff --git a/docs/rh-log.txt b/docs/rh-log.txt
index 942dd2aeb..628663bf8 100644
--- a/docs/rh-log.txt
+++ b/docs/rh-log.txt
@@ -1,3 +1,12 @@
+March 12, 2008
+- Changed the sentinels in the thinker lists into a proper thinker. The old
+  way wasn't playing well with the write barriers.
+- Fixed: DFrameBuffer::WriteSavePic needs to fix the canvas in place while
+  using it.
+- Fixed: The sound system was updated every frame, which is a complete waste
+  of time. Doing it only once each tic is quite sufficient, since nothing
+  really moves between tics, even if the display makes it look otherwise.
+
 March 12, 2008 (Changes by Graf Zahl)
 - Fixed: The SimpleCanvas in FCanvasTexture must be declared as a soft
   root to the garbage collector.
diff --git a/src/d_main.cpp b/src/d_main.cpp
index 0a614962e..7c726cdbb 100644
--- a/src/d_main.cpp
+++ b/src/d_main.cpp
@@ -771,6 +771,8 @@ void D_DoomLoop ()
 				C_Ticker ();
 				M_Ticker ();
 				G_Ticker ();
+				// [RH] Use the consoleplayer's camera to update sounds
+				S_UpdateSounds (players[consoleplayer].camera);	// move positional sounds
 				gametic++;
 				maketic++;
 				GC::CheckGC ();
@@ -780,8 +782,6 @@ void D_DoomLoop ()
 			{
 				TryRunTics (); // will run at least one tic
 			}
-			// [RH] Use the consoleplayer's camera to update sounds
-			S_UpdateSounds (players[consoleplayer].camera);	// move positional sounds
 			// Update display, next frame, with current state.
 			I_StartTic ();
 			D_Display ();
diff --git a/src/d_net.cpp b/src/d_net.cpp
index 1bb322540..f17f7b460 100644
--- a/src/d_net.cpp
+++ b/src/d_net.cpp
@@ -1825,25 +1825,29 @@ void TryRunTics (void)
 	}
 
 	// run the count tics
-	while (counts--)
+	if (counts > 0)
 	{
-		if (gametic > lowtic)
+		while (counts--)
 		{
-			I_Error ("gametic>lowtic");
-		}
-		if (advancedemo)
-		{
-			D_DoAdvanceDemo ();
-		}
-		if (debugfile) fprintf (debugfile, "run tic %d\n", gametic);
-		C_Ticker ();
-		M_Ticker ();
-		I_GetTime (true);
-		G_Ticker ();
-		GC::CheckGC ();
-		gametic++;
+			if (gametic > lowtic)
+			{
+				I_Error ("gametic>lowtic");
+			}
+			if (advancedemo)
+			{
+				D_DoAdvanceDemo ();
+			}
+			if (debugfile) fprintf (debugfile, "run tic %d\n", gametic);
+			C_Ticker ();
+			M_Ticker ();
+			I_GetTime (true);
+			G_Ticker ();
+			GC::CheckGC ();
+			gametic++;
 
-		NetUpdate ();	// check for new console commands
+			NetUpdate ();	// check for new console commands
+		}
+		S_UpdateSounds (players[consoleplayer].camera);	// move positional sounds
 	}
 }
 
diff --git a/src/dobject.cpp b/src/dobject.cpp
index 346089039..711fd934c 100644
--- a/src/dobject.cpp
+++ b/src/dobject.cpp
@@ -412,7 +412,7 @@ DObject::~DObject ()
 
 void DObject::Destroy ()
 {
-	ObjectFlags |= OF_EuthanizeMe;
+	ObjectFlags = (ObjectFlags & ~OF_Fixed) | OF_EuthanizeMe;
 }
 
 size_t DObject::PropagateMark()
diff --git a/src/dobject.h b/src/dobject.h
index 00cd163d9..e2ad22aa0 100644
--- a/src/dobject.h
+++ b/src/dobject.h
@@ -226,6 +226,7 @@ enum EObjectFlags
 	// Other flags
 	OF_JustSpawned		= 1 << 8,		// Thinker was spawned this tic
 	OF_SerialSuccess	= 1 << 9,		// For debugging Serialize() calls
+	OF_Sentinel			= 1 << 10,		// Object is serving as the sentinel in a ring list
 };
 
 template<class T> class TObjPtr;
diff --git a/src/dthinker.cpp b/src/dthinker.cpp
index bd8990bbb..37ff39543 100644
--- a/src/dthinker.cpp
+++ b/src/dthinker.cpp
@@ -44,24 +44,71 @@ static cycle_t ThinkCycles;
 extern cycle_t BotSupportCycles;
 extern cycle_t BotWTG;
 
-IMPLEMENT_CLASS (DThinker)
+IMPLEMENT_POINTY_CLASS (DThinker)
+ DECLARE_POINTER(NextThinker)
+ DECLARE_POINTER(PrevThinker)
+END_POINTERS
 
-static Node *NextToThink;
+static DThinker *NextToThink;
 
-List DThinker::Thinkers[MAX_STATNUM+1];
-List DThinker::FreshThinkers[MAX_STATNUM+1];
+FThinkerList DThinker::Thinkers[MAX_STATNUM+1];
+FThinkerList DThinker::FreshThinkers[MAX_STATNUM+1];
 bool DThinker::bSerialOverride = false;
 
-void DThinker::SaveList(FArchive &arc, Node *node)
+void FThinkerList::AddTail(DThinker *thinker)
 {
-	if (node->Succ != NULL)
+	if (Sentinel == NULL)
 	{
-		do
+		Sentinel = new DThinker(DThinker::NO_LINK);
+		Sentinel->ObjectFlags |= OF_Sentinel;
+		Sentinel->NextThinker = Sentinel;
+		Sentinel->PrevThinker = Sentinel;
+		GC::WriteBarrier(Sentinel);
+	}
+	DThinker *tail = Sentinel->PrevThinker;
+	assert(tail->NextThinker == Sentinel);
+	thinker->PrevThinker = tail;
+	thinker->NextThinker = Sentinel;
+	tail->NextThinker = thinker;
+	Sentinel->PrevThinker = thinker;
+	GC::WriteBarrier(thinker, tail);
+	GC::WriteBarrier(thinker, Sentinel);
+	GC::WriteBarrier(tail, thinker);
+	GC::WriteBarrier(Sentinel, thinker);
+}
+
+DThinker *FThinkerList::GetHead() const
+{
+	if (Sentinel == NULL || Sentinel->NextThinker == Sentinel)
+	{
+		return NULL;
+	}
+	return Sentinel->NextThinker;
+}
+
+DThinker *FThinkerList::GetTail() const
+{
+	if (Sentinel == NULL || Sentinel->PrevThinker == Sentinel)
+	{
+		return NULL;
+	}
+	return Sentinel->PrevThinker;
+}
+
+bool FThinkerList::IsEmpty() const
+{
+	return Sentinel == NULL || Sentinel->NextThinker == NULL;
+}
+
+void DThinker::SaveList(FArchive &arc, DThinker *node)
+{
+	if (node != NULL)
+	{
+		while (!(node->ObjectFlags & OF_Sentinel))
 		{
-			DThinker *thinker = static_cast<DThinker *>(node);
-			arc << thinker;
-			node = node->Succ;
-		} while (node->Succ != NULL);
+			arc << node;
+			node = node->NextThinker;
+		}
 	}
 }
 
@@ -92,8 +139,8 @@ void DThinker::SerializeAll(FArchive &arc, bool hubLoad)
 			{
 				stat = i;
 				arc << stat;
-				SaveList(arc, Thinkers[i].Head);
-				SaveList(arc, FreshThinkers[i].Head);
+				SaveList(arc, Thinkers[i].GetHead());
+				SaveList(arc, FreshThinkers[i].GetHead());
 				thinker = NULL;
 				arc << thinker;		// Save a final NULL for this list
 			}
@@ -146,7 +193,8 @@ DThinker::DThinker (int statnum) throw()
 {
 	if (bSerialOverride)
 	{ // The serializer will insert us into the right list
-		Succ = NULL;
+		NextThinker = NULL;
+		PrevThinker = NULL;
 		return;
 	}
 
@@ -155,46 +203,43 @@ DThinker::DThinker (int statnum) throw()
 	{
 		statnum = MAX_STATNUM;
 	}
-	if (FreshThinkers[statnum].TailPred->Pred != NULL)
-	{
-		GC::WriteBarrier(static_cast<DThinker*>(FreshThinkers[statnum].Tail, this));
-	}
-	else
-	{
-		GC::WriteBarrier(this);
-	}
 	FreshThinkers[statnum].AddTail (this);
 }
 
+DThinker::DThinker(no_link_type foo) throw()
+{
+	foo;	// Avoid unused argument warnings.
+}
+
 DThinker::~DThinker ()
 {
-	if (Succ != NULL)
-	{
-		Remove ();
-	}
+	assert(NextThinker == NULL && PrevThinker == NULL);
 }
 
 void DThinker::Destroy ()
 {
-	if (this == NextToThink)
-		NextToThink = Succ;
-
-	if (Succ != NULL)
+	if (NextThinker != NULL)
 	{
-		Remove ();
-		Succ = NULL;
+		Remove();
 	}
-	Super::Destroy ();
+	Super::Destroy();
 }
 
 void DThinker::Remove()
 {
-	if (Pred->Pred != NULL && Succ->Succ != NULL)
+	if (this == NextToThink)
 	{
-		GC::WriteBarrier(static_cast<DThinker *>(Pred), static_cast<DThinker *>(Succ));
-		GC::WriteBarrier(static_cast<DThinker *>(Succ), static_cast<DThinker *>(Pred));
+		NextToThink = NextThinker;
 	}
-	Node::Remove();
+	DThinker *prev = PrevThinker;
+	DThinker *next = NextThinker;
+	assert(prev != NULL && next != NULL);
+	prev->NextThinker = next;
+	next->PrevThinker = prev;
+	GC::WriteBarrier(prev, next);
+	GC::WriteBarrier(next, prev);
+	NextThinker = NULL;
+	PrevThinker = NULL;
 }
 
 void DThinker::PostBeginPlay ()
@@ -203,33 +248,33 @@ void DThinker::PostBeginPlay ()
 
 DThinker *DThinker::FirstThinker (int statnum)
 {
-	Node *node;
+	DThinker *node;
 
 	if ((unsigned)statnum > MAX_STATNUM)
 	{
 		statnum = MAX_STATNUM;
 	}
-	node = Thinkers[statnum].Head;
-	if (node->Succ == NULL)
+	node = Thinkers[statnum].GetHead();
+	if (node == NULL)
 	{
-		node = FreshThinkers[statnum].Head;
-		if (node->Succ == NULL)
+		node = FreshThinkers[statnum].GetHead();
+		if (node == NULL)
 		{
 			return NULL;
 		}
 	}
-	return static_cast<DThinker *>(node);
+	return node;
 }
 
 void DThinker::ChangeStatNum (int statnum)
 {
-	List *list;
+	FThinkerList *list;
 
 	if ((unsigned)statnum > MAX_STATNUM)
 	{
 		statnum = MAX_STATNUM;
 	}
-	Remove ();
+	Remove();
 	if ((ObjectFlags & OF_JustSpawned) && statnum >= STAT_FIRST_THINKING)
 	{
 		list = &FreshThinkers[statnum];
@@ -238,14 +283,6 @@ void DThinker::ChangeStatNum (int statnum)
 	{
 		list = &Thinkers[statnum];
 	}
-	if (list->TailPred->Pred != NULL)
-	{
-		GC::WriteBarrier(static_cast<DThinker*>(list->Tail, this));
-	}
-	else
-	{
-		GC::WriteBarrier(this);
-	}
 	list->AddTail(this);
 }
 
@@ -254,24 +291,11 @@ void DThinker::MarkRoots()
 {
 	for (int i = 0; i <= MAX_STATNUM; ++i)
 	{
-		DThinker *t = static_cast<DThinker *>(Thinkers[i].Head);
-		GC::Mark(t);
-		t = static_cast<DThinker *>(FreshThinkers[i].Head);
-		GC::Mark(t);
+		GC::Mark(Thinkers[i].Sentinel);
+		GC::Mark(FreshThinkers[i].Sentinel);
 	}
 }
 
-size_t DThinker::PropagateMark()
-{
-	// Mark the next thinker in my list
-	if (Succ != NULL && Succ->Succ != NULL)
-	{
-		DThinker *t = static_cast<DThinker *>(Succ);
-		GC::Mark(t);
-	}
-	return Super::PropagateMark();
-}
-
 // Destroy every thinker
 void DThinker::DestroyAllThinkers ()
 {
@@ -281,11 +305,11 @@ void DThinker::DestroyAllThinkers ()
 	{
 		if (i != STAT_TRAVELLING)
 		{
-			DestroyThinkersInList (Thinkers[i].Head);
-			DestroyThinkersInList (FreshThinkers[i].Head);
+			DestroyThinkersInList (Thinkers[i]);
+			DestroyThinkersInList (FreshThinkers[i]);
 		}
 	}
-	GC::FullGC ();
+	GC::FullGC();
 }
 
 // Destroy all thinkers except for player-controlled actors
@@ -303,34 +327,42 @@ void DThinker::DestroyMostThinkers ()
 			DestroyMostThinkersInList (FreshThinkers[i], i);
 		}
 	}
-	GC::FullGC ();
+	GC::FullGC();
 }
 
-void DThinker::DestroyThinkersInList (Node *node)
+void DThinker::DestroyThinkersInList (FThinkerList &list)
 {
-	while (node->Succ != NULL)
+	if (list.Sentinel != NULL)
 	{
-		Node *next = node->Succ;
-		static_cast<DThinker *> (node)->Destroy ();
-		node = next;
+		DThinker *node = list.Sentinel->NextThinker;
+		while (node != list.Sentinel)
+		{
+			DThinker *next = node->NextThinker;
+			node->Destroy();
+			node = next;
+		}
+		list.Sentinel->Destroy();
+		list.Sentinel = NULL;
 	}
 }
 
-void DThinker::DestroyMostThinkersInList (List &list, int stat)
+void DThinker::DestroyMostThinkersInList (FThinkerList &list, int stat)
 {
 	if (stat != STAT_PLAYER)
 	{
-		DestroyThinkersInList (list.Head);
+		DestroyThinkersInList (list);
 	}
-	else
+	else if (list.Sentinel != NULL)
 	{
-		Node *node = list.Head;
-		while (node->Succ != NULL)
+		DThinker *node = list.Sentinel->NextThinker;
+		while (node != list.Sentinel)
 		{
-			Node *next = node->Succ;
-			node->Remove ();
+			DThinker *next = node->NextThinker;
+			node->Remove();
 			node = next;
 		}
+		list.Sentinel->Destroy();
+		list.Sentinel = NULL;
 	}
 }
 
@@ -361,42 +393,38 @@ void DThinker::RunThinkers ()
 	unclock (ThinkCycles);
 }
 
-int DThinker::TickThinkers (List *list, List *dest)
+int DThinker::TickThinkers (FThinkerList *list, FThinkerList *dest)
 {
 	int count = 0;
-	Node *node = list->Head;
+	DThinker *node = list->GetHead();
 
-	while (node->Succ != NULL)
+	if (node == NULL)
+	{
+		return 0;
+	}
+
+	while (node != list->Sentinel)
 	{
 		++count;
-		NextToThink = node->Succ;
-		DThinker *thinker = static_cast<DThinker *> (node);
-		if (thinker->ObjectFlags & OF_JustSpawned)
+		NextToThink = node->NextThinker;
+		if (node->ObjectFlags & OF_JustSpawned)
 		{
-			thinker->ObjectFlags &= ~OF_JustSpawned;
+			node->ObjectFlags &= ~OF_JustSpawned;
 			if (dest != NULL)
 			{ // Move thinker from this list to the destination list
-				node->Remove ();
-				if (dest->TailPred->Pred != NULL)
-				{
-					GC::WriteBarrier(static_cast<DThinker*>(dest->Tail, thinker));
-				}
-				else
-				{
-					GC::WriteBarrier(thinker);
-				}
-				dest->AddTail (node);
+				node->Remove();
+				dest->AddTail(node);
 			}
-			thinker->PostBeginPlay ();
+			node->PostBeginPlay();
 		}
 		else if (dest != NULL)
 		{ // Move thinker from this list to the destination list
-			I_Error ("There is a thinker in the fresh list that has already ticked.\n");
+			I_Error("There is a thinker in the fresh list that has already ticked.\n");
 		}
 
-		if (!(thinker->ObjectFlags & OF_EuthanizeMe))
+		if (!(node->ObjectFlags & OF_EuthanizeMe))
 		{ // Only tick thinkers not scheduled for destruction
-			thinker->Tick ();
+			node->Tick ();
 		}
 		node = NextToThink;
 	}
@@ -420,7 +448,7 @@ FThinkerIterator::FThinkerIterator (const PClass *type, int statnum)
 		m_SearchStats = false;
 	}
 	m_ParentType = type;
-	m_CurrThinker = DThinker::Thinkers[m_Stat].Head;
+	m_CurrThinker = DThinker::Thinkers[m_Stat].GetHead();
 	m_SearchingFresh = false;
 }
 
@@ -437,43 +465,48 @@ FThinkerIterator::FThinkerIterator (const PClass *type, int statnum, DThinker *p
 		m_SearchStats = false;
 	}
 	m_ParentType = type;
-	if (prev == NULL || prev->Succ == NULL)
+	if (prev == NULL || (prev->NextThinker->ObjectFlags & OF_Sentinel))
 	{
-		Reinit ();
+		Reinit();
 	}
 	else
 	{
-		m_CurrThinker = prev->Succ;
+		m_CurrThinker = prev->NextThinker;
 		m_SearchingFresh = false;
 	}
 }
 
 void FThinkerIterator::Reinit ()
 {
-	m_CurrThinker = DThinker::Thinkers[m_Stat].Head;
+	m_CurrThinker = DThinker::Thinkers[m_Stat].GetHead();
 	m_SearchingFresh = false;
 }
 
 DThinker *FThinkerIterator::Next ()
 {
-	if (m_ParentType == NULL) return NULL;
+	if (m_ParentType == NULL)
+	{
+		return NULL;
+	}
 	do
 	{
 		do
 		{
-			while (m_CurrThinker->Succ)
+			if (m_CurrThinker != NULL)
 			{
-				DThinker *thinker = static_cast<DThinker *> (m_CurrThinker);
-				if (thinker->IsKindOf (m_ParentType))
+				while (!(m_CurrThinker->ObjectFlags & OF_Sentinel))
 				{
-					m_CurrThinker = m_CurrThinker->Succ;
-					return thinker;
+					DThinker *thinker = m_CurrThinker;
+					m_CurrThinker = thinker->NextThinker;
+					if (thinker->IsKindOf(m_ParentType))
+					{
+						return thinker;
+					}
 				}
-				m_CurrThinker = m_CurrThinker->Succ;
 			}
 			if ((m_SearchingFresh = !m_SearchingFresh))
 			{
-				m_CurrThinker = DThinker::FreshThinkers[m_Stat].Head;
+				m_CurrThinker = DThinker::FreshThinkers[m_Stat].GetHead();
 			}
 		} while (m_SearchingFresh);
 		if (m_SearchStats)
@@ -484,7 +517,7 @@ DThinker *FThinkerIterator::Next ()
 				m_Stat = STAT_FIRST_THINKING;
 			}
 		}
-		m_CurrThinker = DThinker::Thinkers[m_Stat].Head;
+		m_CurrThinker = DThinker::Thinkers[m_Stat].GetHead();
 		m_SearchingFresh = false;
 	} while (m_SearchStats && m_Stat != STAT_FIRST_THINKING);
 	return NULL;
diff --git a/src/dthinker.h b/src/dthinker.h
index 53e52846a..d6611c0c7 100644
--- a/src/dthinker.h
+++ b/src/dthinker.h
@@ -36,7 +36,6 @@
 
 #include <stdlib.h>
 #include "dobject.h"
-#include "lists.h"
 
 class AActor;
 class player_s;
@@ -48,15 +47,25 @@ class FThinkerIterator;
 
 enum { MAX_STATNUM = 127 };
 
-// Doubly linked list of thinkers
-class DThinker : public DObject, private Node
+// Doubly linked ring list of thinkers
+struct FThinkerList
+{
+	FThinkerList() : Sentinel(0) {}
+	void AddTail(DThinker *thinker);
+	DThinker *GetHead() const;
+	DThinker *GetTail() const;
+	bool IsEmpty() const;
+
+	DThinker *Sentinel;
+};
+
+class DThinker : public DObject
 {
 	DECLARE_CLASS (DThinker, DObject)
-
+	HAS_OBJECT_POINTERS
 public:
 	DThinker (int statnum = MAX_STATNUM) throw();
 	void Destroy ();
-	size_t PropagateMark();
 	virtual ~DThinker ();
 	virtual void Tick ();
 	virtual void PostBeginPlay ();	// Called just before the first tick
@@ -73,18 +82,23 @@ public:
 	static DThinker *FirstThinker (int statnum);
 
 private:
-	static void DestroyThinkersInList (Node *first);
-	static void DestroyMostThinkersInList (List &list, int stat);
-	static int TickThinkers (List *list, List *dest);	// Returns: # of thinkers ticked
-	static void SaveList(FArchive &arc, Node *node);
+	enum no_link_type { NO_LINK };
+	DThinker(no_link_type) throw();
+	static void DestroyThinkersInList (FThinkerList &list);
+	static void DestroyMostThinkersInList (FThinkerList &list, int stat);
+	static int TickThinkers (FThinkerList *list, FThinkerList *dest);	// Returns: # of thinkers ticked
+	static void SaveList(FArchive &arc, DThinker *node);
 	void Remove();
 
-	static List Thinkers[MAX_STATNUM+1];		// Current thinkers
-	static List FreshThinkers[MAX_STATNUM+1];	// Newly created thinkers
+	static FThinkerList Thinkers[MAX_STATNUM+1];		// Current thinkers
+	static FThinkerList FreshThinkers[MAX_STATNUM+1];	// Newly created thinkers
 	static bool bSerialOverride;
 
+	friend struct FThinkerList;
 	friend class FThinkerIterator;
 	friend class DObject;
+
+	DThinker *NextThinker, *PrevThinker;
 };
 
 class FThinkerIterator
@@ -92,7 +106,7 @@ class FThinkerIterator
 protected:
 	const PClass *m_ParentType;
 private:
-	Node *m_CurrThinker;
+	DThinker *m_CurrThinker;
 	BYTE m_Stat;
 	bool m_SearchStats;
 	bool m_SearchingFresh;
diff --git a/src/v_video.cpp b/src/v_video.cpp
index cec1edbad..b90898eff 100644
--- a/src/v_video.cpp
+++ b/src/v_video.cpp
@@ -1385,11 +1385,13 @@ void DFrameBuffer::WriteSavePic (player_t *player, FILE *file, int width, int he
 	PalEntry palette[256];
 
 	// Take a snapshot of the player's view
+	pic->ObjectFlags |= OF_Fixed;
 	pic->Lock ();
 	R_RenderViewToCanvas (player->mo, pic, 0, 0, width, height);
 	GetFlashedPalette (palette);
 	M_CreatePNG (file, pic->GetBuffer(), palette, SS_PAL, width, height, pic->GetPitch());
 	pic->Unlock ();
+	pic->Destroy();
 	pic->ObjectFlags |= OF_YesReallyDelete;
 	delete pic;
 }