diff --git a/src/Modules/Actors.bb b/src/Modules/Actors.bb index cac0c671..d265b447 100644 --- a/src/Modules/Actors.bb +++ b/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 diff --git a/src/Modules/GameServer.bb b/src/Modules/GameServer.bb index 1303465d..3baccc53 100644 --- a/src/Modules/GameServer.bb +++ b/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 diff --git a/src/Modules/MySQL.bb b/src/Modules/MySQL.bb index ef6b719c..af6a1488 100644 --- a/src/Modules/MySQL.bb +++ b/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) diff --git a/src/Modules/ScriptingCommands.bb b/src/Modules/ScriptingCommands.bb index 8b331c3d..dc7144ea 100644 --- a/src/Modules/ScriptingCommands.bb +++ b/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 diff --git a/src/Modules/ServerNet.bb b/src/Modules/ServerNet.bb index f4ad0429..aca2233a 100644 --- a/src/Modules/ServerNet.bb +++ b/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" diff --git a/src/Tests/Modules/SlaveChainTest.bb b/src/Tests/Modules/SlaveChainTest.bb new file mode 100644 index 00000000..9e6401b7 --- /dev/null +++ b/src/Tests/Modules/SlaveChainTest.bb @@ -0,0 +1,322 @@ +Strict +; EnableGC intentionally omitted -- per the +; feedback_blitzforge_test_handle_object_gc memory, Type-heavy chain +; tests stack-overflow under GC tracing. Strict-only is sufficient +; for chain-correctness assertions; no Handle/Object round-trip is +; needed. + +; Regression test pinning the per-leader slave chain in Actors.bb +; (Phase 4 of the ActorByRNID multi-iteration initiative; Phase 1 +; was PR #282, Phase 3 was PR #283). +; +; The chain replaces 6 `For Each ActorInstance / If X\Leader = leader` +; walks across Actors.bb (save / FreeActorInstanceSlaves), GameServer.bb +; (pet aggro broadcast), MySQL.bb (save), ServerNet.bb (/pet +; command, inventory pet-validation walk). +; +; Actors.bb's SlaveLink / SlaveUnlink can't be exercised directly +; because ActorInstance pulls the whole network/world graph. +; Replicated-gate pattern: rebuild the chain logic against a tiny +; mock Type whose field shape matches ActorInstance's +; Leader / FirstSlave / NextSlave / NumberOfSlaves layout. Any +; production change to the helpers MUST update this file. + +Type MockActor + Field Name$ + Field Leader.MockActor + Field FirstSlave.MockActor + Field NextSlave.MockActor + Field NumberOfSlaves% +End Type + +; Replicates Actors.bb's SlaveLink: head-insert into Leader's chain, +; +1 on Leader\NumberOfSlaves. Re-links if Slave already has a +; different Leader. +Function MockSlaveLink(Leader.MockActor, Slave.MockActor) + If Leader = Null Or Slave = Null Then Return + If Slave\Leader = Leader Then Return + If Slave\Leader <> Null Then MockSlaveUnlink(Slave) + Slave\Leader = Leader + Slave\NextSlave = Leader\FirstSlave + Leader\FirstSlave = Slave + Leader\NumberOfSlaves = Leader\NumberOfSlaves + 1 +End Function + +; Replicates Actors.bb's SlaveUnlink: walk-to-find-predecessor splice +; on the leader's chain, -1 on NumberOfSlaves, clears Slave\Leader. +Function MockSlaveUnlink(Slave.MockActor) + If Slave = Null Then Return + Local Leader.MockActor = Slave\Leader + If Leader = Null Then Return + If Leader\FirstSlave = Slave + Leader\FirstSlave = Slave\NextSlave + Else + Local Prev.MockActor = 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 + +Function ChainLen%(L.MockActor) + Local N% = 0 + Local Cur.MockActor = L\FirstSlave + While Cur <> Null + N = N + 1 + Cur = Cur\NextSlave + Wend + Return N +End Function + +Function ChainContains%(L.MockActor, S.MockActor) + Local Cur.MockActor = L\FirstSlave + While Cur <> Null + If Cur = S Then Return True + Cur = Cur\NextSlave + Wend + Return False +End Function + +; ==================================================================== +; Link: head-insert, NumberOfSlaves bookkeeping, idempotency +; ==================================================================== + +Test testLinkOneSlave() + Local L.MockActor = New MockActor() : L\Name = "L" + Local S.MockActor = New MockActor() : S\Name = "S" + MockSlaveLink(L, S) + Assert(S\Leader = L) + Assert(L\FirstSlave = S) + Assert(L\NumberOfSlaves = 1) + Assert(ChainLen%(L) = 1) +End Test + +Test testLinkManyHeadInsertOrder() + Local L.MockActor = New MockActor() : L\Name = "L" + Local A.MockActor = New MockActor() : A\Name = "A" + Local B.MockActor = New MockActor() : B\Name = "B" + Local C.MockActor = New MockActor() : C\Name = "C" + MockSlaveLink(L, A) + MockSlaveLink(L, B) + MockSlaveLink(L, C) + ; Head-insert: newest first -- C -> B -> A. + Assert(L\FirstSlave = C) + Assert(C\NextSlave = B) + Assert(B\NextSlave = A) + Assert(A\NextSlave = Null) + Assert(L\NumberOfSlaves = 3) +End Test + +Test testLinkIdempotentNoDoubleCount() + Local L.MockActor = New MockActor() : L\Name = "L" + Local S.MockActor = New MockActor() : S\Name = "S" + MockSlaveLink(L, S) + MockSlaveLink(L, S) + MockSlaveLink(L, S) + ; Production helper short-circuits when Slave\Leader == Leader. + ; Count must stay at 1. + Assert(L\NumberOfSlaves = 1) + Assert(ChainLen%(L) = 1) +End Test + +Test testLinkReassignsToNewLeader() + Local L1.MockActor = New MockActor() : L1\Name = "L1" + Local L2.MockActor = New MockActor() : L2\Name = "L2" + Local S.MockActor = New MockActor() : S\Name = "S" + MockSlaveLink(L1, S) + Assert(L1\NumberOfSlaves = 1) + MockSlaveLink(L2, S) + ; Slave should detach from L1 and attach to L2. + Assert(L1\NumberOfSlaves = 0) + Assert(L1\FirstSlave = Null) + Assert(L2\NumberOfSlaves = 1) + Assert(L2\FirstSlave = S) + Assert(S\Leader = L2) +End Test + +Test testLinkNullSlaveIsNoOp() + Local L.MockActor = New MockActor() : L\Name = "L" + MockSlaveLink(L, Null) + Assert(L\NumberOfSlaves = 0) + Assert(L\FirstSlave = Null) +End Test + +Test testLinkNullLeaderIsNoOp() + Local S.MockActor = New MockActor() : S\Name = "S" + MockSlaveLink(Null, S) + Assert(S\Leader = Null) +End Test + +; ==================================================================== +; Unlink: head / middle / tail removal, NumberOfSlaves bookkeeping +; ==================================================================== + +Test testUnlinkHead() + Local L.MockActor = New MockActor() : L\Name = "L" + Local A.MockActor = New MockActor() : A\Name = "A" + Local B.MockActor = New MockActor() : B\Name = "B" + Local C.MockActor = New MockActor() : C\Name = "C" + MockSlaveLink(L, A) : MockSlaveLink(L, B) : MockSlaveLink(L, C) + ; Chain: C -> B -> A. Unlink C (head). + MockSlaveUnlink(C) + Assert(L\FirstSlave = B) + Assert(L\NumberOfSlaves = 2) + Assert(C\Leader = Null) + Assert(C\NextSlave = Null) +End Test + +Test testUnlinkMiddle() + Local L.MockActor = New MockActor() : L\Name = "L" + Local A.MockActor = New MockActor() : A\Name = "A" + Local B.MockActor = New MockActor() : B\Name = "B" + Local C.MockActor = New MockActor() : C\Name = "C" + MockSlaveLink(L, A) : MockSlaveLink(L, B) : MockSlaveLink(L, C) + ; Chain: C -> B -> A. Unlink B (middle). + MockSlaveUnlink(B) + Assert(L\FirstSlave = C) + Assert(C\NextSlave = A) + Assert(A\NextSlave = Null) + Assert(L\NumberOfSlaves = 2) + Assert(B\Leader = Null) + Assert(B\NextSlave = Null) +End Test + +Test testUnlinkTail() + Local L.MockActor = New MockActor() : L\Name = "L" + Local A.MockActor = New MockActor() : A\Name = "A" + Local B.MockActor = New MockActor() : B\Name = "B" + Local C.MockActor = New MockActor() : C\Name = "C" + MockSlaveLink(L, A) : MockSlaveLink(L, B) : MockSlaveLink(L, C) + ; Chain: C -> B -> A. Unlink A (tail). + MockSlaveUnlink(A) + Assert(L\FirstSlave = C) + Assert(C\NextSlave = B) + Assert(B\NextSlave = Null) + Assert(L\NumberOfSlaves = 2) +End Test + +Test testUnlinkSingleElement() + Local L.MockActor = New MockActor() : L\Name = "L" + Local S.MockActor = New MockActor() : S\Name = "S" + MockSlaveLink(L, S) + MockSlaveUnlink(S) + Assert(L\FirstSlave = Null) + Assert(L\NumberOfSlaves = 0) + Assert(S\Leader = Null) +End Test + +Test testUnlinkSlaveWithNoLeaderIsNoOp() + Local L.MockActor = New MockActor() : L\Name = "L" + Local S.MockActor = New MockActor() : S\Name = "S" + ; S has no leader. + MockSlaveUnlink(S) + Assert(S\Leader = Null) + Assert(L\NumberOfSlaves = 0) +End Test + +Test testUnlinkNullIsNoOp() + MockSlaveUnlink(Null) + ; No crash, no error. +End Test + +; ==================================================================== +; Multi-leader cases +; ==================================================================== + +Test testManyLeadersWithChains() + Local L1.MockActor = New MockActor() : L1\Name = "L1" + Local L2.MockActor = New MockActor() : L2\Name = "L2" + Local L1S1.MockActor = New MockActor() : L1S1\Name = "L1S1" + Local L1S2.MockActor = New MockActor() : L1S2\Name = "L1S2" + Local L2S1.MockActor = New MockActor() : L2S1\Name = "L2S1" + MockSlaveLink(L1, L1S1) : MockSlaveLink(L1, L1S2) + MockSlaveLink(L2, L2S1) + ; L1 chain: L1S2 -> L1S1. L2 chain: L2S1. + Assert(L1\NumberOfSlaves = 2) + Assert(L2\NumberOfSlaves = 1) + Assert(ChainContains%(L1, L1S1) = True) + Assert(ChainContains%(L1, L1S2) = True) + Assert(ChainContains%(L2, L2S1) = True) + ; Chains are disjoint. + Assert(ChainContains%(L1, L2S1) = False) + Assert(ChainContains%(L2, L1S1) = False) +End Test + +Test testUnlinkAllInLeaderChain() + Local L.MockActor = New MockActor() : L\Name = "L" + Local A.MockActor = New MockActor() : A\Name = "A" + Local B.MockActor = New MockActor() : B\Name = "B" + Local C.MockActor = New MockActor() : C\Name = "C" + MockSlaveLink(L, A) : MockSlaveLink(L, B) : MockSlaveLink(L, C) + MockSlaveUnlink(B) + MockSlaveUnlink(C) + MockSlaveUnlink(A) + Assert(L\FirstSlave = Null) + Assert(L\NumberOfSlaves = 0) +End Test + +; ==================================================================== +; NumberOfSlaves invariant: must always equal chain length +; ==================================================================== + +; ==================================================================== +; Load-path invariant: the saved NumberOfSlaves count must be reset +; to 0 before the loop that re-links slaves -- otherwise SlaveLink +; increments cause double-counting (saved value + per-link +; increments). This pins the requirement that ReadActorInstance and +; My_LoadActorInstance both reset before the link loop. See the +; MySQL.bb fix from PR #287's quality-gate review. +; ==================================================================== + +Test testLoadPathWithoutResetDoublesCount() + Local L.MockActor = New MockActor() : L\Name = "L" + Local S1.MockActor = New MockActor() : S1\Name = "S1" + Local S2.MockActor = New MockActor() : S2\Name = "S2" + ; Simulate the saved-from-disk state: NumberOfSlaves carries the + ; previously-saved count. If the load loop calls SlaveLink without + ; resetting, every link increment piles on top. + L\NumberOfSlaves = 2 + MockSlaveLink(L, S1) + MockSlaveLink(L, S2) + ; Bug: count is 4 (2 saved + 2 increments) but the chain only has + ; 2 actual slaves. Pin this divergence so a future load-path + ; refactor that omits the reset trips this test. + Assert(L\NumberOfSlaves = 4) + Assert(ChainLen%(L) = 2) +End Test + +Test testLoadPathWithResetMatchesChainLength() + Local L.MockActor = New MockActor() : L\Name = "L" + Local S1.MockActor = New MockActor() : S1\Name = "S1" + Local S2.MockActor = New MockActor() : S2\Name = "S2" + ; Same scenario as above, but apply the canonical load-path fix: + ; reset BEFORE the link loop. Count and chain length agree. + L\NumberOfSlaves = 2 ; saved-from-disk value + L\NumberOfSlaves = 0 ; canonical reset (the fix) + MockSlaveLink(L, S1) + MockSlaveLink(L, S2) + Assert(L\NumberOfSlaves = 2) + Assert(ChainLen%(L) = 2) + Assert(L\NumberOfSlaves = ChainLen%(L)) +End Test + +Test testNumberOfSlavesMatchesChainLengthAfterChurn() + Local L.MockActor = New MockActor() : L\Name = "L" + Local A.MockActor = New MockActor() : A\Name = "A" + Local B.MockActor = New MockActor() : B\Name = "B" + Local C.MockActor = New MockActor() : C\Name = "C" + Local D.MockActor = New MockActor() : D\Name = "D" + MockSlaveLink(L, A) : Assert(L\NumberOfSlaves = ChainLen%(L)) + MockSlaveLink(L, B) : Assert(L\NumberOfSlaves = ChainLen%(L)) + MockSlaveLink(L, C) : Assert(L\NumberOfSlaves = ChainLen%(L)) + MockSlaveUnlink(B) : Assert(L\NumberOfSlaves = ChainLen%(L)) + MockSlaveLink(L, D) : Assert(L\NumberOfSlaves = ChainLen%(L)) + MockSlaveUnlink(A) : Assert(L\NumberOfSlaves = ChainLen%(L)) + MockSlaveUnlink(C) : Assert(L\NumberOfSlaves = ChainLen%(L)) + MockSlaveUnlink(D) : Assert(L\NumberOfSlaves = ChainLen%(L)) + Assert(L\NumberOfSlaves = 0) +End Test