chore: Move static Player functions and internal linkage to manager class (#1412)

* Moving and organizing Player code

- Move code to CharacterComponent
- Remove extraneous interfaces
- Simplify some code greatly
- Change some types to return and take in const ref (only structs larger than 8 bytes benefit from this change.)
- Update code to use CharacterComponent for sending to zone instead of Player*.

* Moving and organizing Player code

- Move code to CharacterComponent
- Remove extraneous interfaces
- Simplify some code greatly
- Change some types to return and take in const ref (only structs larger than 8 bytes benefit from this change.)
- Update code to use CharacterComponent for sending to zone instead of Player*.
- Remove static storage container (static containers can be destroyed before exit/terminate handler executes)

* remove player cast

* Remove extra includes

* Add a player manager

Used for the static Player functions.  Further removes stuff from the Player class/file.

dGame/CMakeLists.txt

@@ -3,6 +3,7 @@ set(DGAME_SOURCES "Character.cpp"
 		"EntityManager.cpp"
 		"LeaderboardManager.cpp"
 		"Player.cpp"
+		"PlayerManager.cpp"
 		"TeamManager.cpp"
 		"TradingManager.cpp"
 		"User.cpp"

dGame/EntityManager.cpp

@@ -23,6 +23,7 @@
 #include "eGameMasterLevel.h"
 #include "eReplicaComponentType.h"
 #include "eReplicaPacketType.h"
+#include "PlayerManager.h"
 
 // Configure which zones have ghosting disabled, mostly small worlds.
 std::vector<LWOMAPID> EntityManager::m_GhostingExcludedZones = {
@@ -187,7 +188,7 @@ void EntityManager::SerializeEntities() {
 		entity->WriteComponents(&stream, eReplicaPacketType::SERIALIZATION);
 
 		if (entity->GetIsGhostingCandidate()) {
-			for (auto* player : Player::GetAllPlayers()) {
+			for (auto* player : PlayerManager::GetAllPlayers()) {
 				if (player->IsObserved(toSerialize)) {
 					Game::server->Send(&stream, player->GetSystemAddress(), false);
 				}
@@ -376,7 +377,7 @@ void EntityManager::ConstructEntity(Entity* entity, const SystemAddress& sysAddr
 		if (skipChecks) {
 			Game::server->Send(&stream, UNASSIGNED_SYSTEM_ADDRESS, true);
 		} else {
-			for (auto* player : Player::GetAllPlayers()) {
+			for (auto* player : PlayerManager::GetAllPlayers()) {
 				if (player->GetPlayerReadyForUpdates()) {
 					Game::server->Send(&stream, player->GetSystemAddress(), false);
 				} else {
@@ -405,7 +406,7 @@ void EntityManager::ConstructAllEntities(const SystemAddress& sysAddr) {
 		}
 	}
 
-	UpdateGhosting(Player::GetPlayer(sysAddr));
+	UpdateGhosting(PlayerManager::GetPlayer(sysAddr));
 }
 
 void EntityManager::DestructEntity(Entity* entity, const SystemAddress& sysAddr) {
@@ -418,7 +419,7 @@ void EntityManager::DestructEntity(Entity* entity, const SystemAddress& sysAddr)
 
 	Game::server->Send(&stream, sysAddr, sysAddr == UNASSIGNED_SYSTEM_ADDRESS);
 
-	for (auto* player : Player::GetAllPlayers()) {
+	for (auto* player : PlayerManager::GetAllPlayers()) {
 		if (!player->GetPlayerReadyForUpdates()) {
 			player->RemoveLimboConstruction(entity->GetObjectID());
 		}
@@ -465,7 +466,7 @@ void EntityManager::QueueGhostUpdate(LWOOBJID playerID) {
 
 void EntityManager::UpdateGhosting() {
 	for (const auto playerID : m_PlayersToUpdateGhosting) {
-		auto* player = Player::GetPlayer(playerID);
+		auto* player = PlayerManager::GetPlayer(playerID);
 
 		if (player == nullptr) {
 			continue;
@@ -548,7 +549,7 @@ void EntityManager::CheckGhosting(Entity* entity) {
 
 	const auto isAudioEmitter = entity->GetLOT() == 6368;
 
-	for (auto* player : Player::GetAllPlayers()) {
+	for (auto* player : PlayerManager::GetAllPlayers()) {
 		const auto& entityPoint = player->GetGhostReferencePoint();
 
 		const int32_t id = entity->GetObjectID();

dGame/Player.cpp

@@ -12,14 +12,7 @@
 #include "CppScripts.h"
 #include "Loot.h"
 #include "eReplicaComponentType.h"
-
+#include "PlayerManager.h"
-namespace {
-	std::vector<Player*> m_Players;
-};
-
-const std::vector<Player*>& Player::GetAllPlayers() {
-	return m_Players;
-}
 
 void Player::SetGhostReferencePoint(const NiPoint3& value) {
 	m_GhostReferencePoint = value;
@@ -63,11 +56,7 @@ Player::Player(const LWOOBJID& objectID, const EntityInfo info, User* user, Enti
 
 	m_Character->SetEntity(this);
 
-	const auto& iter = std::find(m_Players.begin(), m_Players.end(), this);
+	PlayerManager::AddPlayer(this);
-
-	if (iter == m_Players.end()) {
-		m_Players.push_back(this);
-	}
 }
 
 void Player::AddLimboConstruction(LWOOBJID objectId) {
@@ -121,38 +110,6 @@ void Player::GhostEntity(int32_t id) {
 	}
 }
 
-Player* Player::GetPlayer(const SystemAddress& sysAddr) {
-	auto* entity = UserManager::Instance()->GetUser(sysAddr)->GetLastUsedChar()->GetEntity();
-
-	return static_cast<Player*>(entity);
-}
-
-Player* Player::GetPlayer(const std::string& name) {
-	const auto characters = Game::entityManager->GetEntitiesByComponent(eReplicaComponentType::CHARACTER);
-
-	Player* player = nullptr;
-	for (auto* character : characters) {
-		if (!character->IsPlayer()) continue;
-		
-		if (GeneralUtils::CaseInsensitiveStringCompare(name, character->GetCharacter()->GetName())) {
-			player = dynamic_cast<Player*>(character);
-		}
-	}
-
-	return player;
-}
-
-Player* Player::GetPlayer(LWOOBJID playerID) {
-	Player* playerToReturn = nullptr;
-	for (auto* player : m_Players) {
-		if (player->GetObjectID() == playerID) {
-			playerToReturn = player;
-		}
-	}
-
-	return playerToReturn;
-}
-
 Player::~Player() {
 	LOG("Deleted player");
 
@@ -167,9 +124,9 @@ Player::~Player() {
 
 	m_LimboConstructions.clear();
 	
-	const auto iter = std::find(m_Players.begin(), m_Players.end(), this);
+	// Make sure the player exists first.  Remove afterwards to prevent the OnPlayerExist functions from not being able to find the player.
-
+	if (!PlayerManager::GetPlayer(GetObjectID())) {
-	if (iter == m_Players.end()) {
+		LOG("Unable to find player to remove from manager.");
 		return;
 	}
 
@@ -189,5 +146,5 @@ Player::~Player() {
 		}
 	}
 
-	m_Players.erase(iter);
+	PlayerManager::RemovePlayer(this);
 }

dGame/Player.h

@@ -72,18 +72,6 @@ public:
 
 	void GhostEntity(const int32_t id);
 
-	/**
-	 * Static methods
-	 */
-
-	static Player* GetPlayer(const SystemAddress& sysAddr);
-
-	static Player* GetPlayer(const std::string& name);
-
-	static Player* GetPlayer(LWOOBJID playerID);
-
-	static const std::vector<Player*>& GetAllPlayers();
-
 	~Player() override;
 private:
 	SystemAddress m_SystemAddress;

dGame/PlayerManager.cpp

@@ -0,0 +1,65 @@
+#include "PlayerManager.h"
+
+#include "Character.h"
+#include "Player.h"
+#include "User.h"
+#include "UserManager.h"
+#include "eReplicaComponentType.h"
+
+namespace {
+	std::vector<Player*> m_Players;
+};
+
+const std::vector<Player*>& PlayerManager::GetAllPlayers() {
+	return m_Players;
+}
+
+void PlayerManager::AddPlayer(Player* player) {
+	const auto& iter = std::find(m_Players.begin(), m_Players.end(), player);
+
+	if (iter == m_Players.end()) {
+		m_Players.push_back(player);
+	}
+}
+
+void PlayerManager::RemovePlayer(Player* player) {
+	const auto iter = std::find(m_Players.begin(), m_Players.end(), player);
+
+	if (iter != m_Players.end()) {
+		m_Players.erase(iter);
+	}
+}
+
+Player* PlayerManager::GetPlayer(const SystemAddress& sysAddr) {
+	auto* entity = UserManager::Instance()->GetUser(sysAddr)->GetLastUsedChar()->GetEntity();
+
+	return static_cast<Player*>(entity);
+}
+
+Player* PlayerManager::GetPlayer(const std::string& name) {
+	const auto characters = Game::entityManager->GetEntitiesByComponent(eReplicaComponentType::CHARACTER);
+
+	Player* player = nullptr;
+	for (auto* character : characters) {
+		if (!character->IsPlayer()) continue;
+		
+		if (GeneralUtils::CaseInsensitiveStringCompare(name, character->GetCharacter()->GetName())) {
+			player = dynamic_cast<Player*>(character);
+			break;
+		}
+	}
+
+	return player;
+}
+
+Player* PlayerManager::GetPlayer(LWOOBJID playerID) {
+	Player* playerToReturn = nullptr;
+	for (auto* player : m_Players) {
+		if (player->GetObjectID() == playerID) {
+			playerToReturn = player;
+			break;
+		}
+	}
+
+	return playerToReturn;
+}

dGame/PlayerManager.h

@@ -0,0 +1,25 @@
+#ifndef __PLAYERMANAGER__H__
+#define __PLAYERMANAGER__H__
+
+#include "dCommonVars.h"
+
+#include <string>
+
+class Player;
+struct SystemAddress;
+
+namespace PlayerManager {
+	void AddPlayer(Player* player);
+
+	void RemovePlayer(Player* player);
+
+	Player* GetPlayer(const SystemAddress& sysAddr);
+
+	Player* GetPlayer(const std::string& name);
+
+	Player* GetPlayer(LWOOBJID playerID);
+
+	const std::vector<Player*>& GetAllPlayers();
+};
+
+#endif  //!__PLAYERMANAGER__H__

dGame/dComponents/PropertyManagementComponent.cpp

@@ -21,6 +21,7 @@
 #include "eMissionTaskType.h"
 #include "eObjectBits.h"
 #include "CharacterComponent.h"
+#include "PlayerManager.h"
 
 #include <vector>
 #include "CppScripts.h"
@@ -227,7 +228,7 @@ void PropertyManagementComponent::OnStartBuilding() {
 
 	if (ownerEntity == nullptr) return;
 
-	const auto players = Player::GetAllPlayers();
+	const auto players = PlayerManager::GetAllPlayers();
 
 	LWOMAPID zoneId = 1100;
 

dGame/dComponents/TriggerComponent.cpp

@@ -13,6 +13,7 @@
 #include "QuickBuildComponent.h"
 #include "SkillComponent.h"
 #include "eEndBehavior.h"
+#include "PlayerManager.h"
 
 
 TriggerComponent::TriggerComponent(Entity* parent, const std::string triggerInfo): Component(parent) {
@@ -175,7 +176,7 @@ std::vector<Entity*> TriggerComponent::GatherTargets(LUTriggers::Command* comman
 		}
 	} else if (command->target == "objGroup") entities = Game::entityManager->GetEntitiesInGroup(command->targetName);
 	else if (command->target == "allPlayers") {
-		for (auto* player : Player::GetAllPlayers()) {
+		for (auto* player : PlayerManager::GetAllPlayers()) {
 			entities.push_back(player);
 		}
 	} else if (command->target == "allNPCs") { /*UNUSED*/ }

dGame/dGameMessages/GameMessages.cpp

@@ -96,6 +96,7 @@
 #include "eGameMessageType.h"
 #include "ePetAbilityType.h"
 #include "ActivityManager.h"
+#include "PlayerManager.h"
 
 #include "CDComponentsRegistryTable.h"
 #include "CDObjectsTable.h"
@@ -2693,7 +2694,7 @@ void GameMessages::HandlePropertyEntranceSync(RakNet::BitStream* inStream, Entit
 		filterText.push_back(c);
 	}
 
-	auto* player = Player::GetPlayer(sysAddr);
+	auto* player = PlayerManager::GetPlayer(sysAddr);
 
 	auto* entranceComponent = entity->GetComponent<PropertyEntranceComponent>();
 
@@ -2720,7 +2721,7 @@ void GameMessages::HandleEnterProperty(RakNet::BitStream* inStream, Entity* enti
 	inStream->Read(index);
 	inStream->Read(returnToZone);
 
-	auto* player = Player::GetPlayer(sysAddr);
+	auto* player = PlayerManager::GetPlayer(sysAddr);
 
 	auto* entranceComponent = entity->GetComponent<PropertyEntranceComponent>();
 	if (entranceComponent != nullptr) {
@@ -4601,7 +4602,7 @@ void GameMessages::HandleToggleGhostReferenceOverride(RakNet::BitStream* inStrea
 
 	inStream->Read(bOverride);
 
-	auto* player = Player::GetPlayer(sysAddr);
+	auto* player = PlayerManager::GetPlayer(sysAddr);
 
 	if (player != nullptr) {
 		player->SetGhostOverride(bOverride);
@@ -4616,7 +4617,7 @@ void GameMessages::HandleSetGhostReferencePosition(RakNet::BitStream* inStream,
 
 	inStream->Read(position);
 
-	auto* player = Player::GetPlayer(sysAddr);
+	auto* player = PlayerManager::GetPlayer(sysAddr);
 
 	if (player != nullptr) {
 		player->SetGhostOverridePoint(position);
@@ -4880,7 +4881,7 @@ void GameMessages::HandleFireEventServerSide(RakNet::BitStream* inStream, Entity
 	inStream->Read(senderID);
 
 	auto* sender = Game::entityManager->GetEntity(senderID);
-	auto* player = Player::GetPlayer(sysAddr);
+	auto* player = PlayerManager::GetPlayer(sysAddr);
 
 	if (!player) {
 		return;

dGame/dUtilities/CheatDetection.cpp

@@ -10,6 +10,7 @@
 #include "UserManager.h"
 #include "dConfig.h"
 #include <optional>
+#include "PlayerManager.h"
 
 Entity* GetPossessedEntity(const LWOOBJID& objId) {
 	auto* entity = Game::entityManager->GetEntity(objId);
@@ -49,7 +50,7 @@ void LogAndSaveFailedAntiCheatCheck(const LWOOBJID& id, const SystemAddress& sys
 	User* toReport = nullptr;
 	switch (checkType) {
 	case CheckType::Entity: {
-		auto* player = Player::GetPlayer(sysAddr);
+		auto* player = PlayerManager::GetPlayer(sysAddr);
 		auto* entity = GetPossessedEntity(id);
 		
 		// If player exists and entity exists in world, use both for logging info.

dGame/dUtilities/SlashCommandHandler.cpp

@@ -82,6 +82,7 @@
 #include "eConnectionType.h"
 #include "eChatInternalMessageType.h"
 #include "eMasterMessageType.h"
+#include "PlayerManager.h"
 
 #include "CDRewardCodesTable.h"
 #include "CDObjectsTable.h"
@@ -214,10 +215,10 @@ void SlashCommandHandler::HandleChatCommand(const std::u16string& command, Entit
 	if (chatCommand == "who") {
 		ChatPackets::SendSystemMessage(
 			sysAddr,
-			u"Players in this instance: (" + GeneralUtils::to_u16string(Player::GetAllPlayers().size()) + u")"
+			u"Players in this instance: (" + GeneralUtils::to_u16string(PlayerManager::GetAllPlayers().size()) + u")"
 		);
 
-		for (auto* player : Player::GetAllPlayers()) {
+		for (auto* player : PlayerManager::GetAllPlayers()) {
 			const auto& name = player->GetCharacter()->GetName();
 
 			ChatPackets::SendSystemMessage(
@@ -473,7 +474,7 @@ void SlashCommandHandler::HandleChatCommand(const std::u16string& command, Entit
 	if (chatCommand == "kill" && args.size() == 1 && entity->GetGMLevel() >= eGameMasterLevel::DEVELOPER) {
 		ChatPackets::SendSystemMessage(sysAddr, u"Brutally murdering that player, if online on this server.");
 
-		auto* player = Player::GetPlayer(args[0]);
+		auto* player = PlayerManager::GetPlayer(args[0]);
 		if (player) {
 			player->Smash(entity->GetObjectID());
 			ChatPackets::SendSystemMessage(sysAddr, u"It has been done, do you feel good about yourself now?");
@@ -993,7 +994,7 @@ void SlashCommandHandler::HandleChatCommand(const std::u16string& command, Entit
 
 	if (chatCommand == "mute" && entity->GetGMLevel() >= eGameMasterLevel::JUNIOR_DEVELOPER) {
 		if (args.size() >= 1) {
-			auto* player = Player::GetPlayer(args[0]);
+			auto* player = PlayerManager::GetPlayer(args[0]);
 
 			uint32_t accountId = 0;
 			LWOOBJID characterId = 0;
@@ -1072,7 +1073,7 @@ void SlashCommandHandler::HandleChatCommand(const std::u16string& command, Entit
 
 	if (chatCommand == "kick" && entity->GetGMLevel() >= eGameMasterLevel::JUNIOR_MODERATOR) {
 		if (args.size() == 1) {
-			auto* player = Player::GetPlayer(args[0]);
+			auto* player = PlayerManager::GetPlayer(args[0]);
 
 			std::u16string username = GeneralUtils::UTF8ToUTF16(args[0]);
 			if (player == nullptr) {
@@ -1090,7 +1091,7 @@ void SlashCommandHandler::HandleChatCommand(const std::u16string& command, Entit
 
 	if (chatCommand == "ban" && entity->GetGMLevel() >= eGameMasterLevel::SENIOR_MODERATOR) {
 		if (args.size() == 1) {
-			auto* player = Player::GetPlayer(args[0]);
+			auto* player = PlayerManager::GetPlayer(args[0]);
 
 			uint32_t accountId = 0;
 
@@ -1303,7 +1304,7 @@ void SlashCommandHandler::HandleChatCommand(const std::u16string& command, Entit
 		if (args.size() > 1) {
 			requestedPlayerToSetLevelOf = args[1];
 
-			auto requestedPlayer = Player::GetPlayer(requestedPlayerToSetLevelOf);
+			auto requestedPlayer = PlayerManager::GetPlayer(requestedPlayerToSetLevelOf);
 
 			if (!requestedPlayer) {
 				ChatPackets::SendSystemMessage(sysAddr, u"No player found with username: (" + GeneralUtils::UTF8ToUTF16(requestedPlayerToSetLevelOf) + u").");

dWorldServer/WorldServer.cpp

@@ -78,6 +78,7 @@
 #include "StringifiedEnum.h"
 #include "Server.h"
 #include "PositionUpdate.h"
+#include "PlayerManager.h"
 
 namespace Game {
 	Logger* logger = nullptr;
@@ -798,7 +799,7 @@ void HandlePacket(Packet* packet) {
 		auto* entity = Game::entityManager->GetEntity(c->GetObjectID());
 
 		if (!entity) {
-			entity = Player::GetPlayer(packet->systemAddress);
+			entity = PlayerManager::GetPlayer(packet->systemAddress);
 		}
 
 		if (entity) {
@@ -1205,7 +1206,7 @@ void HandlePacket(Packet* packet) {
 			return;
 		}
 
-		auto* entity = Player::GetPlayer(packet->systemAddress);
+		auto* entity = PlayerManager::GetPlayer(packet->systemAddress);
 
 		if (entity == nullptr) {
 			LOG("Unable to get player to parse chat moderation request");
@@ -1365,7 +1366,7 @@ void WorldShutdownProcess(uint32_t zoneId) {
 	for (auto i = 0; i < Game::server->GetReplicaManager()->GetParticipantCount(); ++i) {
 		const auto& player = Game::server->GetReplicaManager()->GetParticipantAtIndex(i);
 
-		auto* entity = Player::GetPlayer(player);
+		auto* entity = PlayerManager::GetPlayer(player);
 		LOG("Saving data!");
 		if (entity != nullptr && entity->GetCharacter() != nullptr) {
 			auto* skillComponent = entity->GetComponent<SkillComponent>();