From b60bdf8b9bd65fc91a14e89d9eba182968cda015 Mon Sep 17 00:00:00 2001
From: ephphatha <ephphatha@thelettereph.com>
Date: Mon, 4 Jul 2022 00:29:18 +1000
Subject: [PATCH] Unset leader of minions when the leader dies

---
 Source/monster.cpp | 61 +++++++++++++++++++++++++++-------------------
 Source/monster.h   |  4 +--
 Source/msg.cpp     |  2 +-
 3 files changed, 39 insertions(+), 28 deletions(-)

diff --git a/Source/monster.cpp b/Source/monster.cpp
index 07efbfb4d..3e438c43c 100644
--- a/Source/monster.cpp
+++ b/Source/monster.cpp
@@ -366,9 +366,7 @@ void PlaceGroup(int mtype, unsigned num, UniqueMonsterPack uniqueMonsterPack, Mo
 				minion.intelligence = leader->intelligence;
 
 				if (uniqueMonsterPack == UniqueMonsterPack::Leashed) {
-					minion.setLeader(*leader);
-					minion.leaderRelation = LeaderRelation::Leashed;
-					minion.ai = leader->ai;
+					minion.setLeader(leader);
 				}
 
 				if (minion.ai != AI_GARG) {
@@ -1646,11 +1644,25 @@ bool MonsterGotHit(Monster &monster)
 	return false;
 }
 
-void MonsterDeath(int monsterId)
+void ReleaseMinions(const Monster &leader)
 {
-	assert(static_cast<size_t>(monsterId) < MaxMonsters);
-	auto &monster = Monsters[monsterId];
+	for (int j = 0; j < ActiveMonsterCount; j++) {
+		auto &minion = Monsters[ActiveMonsters[j]];
+		if (minion.leaderRelation == LeaderRelation::Leashed && minion.getLeader() == &leader) {
+			minion.setLeader(nullptr);
+		}
+	}
+}
 
+void ShrinkLeaderPacksize(const Monster &monster)
+{
+	if (monster.leaderRelation == LeaderRelation::Leashed) {
+		monster.getLeader()->packSize--;
+	}
+}
+
+void MonsterDeath(Monster &monster)
+{
 	monster.var1++;
 	if (monster.type().type == MT_DIABLO) {
 		if (monster.position.tile.x < ViewPosition.x) {
@@ -1676,7 +1688,7 @@ void MonsterDeath(int monsterId)
 		dMonster[monster.position.tile.x][monster.position.tile.y] = 0;
 		monster.isInvalid = true;
 
-		M_UpdateLeader(monsterId);
+		M_UpdateRelations(monster);
 	}
 }
 
@@ -2344,8 +2356,7 @@ void ScavengerAi(int monsterId)
 		return;
 	if (monster.hitPoints < (monster.maxHitPoints / 2) && monster.goal != MonsterGoal::Healing) {
 		if (monster.leaderRelation != LeaderRelation::None) {
-			if (monster.leaderRelation == LeaderRelation::Leashed)
-				monster.getLeader()->packSize--;
+			ShrinkLeaderPacksize(monster);
 			monster.leaderRelation = LeaderRelation::None;
 		}
 		monster.goal = MonsterGoal::Healing;
@@ -3384,7 +3395,7 @@ bool UpdateModeStance(int monsterId)
 	case MonsterMode::HitRecovery:
 		return MonsterGotHit(monster);
 	case MonsterMode::Death:
-		MonsterDeath(monsterId);
+		MonsterDeath(monster);
 		return false;
 	case MonsterMode::SpecialMeleeAttack:
 		return MonsterSpecialAttack(monsterId);
@@ -3998,20 +4009,10 @@ void M_SyncStartKill(int monsterId, Point position, int pnum)
 	StartMonsterDeath(monster, pnum, false);
 }
 
-void M_UpdateLeader(int monsterId)
+void M_UpdateRelations(const Monster &monster)
 {
-	assert(static_cast<size_t>(monsterId) < MaxMonsters);
-	auto &monster = Monsters[monsterId];
-
-	for (size_t j = 0; j < ActiveMonsterCount; j++) {
-		auto &minion = Monsters[ActiveMonsters[j]];
-		if (minion.leaderRelation == LeaderRelation::Leashed && minion.getLeader() == &monster)
-			minion.leaderRelation = LeaderRelation::None;
-	}
-
-	if (monster.leaderRelation == LeaderRelation::Leashed) {
-		monster.getLeader()->packSize--;
-	}
+	ReleaseMinions(monster);
+	ShrinkLeaderPacksize(monster);
 }
 
 void DoEnding()
@@ -4851,9 +4852,19 @@ Monster *Monster::getLeader() const
 	return &Monsters[leader];
 }
 
-void Monster::setLeader(const Monster &leader)
+void Monster::setLeader(const Monster *leader)
 {
-	this->leader = leader.getId();
+	if (leader == nullptr) {
+		// really we should update this->leader to NoLeader to avoid leaving a dangling reference to a dead monster
+		// when passed nullptr. So that buffed minions are drawn with a distinct colour in monhealthbar we leave the
+		// reference and hope that no code tries to modify the leader through this instance later.
+		leaderRelation = LeaderRelation::None;
+		return;
+	}
+
+	this->leader = leader->getId();
+	leaderRelation = LeaderRelation::Leashed;
+	ai = leader->ai;
 }
 
 void Monster::checkStandAnimationIsLoaded(Direction mdir)
diff --git a/Source/monster.h b/Source/monster.h
index 7f5ad9521..e39bfea1e 100644
--- a/Source/monster.h
+++ b/Source/monster.h
@@ -292,7 +292,7 @@ struct Monster { // note: missing field _mAFNum
 	[[nodiscard]] size_t getId() const;
 
 	[[nodiscard]] Monster *getLeader() const;
-	void setLeader(const Monster &leader);
+	void setLeader(const Monster *leader);
 
 	/**
 	 * @brief Is the monster currently walking?
@@ -330,7 +330,7 @@ void M_StartHit(Monster &monster, int pnum, int dam);
 void StartMonsterDeath(Monster &monster, int pnum, bool sendmsg);
 void M_StartKill(int monsterId, int pnum);
 void M_SyncStartKill(int monsterId, Point position, int pnum);
-void M_UpdateLeader(int monsterId);
+void M_UpdateRelations(const Monster &monster);
 void DoEnding();
 void PrepDoEnding();
 bool Walk(Monster &monster, Direction md);
diff --git a/Source/msg.cpp b/Source/msg.cpp
index d5f2f2d6b..32472a801 100644
--- a/Source/msg.cpp
+++ b/Source/msg.cpp
@@ -2471,7 +2471,7 @@ void DeltaLoadLevel()
 					}
 				}
 				monster.isInvalid = true;
-				M_UpdateLeader(i);
+				M_UpdateRelations(monster);
 			} else {
 				decode_enemy(monster, deltaLevel.monster[i]._menemy);
 				if (monster.position.tile != Point { 0, 0 } && monster.position.tile != GolemHoldingCell)