Tidy up ActivateSkeleton and take monster by reference (#5080)

* Take reference in OperateSarcophagus
* Use monster reference instead of pointer in ActivateSkeleton
* Clean up ActivateSkeleton

Source/levels/themes.cpp

@@ -582,7 +582,8 @@ void SpawnObjectOrSkeleton(unsigned frequency, _object_id objectType, Point tile
 		AddObject(objectType, tile);
 	} else {
 		Monster *skeleton = PreSpawnSkeleton();
-		SpawnSkeleton(skeleton, tile);
+		if (skeleton != nullptr)
+			ActivateSkeleton(*skeleton, tile);
 	}
 }
 } // namespace
@@ -607,7 +608,8 @@ void Theme_SkelRoom(int t)
 
 	{
 		Monster *skeleton = PreSpawnSkeleton();
-		SpawnSkeleton(skeleton, { xp, yp - 1 });
+		if (skeleton != nullptr)
+			ActivateSkeleton(*skeleton, { xp, yp - 1 });
 	}
 
 	SpawnObjectOrSkeleton(monstrnd[leveltype - 1], OBJ_BANNERR, { xp + 1, yp - 1 });
@@ -620,7 +622,8 @@ void Theme_SkelRoom(int t)
 
 	{
 		Monster *skeleton = PreSpawnSkeleton();
-		SpawnSkeleton(skeleton, { xp, yp + 1 });
+		if (skeleton != nullptr)
+			ActivateSkeleton(*skeleton, { xp, yp + 1 });
 	}
 
 	SpawnObjectOrSkeleton(monstrnd[leveltype - 1], OBJ_BANNERL, { xp + 1, yp + 1 });

Source/monster.cpp

@@ -4644,56 +4644,40 @@ bool IsGoat(_monster_id mt)
 	    MT_NGOATBW, MT_BGOATBW, MT_RGOATBW, MT_GGOATBW);
 }
 
-bool SpawnSkeleton(Monster *monster, Point position)
+void ActivateSkeleton(Monster &monster, Point position)
 {
-	if (monster == nullptr)
-		return false;
-
 	if (IsTileAvailable(position)) {
-		Direction dir = GetDirection(position, position); // TODO useless calculation
-		ActivateSpawn(*monster, position, dir);
-		return true;
+		ActivateSpawn(monster, position, Direction::SouthWest);
+		return;
 	}
 
-	bool monstok[3][3];
+	constexpr std::array<Direction, 8> spawnDirections {
+		Direction::North, Direction::NorthEast, Direction::East, Direction::NorthWest, Direction::SouthEast, Direction::West, Direction::SouthWest, Direction::South
+	};
+	std::bitset<8> spawnOk;
 
-	bool savail = false;
-	int yy = 0;
-	for (int j = position.y - 1; j <= position.y + 1; j++) {
-		int xx = 0;
-		for (int k = position.x - 1; k <= position.x + 1; k++) {
-			monstok[xx][yy] = IsTileAvailable({ k, j });
-			savail = savail || monstok[xx][yy];
-			xx++;
-		}
-		yy++;
-	}
-	if (!savail) {
-		return false;
+	for (size_t i = 0; i < spawnDirections.size(); i++) {
+		if (IsTileAvailable(position + spawnDirections[i]))
+			spawnOk.set(i);
 	}
+	if (spawnOk.none())
+		return;
 
-	int rs = GenerateRnd(15) + 1;
-	int x2 = 0;
-	int y2 = 0;
-	while (rs > 0) {
-		if (monstok[x2][y2])
-			rs--;
-		if (rs > 0) {
-			x2++;
-			if (x2 == 3) {
-				x2 = 0;
-				y2++;
-				if (y2 == 3)
-					y2 = 0;
-			}
-		}
-	}
+	// this is used in the following loop to find the nth set bit.
+	int spawnChoice = GenerateRnd(15) % spawnOk.count();
 
-	Point spawn = position + Displacement { x2 - 1, y2 - 1 };
-	Direction dir = GetDirection(spawn, position);
-	ActivateSpawn(*monster, spawn, dir);
+	for (size_t i = 0; i < spawnOk.size(); i++) {
+		if (!spawnOk.test(i))
+			continue;
 
-	return true;
+		if (spawnChoice > 0) {
+			spawnChoice--;
+			continue;
+		}
+
+		ActivateSpawn(monster, position + spawnDirections[i], Opposite(spawnDirections[i]));
+		return;
+	}
 }
 
 Monster *PreSpawnSkeleton()

Source/monster.h

@@ -403,7 +403,12 @@ Monster *MonsterAtPosition(Point position);
 bool IsTileAvailable(const Monster &monster, Point position);
 bool IsSkel(_monster_id mt);
 bool IsGoat(_monster_id mt);
-bool SpawnSkeleton(Monster *monster, Point position);
+/**
+ * @brief Reveals a monster that was hiding in a container
+ * @param monster instance returned from a previous call to PreSpawnSkeleton
+ * @param position tile to try spawn the monster at, neighboring tiles will be used as a fallback
+ */
+void ActivateSkeleton(Monster &monster, Point position);
 Monster *PreSpawnSkeleton();
 void TalktoMonster(Monster &monster);
 void SpawnGolem(Player &player, Monster &golem, Point position, Missile &missile);

Source/objects.cpp

@@ -2492,23 +2492,23 @@ void OperateTrapLever(Object &flameLever)
 	}
 }
 
-void OperateSarc(int i, bool sendMsg, bool sendLootMsg)
+void OperateSarcophagus(Object &sarcophagus, bool sendMsg, bool sendLootMsg)
 {
-	if (Objects[i]._oSelFlag == 0) {
+	if (sarcophagus._oSelFlag == 0) {
 		return;
 	}
 
-	PlaySfxLoc(IS_SARC, Objects[i].position);
-	Objects[i]._oSelFlag = 0;
-	Objects[i]._oAnimFlag = true;
-	Objects[i]._oAnimDelay = 3;
-	SetRndSeed(Objects[i]._oRndSeed);
-	if (Objects[i]._oVar1 <= 2)
-		CreateRndItem(Objects[i].position, false, sendLootMsg, false);
-	if (Objects[i]._oVar1 >= 8 && Objects[i]._oVar2 >= 0)
-		SpawnSkeleton(&Monsters[Objects[i]._oVar2], Objects[i].position);
+	PlaySfxLoc(IS_SARC, sarcophagus.position);
+	sarcophagus._oSelFlag = 0;
+	sarcophagus._oAnimFlag = true;
+	sarcophagus._oAnimDelay = 3;
+	SetRndSeed(sarcophagus._oRndSeed);
+	if (sarcophagus._oVar1 <= 2)
+		CreateRndItem(sarcophagus.position, false, sendLootMsg, false);
+	if (sarcophagus._oVar1 >= 8 && sarcophagus._oVar2 >= 0)
+		ActivateSkeleton(Monsters[sarcophagus._oVar2], sarcophagus.position);
 	if (sendMsg)
-		NetSendCmdParam1(false, CMD_OPERATEOBJ, i);
+		NetSendCmdParam1(false, CMD_OPERATEOBJ, sarcophagus.GetId());
 }
 
 void OperateL2Door(const Player &player, int i)
@@ -3855,7 +3855,7 @@ void BreakBarrel(const Player &player, Object &barrel, bool forcebreak, bool sen
 				CreateRndItem(barrel.position, false, sendmsg, false);
 		}
 		if (barrel._oVar2 >= 8 && barrel._oVar4 >= 0)
-			SpawnSkeleton(&Monsters[barrel._oVar4], barrel.position);
+			ActivateSkeleton(Monsters[barrel._oVar4], barrel.position);
 	}
 	if (&player == MyPlayer) {
 		NetSendCmdLoc(MyPlayerId, false, CMD_BREAKOBJ, barrel.position);
@@ -4877,7 +4877,7 @@ void OperateObject(Player &player, int i, bool teleFlag)
 		break;
 	case OBJ_SARC:
 	case OBJ_L5SARC:
-		OperateSarc(i, sendmsg, sendmsg);
+		OperateSarcophagus(object, sendmsg, sendmsg);
 		break;
 	case OBJ_FLAMELVR:
 		OperateTrapLever(object);
@@ -5069,7 +5069,7 @@ void SyncOpObject(Player &player, int cmd, int i)
 		break;
 	case OBJ_SARC:
 	case OBJ_L5SARC:
-		OperateSarc(i, sendmsg, false);
+		OperateSarcophagus(object, sendmsg, false);
 		break;
 	case OBJ_BLINDBOOK:
 	case OBJ_BLOODBOOK: