Add helper to check if a monster belongs to a player

Co-authored-by: Anders Jenbo <anders@jenbo.dk>

Source/controls/plrctrls.cpp

@@ -230,7 +230,7 @@ bool CanTargetMonster(const Monster &monster)
 {
 	if ((monster.flags & MFLAG_HIDDEN) != 0)
 		return false;
-	if (monster.ai == AI_GOLUM)
+	if (monster.isPlayerMinion())
 		return false;
 	if (monster.hitPoints >> 6 <= 0) // dead
 		return false;

Source/cursor.cpp

@@ -469,7 +469,7 @@ void CheckCursMove()
 				pcursmonst = -1;
 				cursPosition = { mx, my };
 			}
-			if (pcursmonst != -1 && (Monsters[pcursmonst].flags & MFLAG_GOLEM) != 0 && (Monsters[pcursmonst].flags & MFLAG_BERSERK) == 0) {
+			if (pcursmonst != -1 && Monsters[pcursmonst].isPlayerMinion()) {
 				pcursmonst = -1;
 			}
 			if (pcursmonst != -1) {
@@ -529,7 +529,7 @@ void CheckCursMove()
 			pcursmonst = -1;
 			cursPosition = { mx, my };
 		}
-		if (pcursmonst != -1 && (Monsters[pcursmonst].flags & MFLAG_GOLEM) != 0 && (Monsters[pcursmonst].flags & MFLAG_BERSERK) == 0) {
+		if (pcursmonst != -1 && Monsters[pcursmonst].isPlayerMinion()) {
 			pcursmonst = -1;
 		}
 	} else {
@@ -680,7 +680,7 @@ void CheckCursMove()
 		pcursitem = -1;
 		cursPosition = { mx, my };
 	}
-	if (pcursmonst != -1 && (Monsters[pcursmonst].flags & MFLAG_GOLEM) != 0 && (Monsters[pcursmonst].flags & MFLAG_BERSERK) == 0) {
+	if (pcursmonst != -1 && Monsters[pcursmonst].isPlayerMinion()) {
 		pcursmonst = -1;
 	}
 }

Source/loadsave.cpp

@@ -640,7 +640,7 @@ void LoadMonster(LoadHelper *file, Monster &monster)
 	file->Skip(1); // Alignment
 	monster.exp = file->NextLE<uint16_t>();
 
-	if ((monster.flags & MFLAG_GOLEM) != 0) // Don't skip for golems
+	if (monster.isPlayerMinion()) // Don't skip for golems
 		monster.toHit = file->NextLE<uint8_t>();
 	else
 		file->Skip(1); // Skip hit as it's already initialized

Source/missiles.cpp

@@ -393,9 +393,12 @@ void CheckMissileCol(Missile &missile, int minDamage, int maxDamage, bool isDama
 	if (mid > 0 || (mid != 0 && Monsters[abs(mid) - 1].mode == MonsterMode::Petrified)) {
 		mid = abs(mid) - 1;
 		if (missile.IsTrap()
-		    || (missile._micaster == TARGET_PLAYERS
-		        && (Monsters[missile._misource].flags & MFLAG_TARGETS_MONSTER) != 0
-		        && ((Monsters[mid].flags & MFLAG_GOLEM) != 0 || (Monsters[missile._misource].flags & MFLAG_BERSERK) != 0))) {
+		    || (missile._micaster == TARGET_PLAYERS && (                                           // or was fired by a monster and
+		            Monsters[mid].isPlayerMinion() != Monsters[missile._misource].isPlayerMinion() //  the monsters are on opposing factions
+		            || (Monsters[missile._misource].flags & MFLAG_BERSERK) != 0                    //  or the attacker is berserked
+		            || (Monsters[mid].flags & MFLAG_BERSERK) != 0                                  //  or the target is berserked
+		            ))) {
+			// then the missile can potentially hit this target
 			isMonsterHit = MonsterTrapHit(mid, minDamage, maxDamage, missile._midist, missile._mitype, isDamageShifted);
 		} else if (IsAnyOf(missile._micaster, TARGET_BOTH, TARGET_MONSTERS)) {
 			isMonsterHit = MonsterMHit(missile._misource, mid, minDamage, maxDamage, missile._midist, missile._mitype, isDamageShifted);
@@ -595,7 +598,7 @@ bool GuardianTryFireAt(Missile &missile, Point target)
 	if (mid < 0)
 		return false;
 	const Monster &monster = Monsters[mid];
-	if (monster.type().type == MT_GOLEM)
+	if (monster.isPlayerMinion())
 		return false;
 	if (monster.hitPoints >> 6 <= 0)
 		return false;
@@ -883,9 +886,8 @@ bool MonsterTrapHit(int monsterId, int mindam, int maxdam, int dist, missile_id
 		MonsterDeath(monster, monster.direction, true);
 	} else if (resist) {
 		PlayEffect(monster, 1);
-	} else {
-		if (monster.type().type != MT_GOLEM)
-			M_StartHit(monster, dam);
+	} else if (monster.type().type != MT_GOLEM) {
+		M_StartHit(monster, dam);
 	}
 	return true;
 }
@@ -1145,7 +1147,7 @@ void AddBerserk(Missile &missile, const AddMissileParameter &parameter)
 			    return false;
 
 		    const Monster &monster = Monsters[monsterId];
-		    if (monster.type().type == MT_GOLEM)
+		    if (monster.isPlayerMinion())
 			    return false;
 		    if ((monster.flags & MFLAG_BERSERK) != 0)
 			    return false;
@@ -3637,7 +3639,7 @@ void MI_Apoca(Missile &missile)
 			int mid = dMonster[k][j] - 1;
 			if (mid < 0)
 				continue;
-			if (Monsters[mid].type().type == MT_GOLEM)
+			if (Monsters[mid].isPlayerMinion())
 				continue;
 			if (TileHasAny(dPiece[k][j], TileProperties::Solid))
 				continue;

Source/monster.cpp

@@ -730,8 +730,7 @@ void UpdateEnemy(Monster &monster)
 			continue;
 		if (M_Talker(otherMonster) && otherMonster.talkMsg != TEXT_NONE)
 			continue;
-		bool isBerserked = (monster.flags & MFLAG_BERSERK) != 0 || (otherMonster.flags & MFLAG_BERSERK) != 0;
-		if ((monster.flags & MFLAG_GOLEM) != 0 && (otherMonster.flags & MFLAG_GOLEM) != 0 && !isBerserked) // prevent golems from fighting each other
+		if (monster.isPlayerMinion() && otherMonster.isPlayerMinion()) // prevent golems from fighting each other
 			continue;
 
 		int dist = otherMonster.position.tile.WalkingDistance(position);
@@ -987,7 +986,7 @@ void SpawnLoot(Monster &monster, bool sendmsg)
 		CreateMagicWeapon(monster.position.tile, ItemType::Staff, ICURS_WAR_STAFF, sendmsg, false);
 		CreateMagicWeapon(monster.position.tile, ItemType::Bow, ICURS_LONG_WAR_BOW, sendmsg, false);
 		CreateSpellBook(monster.position.tile, SPL_APOCA, sendmsg, false);
-	} else if (monster.type().type != MT_GOLEM) {
+	} else if (!monster.isPlayerMinion()) {
 		SpawnItem(monster, monster.position.tile, sendmsg);
 	}
 }
@@ -1057,7 +1056,7 @@ void HitMonster(Monster &monster, int dam)
 
 void MonsterHitMonster(Monster &attacker, Monster &target, int dam)
 {
-	if (attacker.type().type == MT_GOLEM)
+	if (attacker.isPlayerMinion())
 		target.whoHit |= 1 << attacker.getId(); // really the id the player who controls this golem
 
 	if (IsAnyOf(target.type().type, MT_SNEAK, MT_STALKER, MT_UNSEEN, MT_ILLWEAV) || dam >> 6 >= target.level + 3) {
@@ -1072,7 +1071,7 @@ void StartDeathFromMonster(Monster &attacker, Monster &target)
 	delta_kill_monster(target, target.position.tile, *MyPlayer);
 	NetSendCmdLocParam1(false, CMD_MONSTDEATH, target.position.tile, target.getId());
 
-	if (attacker.type().type == MT_GOLEM)
+	if (attacker.isPlayerMinion())
 		target.whoHit |= 1 << attacker.getId(); // really the id the player who controls this golem
 
 	Direction md = GetDirection(target.position.tile, attacker.position.tile);
@@ -1911,7 +1910,7 @@ bool AiPlanPath(Monster &monster)
 			return false;
 		if (IsNoneOf(monster.goal, MonsterGoal::Normal, MonsterGoal::Move, MonsterGoal::Attack))
 			return false;
-		if (monster.position.tile.x == 1 && monster.position.tile.y == 0)
+		if (monster.position.tile == GolemHoldingCell)
 			return false;
 	}
 
@@ -3875,7 +3874,7 @@ void M_StartHit(Monster &monster, const Player &player, int dam)
 
 void MonsterDeath(Monster &monster, Direction md, bool sendmsg)
 {
-	if (monster.type().type != MT_GOLEM)
+	if (!monster.isPlayerMinion())
 		AddPlrMonstExper(monster.level, monster.exp, monster.whoHit);
 
 	MonsterKillCounts[monster.type().type]++;
@@ -4844,6 +4843,12 @@ bool Monster::isResistant(missile_id missileType) const
 	return false;
 }
 
+bool Monster::isPlayerMinion() const
+{
+	// This could be HasAnyOf(GOLEM) && HasNoneOf(BERSERK), I think referencing the type and player index is more robust though
+	return type().type == MT_GOLEM && getId() < sizeof(Players) / sizeof(Players[0]);
+}
+
 bool Monster::isPossibleToHit() const
 {
 	return !(hitPoints >> 6 <= 0

Source/monster.h

@@ -338,6 +338,12 @@ struct Monster { // note: missing field _mAFNum
 	bool isWalking() const;
 	bool isImmune(missile_id mitype) const;
 	bool isResistant(missile_id mitype) const;
+
+	/**
+	 * Is this a player's golem?
+	 */
+	[[nodiscard]] bool isPlayerMinion() const;
+
 	bool isPossibleToHit() const;
 
 	[[nodiscard]] bool isUnique() const