refactor(actors): per-leader FirstSlave chain — convert 6 walks to O(slaves) (Phase 4)

src/Modules/Actors.bb

@@ -107,6 +107,19 @@ Type ActorInstance
 	; (login / logout / FreeActorInstance); see Actors.bb's helper
 	; functions and ServerNet.bb's P_StartGame / P_Disconnect handlers.
 	Field NextOnlinePlayer.ActorInstance
+	; Linked list of this actor's slaves (pets / mounts / summons).
+	; Head is FirstSlave; chained via Slave\NextSlave on each slave.
+	; Replaces `For Each ActorInstance / If X\Leader = this` walks
+	; (Actors.bb's WriteActorInstance + FreeActorInstanceSlaves,
+	; GameServer.bb's pet-aggro broadcast, MySQL.bb's My_SaveActorInstance,
+	; ServerNet.bb's /pet command + inventory pet-validation walk).
+	; Maintained by SlaveLink / SlaveUnlink helpers and at the three
+	; sites that mutate \Leader: load-from-stream (Actors.bb's
+	; ReadActorInstance), load-from-DB (MySQL.bb's
+	; My_LoadActorInstance), and BVM_SETLEADER (handles both
+	; assignment and clearing). FreeActorInstance unlinks defensively.
+	Field FirstSlave.ActorInstance
+	Field NextSlave.ActorInstance
 	Field X#, Y#, Z#
 	Field OldX#, OldZ#
 	Field DestX#, DestZ#
@@ -344,15 +357,13 @@ Function WriteActorInstance(Stream, A.ActorInstance)
 	WriteShort Stream, A\LastPortal
 	WriteInt Stream, A\LastPortalTime
 
-	; Data for any slaves
-	Slaves = A\NumberOfSlaves
-	While Slaves > 0
-		For Slave.ActorInstance = Each ActorInstance
-			If Slave\Leader = A
-				WriteActorInstance(Stream, Slave)
-				Slaves = Slaves - 1
-			EndIf
-		Next
+	; Data for any slaves. Walk this leader's FirstSlave chain
+	; instead of the global ActorInstance list. The chain replaces
+	; the previous O(global_actors) walk filtered by `Leader = A`.
+	Local Slave.ActorInstance = A\FirstSlave
+	While Slave <> Null
+		WriteActorInstance(Stream, Slave)
+		Slave = Slave\NextSlave
 	Wend
 
 End Function
@@ -497,10 +508,21 @@ Function ReadActorInstance.ActorInstance(Stream)
 	EndIf
 
 	; Slaves
-	For i = 1 To A\NumberOfSlaves
+	;
+	; SlaveLink maintains the FirstSlave chain + NumberOfSlaves count.
+	; The load loop reads N records from disk where N was the
+	; previously-saved NumberOfSlaves; SlaveLink will INCREMENT
+	; NumberOfSlaves on each call. The post-load count must match the
+	; pre-load count, so reset to 0 before the loop and let SlaveLink
+	; restore it.
+	Local SavedSlaveCount% = A\NumberOfSlaves
+	A\NumberOfSlaves = 0
+	For i = 1 To SavedSlaveCount
 		Slave.ActorInstance = ReadActorInstance(Stream)
-		Slave\Leader = A
-		Slave\AIMode = AI_Pet
+		If Slave <> Null
+			SlaveLink(A, Slave)
+			Slave\AIMode = AI_Pet
+		EndIf
 	Next
 
 	; If actor didn't exist, delete all slaves and return nothing
@@ -593,6 +615,51 @@ Function CreateActorInstance.ActorInstance(Actor.Actor)
 
 End Function
 
+; Links Slave under Leader: sets Slave\Leader, head-inserts into
+; Leader\FirstSlave chain, increments Leader\NumberOfSlaves. The
+; canonical replacement for bare `Slave\Leader = Leader` (which left
+; the chain inconsistent) — every leader-assignment site should call
+; this. Safe no-op on Null Slave or Null Leader.
+;
+; If Slave was already linked to a different leader, unlinks from
+; the old chain first to avoid being in two chains simultaneously.
+Function SlaveLink(Leader.ActorInstance, Slave.ActorInstance)
+
+	If Leader = Null Or Slave = Null Then Return
+	If Slave\Leader = Leader Then Return
+	; Detach from any current leader before re-attaching.
+	If Slave\Leader <> Null Then SlaveUnlink(Slave)
+	Slave\Leader = Leader
+	Slave\NextSlave = Leader\FirstSlave
+	Leader\FirstSlave = Slave
+	Leader\NumberOfSlaves = Leader\NumberOfSlaves + 1
+
+End Function
+
+; Removes Slave from its current Leader's chain, decrements
+; NumberOfSlaves, clears Slave\Leader. Safe no-op when Slave has no
+; leader (NPCs without a master).
+Function SlaveUnlink(Slave.ActorInstance)
+
+	If Slave = Null Then Return
+	Local Leader.ActorInstance = Slave\Leader
+	If Leader = Null Then Return
+	; Walk-to-find-predecessor splice on the leader's chain.
+	If Leader\FirstSlave = Slave
+		Leader\FirstSlave = Slave\NextSlave
+	Else
+		Local Prev.ActorInstance = Leader\FirstSlave
+		While Prev <> Null And Prev\NextSlave <> Slave
+			Prev = Prev\NextSlave
+		Wend
+		If Prev <> Null Then Prev\NextSlave = Slave\NextSlave
+	EndIf
+	Slave\NextSlave = Null
+	Slave\Leader = Null
+	Leader\NumberOfSlaves = Leader\NumberOfSlaves - 1
+
+End Function
+
 ; Inserts A at the head of the FirstOnlinePlayer chain. Idempotent
 ; via a presence check (a double-insert from a buggy caller would
 ; create a cycle in the chain). Called at login completion in
@@ -654,42 +721,54 @@ Function FreeActorInstance(A.ActorInstance)
 	; FirstOnlinePlayer chain cleanup -- safe no-op when A wasn't an
 	; online player (NPCs, never-logged-in characters).
 	OnlinePlayerRemove(A)
-	If A\Leader <> Null Then A\Leader\NumberOfSlaves = A\Leader\NumberOfSlaves - 1
+	; FirstSlave chain cleanup. SlaveUnlink handles the NumberOfSlaves
+	; decrement and clears Slave\Leader; safe no-op when A had no
+	; leader.
+	If A\Leader <> Null Then SlaveUnlink(A)
+	; Also free this actor's own slave chain (defensive — typically
+	; FreeActorInstanceSlaves was called first by the caller, but if
+	; not, leaving dangling NextSlave pointers from this freed actor's
+	; FirstSlave would corrupt the children's traversal). Clear without
+	; recursing into Delete -- the surviving children are simply
+	; orphaned (Leader = Null).
+	Local Child.ActorInstance = A\FirstSlave
+	Local ChildNext.ActorInstance = Null
+	While Child <> Null
+		ChildNext = Child\NextSlave
+		Child\Leader = Null
+		Child\NextSlave = Null
+		Child = ChildNext
+	Wend
+	A\FirstSlave = Null
 	Delete(A)
 
 End Function
 
 ; Frees all the slaves of an actor instance (RECURSIVE)
 ;
-; Restart-on-Delete cursor pattern: Blitz3D's `For X = Each Type`
-; iterator advances via the deleted element's "next" pointer on
-; each Next, so calling FreeActorInstance(A2) inside the loop body
-; corrupts the cursor for the next iteration. The recursion makes
-; the After-cursor walk used in PausedScript cleanup insufficient
-; (a recursive call can free an actor that's after the outer
-; cursor's captured A2Next).
+; Head-capture pattern: each iteration reads A\FirstSlave fresh,
+; recursively frees the child's slaves, then calls FreeActorInstance
+; which SlaveUnlinks the child from A's chain (mutating A\FirstSlave).
+; The next iteration's read picks up the new head. Safe because slave
+; chains are per-leader and disjoint: the recursive call into Child's
+; own FreeActorInstanceSlaves can only mutate Child's chain, never A's.
 ;
-; Restart the For loop after every free instead. O(n*slaves) but
-; the iteration cost is small (Leader comparison) and slaves are
-; usually 1-10 per leader. The outer loop terminates as soon as
-; the search completes without finding any remaining slaves.
+; Replaces the earlier For-Each + Restart-on-Delete pattern, which was
+; needed when the walk was over the global ActorInstance list filtered
+; by Leader; the chain walk doesn't need restart because the chain
+; mutation is the natural termination condition.
 Function FreeActorInstanceSlaves(A.ActorInstance)
 
-	Local Found = True
-	While Found
-		Found = False
-		For A2.ActorInstance = Each ActorInstance
-			If A\NumberOfSlaves = 0 Then Exit
-			If A2\Leader = A
-				Found = True
-				FreeActorInstanceSlaves(A2)
-				FreeActorInstance(A2)
-				; Restart from a fresh iterator next outer pass --
-				; the For-Each cursor is invalid after the Delete
-				; inside FreeActorInstance above.
-				Exit
-			EndIf
-		Next
+	; Walk A's FirstSlave chain. Body recursively frees nested
+	; slaves first (their FirstSlave chains), then calls
+	; FreeActorInstance which SlaveUnlinks from A's chain and
+	; Delete()s the slave. The unlink mutates A\FirstSlave, so capture
+	; the head before each step rather than walking with a cursor that
+	; could point at freed memory.
+	While A\FirstSlave <> Null
+		Local Child.ActorInstance = A\FirstSlave
+		FreeActorInstanceSlaves(Child)
+		FreeActorInstance(Child)
 	Wend
 
 End Function

src/Modules/GameServer.bb

@@ -585,20 +585,22 @@ Function ActorAttack(A1.ActorInstance, A2.ActorInstance)
 
 	.SkipAttackNet
 
-	; If target was a player with pets, make pets attack too
+	; If target was a player with pets, make pets attack too.
+	; Walk A1's FirstSlave chain instead of every ActorInstance.
+	; Local-name is `PetCur` because A3 is implicit-declared at
+	; function scope by a `For A3.ActorInstance = Each` earlier in
+	; this function (BlitzForge Local + For-Each collision -- see
+	; the feedback_blitz_local_for_each_collision memory).
 	If A1\RNID > 0
 		If A1\NumberOfSlaves > 0
-			Found = 0
-			For A3.ActorInstance = Each ActorInstance
-				If A3\Leader = A1
-					Found = Found + 1
-					If A3\Actor\Aggressiveness < 3 And A3\AITarget = Null
-						A3\AITarget = A2
-						A3\AIMode = AI_PetChase
-					EndIf
-					If Found = A1\NumberOfSlaves Then Exit
+			Local PetCur.ActorInstance = A1\FirstSlave
+			While PetCur <> Null
+				If PetCur\Actor\Aggressiveness < 3 And PetCur\AITarget = Null
+					PetCur\AITarget = A2
+					PetCur\AIMode = AI_PetChase
 				EndIf
-			Next
+				PetCur = PetCur\NextSlave
+			Wend
 		EndIf
 	EndIf
 

src/Modules/MySQL.bb

@@ -486,16 +486,14 @@ Function My_SaveActorInstance(A.ActorInstance, Q.QuestLog, C.ActionbarData, IsSl
 		Next
 	End If
 
-	; Save Slaves
-	Slaves = A\NumberOfSlaves
-	While Slaves > 0
-		For Slave.ActorInstance = Each ActorInstance
-			If Slave\Leader = A
-				;   Saves Slaves | Instance | Quests | Actionbar | isSlave | AccountNumber | Parent
-				My_SaveActorInstance(Slave,    Null,     Null,       True,    AccountID, A\My_ID)
-				Slaves = Slaves -1
-			End If
-		Next
+	; Save Slaves -- walk A's FirstSlave chain instead of every
+	; ActorInstance. Same shape as the flat-file SaveActor path
+	; in Actors.bb.
+	Local Slave.ActorInstance = A\FirstSlave
+	While Slave <> Null
+		;   Saves Slaves | Instance | Quests | Actionbar | isSlave | AccountNumber | Parent
+		My_SaveActorInstance(Slave,    Null,     Null,       True,    AccountID, A\My_ID)
+		Slave = Slave\NextSlave
 	Wend
 
 End Function
@@ -643,7 +641,16 @@ Function My_LoadActorInstance.ActorInstance(ActID, Q.Questlog, C.ActionBarData,
 	A\DeathScript$		= ReadSQLField(Row, "dscript")
 	A\Reputation		= ReadSQLField(Row, "rep")
 	A\Gold				= ReadSQLField(Row, "gold")
-	A\NumberOfslaves	= ReadSQLField(Row, "slaves")
+	A\NumberOfSlaves	= ReadSQLField(Row, "slaves")
+	; SlaveLink (called per slave-row in the loop below) increments
+	; NumberOfSlaves on each call, so the saved count would double-
+	; count if left in place. Reset to 0 here and let SlaveLink rebuild
+	; the count from the actual rows that load successfully -- this
+	; also catches the case where a slave row references a missing
+	; actor and My_LoadActorInstance returns Null (the SlaveLink call
+	; is then skipped via the `If Slave <> Null` guard, and the count
+	; correctly reflects only the slaves that actually loaded).
+	A\NumberOfSlaves	= 0
 	A\HomeFaction		= ReadSQLField(Row, "homefaction")
 	; FactionNames$ / FactionDefaultRatings are Dim'd (99) and
 	; FactionRatings is Field[99]. A corrupt SQL row could carry an
@@ -910,10 +917,13 @@ Function My_LoadActorInstance.ActorInstance(ActID, Q.Questlog, C.ActionBarData,
 		; Get the slaves ID
 		SlavID = ReadSQLField(SlaveRow, "id")
 
-		; Load the slave
+		; Load the slave. SlaveLink maintains the FirstSlave chain +
+		; NumberOfSlaves count.
 		Slave.ActorInstance = My_LoadActorInstance(SlavID, Null, Null,AccountID)
-		Slave\Leader = A
-		Slave\AIMode = AI_Pet
+		If Slave <> Null
+			SlaveLink(A, Slave)
+			Slave\AIMode = AI_Pet
+		EndIf
 
 		; Clean up
 		FreeSQLRow(SlaveRow)

src/Modules/ScriptingCommands.bb

@@ -939,17 +939,17 @@ Function BVM_SETLEADER(Param1%, Param2%)
 	Actor.ActorInstance = Object.ActorInstance(Param1%)
 	If Actor <> Null
 		If Actor\RNID = -1
-			; Remove current leader
+			; Remove current leader. SlaveUnlink maintains the
+			; FirstSlave chain + NumberOfSlaves on the old leader.
 			If Actor\Leader <> Null
-				Actor\Leader\NumberOfSlaves = Actor\Leader\NumberOfSlaves - 1
-				Actor\Leader = Null
+				SlaveUnlink(Actor)
 				Actor\AIMode = AI_Wait
 			EndIf
-			; Set new one, if any
+			; Set new one, if any. SlaveLink does the symmetric
+			; insert into Leader\FirstSlave + NumberOfSlaves increment.
 			Leader.ActorInstance = Object.ActorInstance(Param2%)
 			If Leader <> Null
-				Actor\Leader = Leader
-				Actor\Leader\NumberOfSlaves = Actor\Leader\NumberOfSlaves + 1
+				SlaveLink(Leader, Actor)
 				; Make sure it no longer belongs to any spawn point.
 				; Skip the spawn-count decrement if the actor's area
 				; lookup is Null (mid-warp / freed zone) -- the counter

src/Modules/ServerNet.bb

@@ -259,17 +259,18 @@ Function UpdateNetwork()
 									Name$ = Upper$(Trim$(Split$(Params$, 1, ",")))
 									Command$ = Trim$(Split$(Params$, 2, ","))
 									PetParams$ = Trim$(Split$(Params$, 3, ","))
-									Found = 0
-									For AI2.ActorInstance = Each ActorInstance
-										If AI2\Leader = AI
-											Found = Found + 1
-											If Upper$(AI2\Name$) = Name$ Or Name$ = "ALL"
-												CommandPet(AI2, Command$, PetParams$)
-												If Name$ <> "ALL" Then Exit
-											EndIf
-											If Found = AI\NumberOfSlaves Then Exit
+									; Walk AI's FirstSlave chain. The chain
+									; contains only AI's pets, so the explicit
+									; Leader filter and the NumberOfSlaves
+									; early-exit are no longer needed.
+									Local AI2.ActorInstance = AI\FirstSlave
+									While AI2 <> Null
+										If Upper$(AI2\Name$) = Name$ Or Name$ = "ALL"
+											CommandPet(AI2, Command$, PetParams$)
+											If Name$ <> "ALL" Then Exit
 										EndIf
-									Next
+										AI2 = AI2\NextSlave
+									Wend
 								EndIf
 							Case LanguageString$(LS_SCLeave)
 								LeaveParty(AI)
@@ -1720,15 +1721,15 @@ Function UpdateNetwork()
 						AIFrom.ActorInstance = FindActorInstanceFromRNID(M\FromID)
 						; Check that actor instance is valid (e.g. it isn't trying to change someone else's inventory)
 						If AI <> Null And AIFrom <> Null And SlotA >= 0 And SlotB >= 0
+							; Walk AIFrom's FirstSlave chain to check if the
+							; target actor (AI) is one of the sender's pets.
+							; Replaces a global ActorInstance walk filtered
+							; by `Leader = AIFrom`.
 							IsPet = False
-							Slaves = AIFrom\NumberOfSlaves
-							While Slaves > 0
-								For Slave.ActorInstance = Each ActorInstance
-									If Slave\Leader = AIFrom
-										Slaves = Slaves - 1
-										If Slave = AI Then IsPet = True : Exit
-									EndIf
-								Next
+							Local Slave.ActorInstance = AIFrom\FirstSlave
+							While Slave <> Null
+								If Slave = AI Then IsPet = True : Exit
+								Slave = Slave\NextSlave
 							Wend
 							If (AI = AIFrom Or IsPet = True) And (Amount = 0 Or Amount <= AI\Inventory\Amounts[SlotA])
 								If Left$(M\MessageData$, 1) = "S"