fix: player lose items with wrong loading offline informations (#2919)

When the player is offline, in some scenarios the empty items would load and the items would be overlapped with the empty table of the offline player. Resolves #2917 Related to: #2910
opentibiabr · Sep 29, 2024 · 899c1eb · 899c1eb
1 parent 04fa248
commit 899c1eb
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 50 deletions.
diff --git a/src/creatures/players/cyclopedia/player_badge.cpp b/src/creatures/players/cyclopedia/player_badge.cpp
@@ -116,7 +116,7 @@ bool PlayerBadge::loyalty(uint8_t amount) {
 }
 
 bool PlayerBadge::accountAllLevel(uint8_t amount) {
-	const auto &players = g_game().getPlayersByAccount(m_player.getAccount(), true);
+	auto players = g_game().getPlayersByAccount(m_player.getAccount(), true);
 	uint16_t total = std::accumulate(players.begin(), players.end(), 0, [](uint16_t sum, const std::shared_ptr<Player> &player) {
 		return sum + player->getLevel();
 	});

diff --git a/src/game/game.cpp b/src/game/game.cpp
@@ -1014,8 +1014,9 @@ std::shared_ptr<Player> Game::getPlayerByGUID(const uint32_t &guid, bool allowOf
 	if (!allowOffline) {
 		return nullptr;
 	}
+
 	std::shared_ptr<Player> tmpPlayer = std::make_shared<Player>(nullptr);
-	if (!IOLoginData::loadPlayerById(tmpPlayer, guid)) {
+	if (!IOLoginData::loadPlayerById(tmpPlayer, guid, false)) {
 		return nullptr;
 	}
 	tmpPlayer->setOnline(false);
@@ -1029,7 +1030,9 @@ std::string Game::getPlayerNameByGUID(const uint32_t &guid) {
 	if (m_playerNameCache.contains(guid)) {
 		return m_playerNameCache.at(guid);
 	}
-	auto player = getPlayerByGUID(guid, true);
+
+	// This player need read-only purposes and never saved
+	const auto &player = getPlayerByGUID(guid, true);
 	auto name = player ? player->getName() : "";
 	if (!name.empty()) {
 		m_playerNameCache[guid] = name;
@@ -1068,9 +1071,10 @@ std::vector<std::shared_ptr<Player>> Game::getPlayersByAccount(std::shared_ptr<A
 	if (error != enumToValue(AccountErrors_t::Ok)) {
 		return {};
 	}
+
 	std::vector<std::shared_ptr<Player>> ret;
 	for (const auto &[name, _] : accountPlayers) {
-		auto player = getPlayerByName(name, allowOffline);
+		const auto &player = getPlayerByName(name, allowOffline);
 		if (player) {
 			ret.push_back(player);
 		}

diff --git a/src/io/functions/iologindata_load_player.cpp b/src/io/functions/iologindata_load_player.cpp
@@ -105,9 +105,9 @@ bool IOLoginDataLoad::preLoadPlayer(std::shared_ptr<Player> player, const std::s
 	return true;
 }
 
-bool IOLoginDataLoad::loadPlayerFirst(std::shared_ptr<Player> player, DBResult_ptr result) {
+bool IOLoginDataLoad::loadPlayerBasicInfo(std::shared_ptr<Player> player, DBResult_ptr result) {
 	if (!result || !player) {
-		g_logger().warn("[IOLoginData::loadPlayer] - Player or Result nullptr: {}", __FUNCTION__);
+		g_logger().warn("[{}] - Player or Result nullptr", __FUNCTION__);
 		return false;
 	}
 
@@ -184,12 +184,15 @@ bool IOLoginDataLoad::loadPlayerFirst(std::shared_ptr<Player> player, DBResult_p
 	player->setMaxManaShield(result->getNumber<uint32_t>("max_manashield"));
 
 	player->setMarriageSpouse(result->getNumber<int32_t>("marriage_spouse"));
+
+	// Experience load
+	IOLoginDataLoad::loadPlayerExperience(player, result);
 	return true;
 }
 
 void IOLoginDataLoad::loadPlayerExperience(std::shared_ptr<Player> player, DBResult_ptr result) {
 	if (!result || !player) {
-		g_logger().warn("[IOLoginData::loadPlayer] - Player or Result nullptr: {}", __FUNCTION__);
+		g_logger().warn("[{}] - Player or Result nullptr", __FUNCTION__);
 		return;
 	}
 
@@ -212,7 +215,7 @@ void IOLoginDataLoad::loadPlayerExperience(std::shared_ptr<Player> player, DBRes
 
 void IOLoginDataLoad::loadPlayerBlessings(std::shared_ptr<Player> player, DBResult_ptr result) {
 	if (!result || !player) {
-		g_logger().warn("[IOLoginData::loadPlayer] - Player or Result nullptr: {}", __FUNCTION__);
+		g_logger().warn("[{}] - Player or Result nullptr", __FUNCTION__);
 		return;
 	}
 
@@ -223,7 +226,7 @@ void IOLoginDataLoad::loadPlayerBlessings(std::shared_ptr<Player> player, DBResu
 
 void IOLoginDataLoad::loadPlayerConditions(std::shared_ptr<Player> player, DBResult_ptr result) {
 	if (!result || !player) {
-		g_logger().warn("[IOLoginData::loadPlayer] - Player or Result nullptr: {}", __FUNCTION__);
+		g_logger().warn("[{}] - Player or Result nullptr", __FUNCTION__);
 		return;
 	}
 
@@ -243,13 +246,13 @@ void IOLoginDataLoad::loadPlayerConditions(std::shared_ptr<Player> player, DBRes
 
 void IOLoginDataLoad::loadPlayerDefaultOutfit(std::shared_ptr<Player> player, DBResult_ptr result) {
 	if (!result || !player) {
-		g_logger().warn("[IOLoginData::loadPlayer] - Player or Result nullptr: {}", __FUNCTION__);
+		g_logger().warn("[{}] - Player or Result nullptr", __FUNCTION__);
 		return;
 	}
 
 	player->defaultOutfit.lookType = result->getNumber<uint16_t>("looktype");
 	if (g_configManager().getBoolean(WARN_UNSAFE_SCRIPTS, __FUNCTION__) && player->defaultOutfit.lookType != 0 && !g_game().isLookTypeRegistered(player->defaultOutfit.lookType)) {
-		g_logger().warn("[IOLoginData::loadPlayer] An unregistered creature looktype type with id '{}' was blocked to prevent client crash.", player->defaultOutfit.lookType);
+		g_logger().warn("[{}] An unregistered creature looktype type with id '{}' was blocked to prevent client crash.", __FUNCTION__, player->defaultOutfit.lookType);
 		return;
 	}
 
@@ -265,7 +268,7 @@ void IOLoginDataLoad::loadPlayerDefaultOutfit(std::shared_ptr<Player> player, DB
 	player->defaultOutfit.lookFamiliarsType = result->getNumber<uint16_t>("lookfamiliarstype");
 
 	if (g_configManager().getBoolean(WARN_UNSAFE_SCRIPTS, __FUNCTION__) && player->defaultOutfit.lookFamiliarsType != 0 && !g_game().isLookTypeRegistered(player->defaultOutfit.lookFamiliarsType)) {
-		g_logger().warn("[IOLoginData::loadPlayer] An unregistered creature looktype type with id '{}' was blocked to prevent client crash.", player->defaultOutfit.lookFamiliarsType);
+		g_logger().warn("[{}] An unregistered creature looktype type with id '{}' was blocked to prevent client crash.", __FUNCTION__, player->defaultOutfit.lookFamiliarsType);
 		return;
 	}
 
@@ -274,7 +277,7 @@ void IOLoginDataLoad::loadPlayerDefaultOutfit(std::shared_ptr<Player> player, DB
 
 void IOLoginDataLoad::loadPlayerSkullSystem(std::shared_ptr<Player> player, DBResult_ptr result) {
 	if (!result || !player) {
-		g_logger().warn("[IOLoginData::loadPlayer] - Player or Result nullptr: {}", __FUNCTION__);
+		g_logger().warn("[{}] - Player or Result nullptr", __FUNCTION__);
 		return;
 	}
 
@@ -296,7 +299,7 @@ void IOLoginDataLoad::loadPlayerSkullSystem(std::shared_ptr<Player> player, DBRe
 
 void IOLoginDataLoad::loadPlayerSkill(std::shared_ptr<Player> player, DBResult_ptr result) {
 	if (!result || !player) {
-		g_logger().warn("[IOLoginData::loadPlayer] - Player or Result nullptr: {}", __FUNCTION__);
+		g_logger().warn("[{}] - Player or Result nullptr", __FUNCTION__);
 		return;
 	}
 
@@ -318,7 +321,7 @@ void IOLoginDataLoad::loadPlayerSkill(std::shared_ptr<Player> player, DBResult_p
 
 void IOLoginDataLoad::loadPlayerKills(std::shared_ptr<Player> player, DBResult_ptr result) {
 	if (!result || !player) {
-		g_logger().warn("[IOLoginData::loadPlayer] - Player or Result nullptr: {}", __FUNCTION__);
+		g_logger().warn("[{}] - Player or Result nullptr", __FUNCTION__);
 		return;
 	}
 
@@ -337,7 +340,7 @@ void IOLoginDataLoad::loadPlayerKills(std::shared_ptr<Player> player, DBResult_p
 
 void IOLoginDataLoad::loadPlayerGuild(std::shared_ptr<Player> player, DBResult_ptr result) {
 	if (!result || !player) {
-		g_logger().warn("[IOLoginData::loadPlayer] - Player or Result nullptr: {}", __FUNCTION__);
+		g_logger().warn("[{}] - Player or Result nullptr", __FUNCTION__);
 		return;
 	}
 
@@ -387,7 +390,7 @@ void IOLoginDataLoad::loadPlayerGuild(std::shared_ptr<Player> player, DBResult_p
 
 void IOLoginDataLoad::loadPlayerStashItems(std::shared_ptr<Player> player, DBResult_ptr result) {
 	if (!result || !player) {
-		g_logger().warn("[IOLoginData::loadPlayer] - Player or Result nullptr: {}", __FUNCTION__);
+		g_logger().warn("[{}] - Player or Result nullptr", __FUNCTION__);
 		return;
 	}
 
@@ -403,7 +406,7 @@ void IOLoginDataLoad::loadPlayerStashItems(std::shared_ptr<Player> player, DBRes
 
 void IOLoginDataLoad::loadPlayerBestiaryCharms(std::shared_ptr<Player> player, DBResult_ptr result) {
 	if (!result || !player) {
-		g_logger().warn("[IOLoginData::loadPlayer] - Player or Result nullptr: {}", __FUNCTION__);
+		g_logger().warn("[{}] - Player or Result nullptr", __FUNCTION__);
 		return;
 	}
 
@@ -456,7 +459,7 @@ void IOLoginDataLoad::loadPlayerBestiaryCharms(std::shared_ptr<Player> player, D
 
 void IOLoginDataLoad::loadPlayerInstantSpellList(std::shared_ptr<Player> player, DBResult_ptr result) {
 	if (!player) {
-		g_logger().warn("[IOLoginData::loadPlayer] - Player nullptr: {}", __FUNCTION__);
+		g_logger().warn("[{}] - Player nullptr", __FUNCTION__);
 		return;
 	}
 
@@ -472,7 +475,7 @@ void IOLoginDataLoad::loadPlayerInstantSpellList(std::shared_ptr<Player> player,
 
 void IOLoginDataLoad::loadPlayerInventoryItems(std::shared_ptr<Player> player, DBResult_ptr result) {
 	if (!result || !player) {
-		g_logger().warn("[IOLoginData::loadPlayer] - Player or Result nullptr: {}", __FUNCTION__);
+		g_logger().warn("[{}] - Player or Result nullptr", __FUNCTION__);
 		return;
 	}
 
@@ -554,7 +557,7 @@ void IOLoginDataLoad::loadPlayerInventoryItems(std::shared_ptr<Player> player, D
 
 void IOLoginDataLoad::loadPlayerStoreInbox(std::shared_ptr<Player> player) {
 	if (!player) {
-		g_logger().warn("[IOLoginData::loadPlayer] - Player nullptr: {}", __FUNCTION__);
+		g_logger().warn("[{}] - Player nullptr", __FUNCTION__);
 		return;
 	}
 
@@ -565,7 +568,7 @@ void IOLoginDataLoad::loadPlayerStoreInbox(std::shared_ptr<Player> player) {
 
 void IOLoginDataLoad::loadRewardItems(std::shared_ptr<Player> player) {
 	if (!player) {
-		g_logger().warn("[IOLoginData::loadPlayer] - Player nullptr: {}", __FUNCTION__);
+		g_logger().warn("[{}] - Player nullptr", __FUNCTION__);
 		return;
 	}
 
@@ -583,7 +586,7 @@ void IOLoginDataLoad::loadRewardItems(std::shared_ptr<Player> player) {
 
 void IOLoginDataLoad::loadPlayerDepotItems(std::shared_ptr<Player> player, DBResult_ptr result) {
 	if (!result || !player) {
-		g_logger().warn("[IOLoginData::loadPlayer] - Player or Result nullptr: {}", __FUNCTION__);
+		g_logger().warn("[{}] - Player or Result nullptr", __FUNCTION__);
 		return;
 	}
 
@@ -622,7 +625,7 @@ void IOLoginDataLoad::loadPlayerDepotItems(std::shared_ptr<Player> player, DBRes
 
 void IOLoginDataLoad::loadPlayerInboxItems(std::shared_ptr<Player> player, DBResult_ptr result) {
 	if (!result || !player) {
-		g_logger().warn("[IOLoginData::loadPlayer] - Player or Result nullptr: {}", __FUNCTION__);
+		g_logger().warn("[{}] - Player or Result nullptr", __FUNCTION__);
 		return;
 	}
 
@@ -658,7 +661,7 @@ void IOLoginDataLoad::loadPlayerInboxItems(std::shared_ptr<Player> player, DBRes
 
 void IOLoginDataLoad::loadPlayerStorageMap(std::shared_ptr<Player> player, DBResult_ptr result) {
 	if (!result || !player) {
-		g_logger().warn("[IOLoginData::loadPlayer] - Player or Result nullptr: {}", __FUNCTION__);
+		g_logger().warn("[{}] - Player or Result nullptr", __FUNCTION__);
 		return;
 	}
 
@@ -674,7 +677,7 @@ void IOLoginDataLoad::loadPlayerStorageMap(std::shared_ptr<Player> player, DBRes
 
 void IOLoginDataLoad::loadPlayerVip(std::shared_ptr<Player> player, DBResult_ptr result) {
 	if (!result || !player) {
-		g_logger().warn("[IOLoginData::loadPlayer] - Player or Result nullptr: {}", __FUNCTION__);
+		g_logger().warn("[{}] - Player or Result nullptr", __FUNCTION__);
 		return;
 	}
 
@@ -712,7 +715,7 @@ void IOLoginDataLoad::loadPlayerVip(std::shared_ptr<Player> player, DBResult_ptr
 
 void IOLoginDataLoad::loadPlayerPreyClass(std::shared_ptr<Player> player, DBResult_ptr result) {
 	if (!result || !player) {
-		g_logger().warn("[IOLoginData::loadPlayer] - Player or Result nullptr: {}", __FUNCTION__);
+		g_logger().warn("[{}] - Player or Result nullptr", __FUNCTION__);
 		return;
 	}
 
@@ -759,7 +762,7 @@ void IOLoginDataLoad::loadPlayerPreyClass(std::shared_ptr<Player> player, DBResu
 
 void IOLoginDataLoad::loadPlayerTaskHuntingClass(std::shared_ptr<Player> player, DBResult_ptr result) {
 	if (!result || !player) {
-		g_logger().warn("[IOLoginData::loadPlayer] - Player or Result nullptr: {}", __FUNCTION__);
+		g_logger().warn("[{}] - Player or Result nullptr", __FUNCTION__);
 		return;
 	}
 
@@ -809,7 +812,7 @@ void IOLoginDataLoad::loadPlayerTaskHuntingClass(std::shared_ptr<Player> player,
 
 void IOLoginDataLoad::loadPlayerForgeHistory(std::shared_ptr<Player> player, DBResult_ptr result) {
 	if (!result || !player) {
-		g_logger().warn("[IOLoginData::loadPlayer] - Player or Result nullptr: {}", __FUNCTION__);
+		g_logger().warn("[{}] - Player or Result nullptr", __FUNCTION__);
 		return;
 	}
 
@@ -830,12 +833,12 @@ void IOLoginDataLoad::loadPlayerForgeHistory(std::shared_ptr<Player> player, DBR
 
 void IOLoginDataLoad::loadPlayerBosstiary(std::shared_ptr<Player> player, DBResult_ptr result) {
 	if (!result) {
-		g_logger().warn("[IOLoginData::loadPlayer] - Result nullptr: {}", __FUNCTION__);
+		g_logger().warn("[{}] - Result nullptr", __FUNCTION__);
 		return;
 	}
 
 	if (!player) {
-		g_logger().warn("[IOLoginData::loadPlayer] - Player or Result nullptr: {}", __FUNCTION__);
+		g_logger().warn("[{}] - Player nullptr", __FUNCTION__);
 		return;
 	}
 
@@ -867,7 +870,7 @@ void IOLoginDataLoad::loadPlayerBosstiary(std::shared_ptr<Player> player, DBResu
 
 void IOLoginDataLoad::bindRewardBag(std::shared_ptr<Player> player, ItemsMap &rewardItemsMap) {
 	if (!player) {
-		g_logger().warn("[IOLoginData::loadPlayer] - Player nullptr: {}", __FUNCTION__);
+		g_logger().warn("[{}] - Player nullptr", __FUNCTION__);
 		return;
 	}
 
@@ -907,7 +910,7 @@ void IOLoginDataLoad::insertItemsIntoRewardBag(const ItemsMap &rewardItemsMap) {
 
 void IOLoginDataLoad::loadPlayerInitializeSystem(std::shared_ptr<Player> player) {
 	if (!player) {
-		g_logger().warn("[IOLoginData::loadPlayer] - Player nullptr: {}", __FUNCTION__);
+		g_logger().warn("[{}] - Player nullptr", __FUNCTION__);
 		return;
 	}
 
@@ -926,7 +929,7 @@ void IOLoginDataLoad::loadPlayerInitializeSystem(std::shared_ptr<Player> player)
 
 void IOLoginDataLoad::loadPlayerUpdateSystem(std::shared_ptr<Player> player) {
 	if (!player) {
-		g_logger().warn("[IOLoginData::loadPlayer] - Player nullptr: {}", __FUNCTION__);
+		g_logger().warn("[{}] - Player nullptr", __FUNCTION__);
 		return;
 	}
 

diff --git a/src/io/functions/iologindata_load_player.hpp b/src/io/functions/iologindata_load_player.hpp
@@ -13,7 +13,7 @@
 
 class IOLoginDataLoad : public IOLoginData {
 public:
-	static bool loadPlayerFirst(std::shared_ptr<Player> player, DBResult_ptr result);
+	static bool loadPlayerBasicInfo(std::shared_ptr<Player> player, DBResult_ptr result);
 	static bool preLoadPlayer(std::shared_ptr<Player> player, const std::string &name);
 	static void loadPlayerExperience(std::shared_ptr<Player> player, DBResult_ptr result);
 	static void loadPlayerBlessings(std::shared_ptr<Player> player, DBResult_ptr result);

diff --git a/src/io/iologindata.cpp b/src/io/iologindata.cpp
@@ -94,22 +94,22 @@ void IOLoginData::updateOnlineStatus(uint32_t guid, bool login) {
 	Database::getInstance().executeQuery(query.str());
 }
 
-// The boolean "disableIrrelevantInfo" will deactivate the loading of information that is not relevant to the preload, for example, forge, bosstiary, etc. None of this we need to access if the player is offline
-bool IOLoginData::loadPlayerById(std::shared_ptr<Player> player, uint32_t id, bool disableIrrelevantInfo /* = true*/) {
+// The boolean "loadBasicInfoOnly" will deactivate the loading of information that is not relevant to the preload, for example, forge, bosstiary, etc. None of this we need to access if the player is offline
+bool IOLoginData::loadPlayerById(std::shared_ptr<Player> player, uint32_t id, bool loadBasicInfoOnly /* = true*/) {
 	Database &db = Database::getInstance();
 	std::ostringstream query;
 	query << "SELECT * FROM `players` WHERE `id` = " << id;
-	return loadPlayer(player, db.storeQuery(query.str()), disableIrrelevantInfo);
+	return loadPlayer(player, db.storeQuery(query.str()), loadBasicInfoOnly);
 }
 
-bool IOLoginData::loadPlayerByName(std::shared_ptr<Player> player, const std::string &name, bool disableIrrelevantInfo /* = true*/) {
+bool IOLoginData::loadPlayerByName(std::shared_ptr<Player> player, const std::string &name, bool loadBasicInfoOnly /* = true*/) {
 	Database &db = Database::getInstance();
 	std::ostringstream query;
 	query << "SELECT * FROM `players` WHERE `name` = " << db.escapeString(name);
-	return loadPlayer(player, db.storeQuery(query.str()), disableIrrelevantInfo);
+	return loadPlayer(player, db.storeQuery(query.str()), loadBasicInfoOnly);
 }
 
-bool IOLoginData::loadPlayer(std::shared_ptr<Player> player, DBResult_ptr result, bool disableIrrelevantInfo /* = false*/) {
+bool IOLoginData::loadPlayer(std::shared_ptr<Player> player, DBResult_ptr result, bool loadBasicInfoOnly /* = false*/) {
 	if (!result || !player) {
 		std::string nullptrType = !result ? "Result" : "Player";
 		g_logger().warn("[{}] - {} is nullptr", __FUNCTION__, nullptrType);
@@ -118,12 +118,8 @@ bool IOLoginData::loadPlayer(std::shared_ptr<Player> player, DBResult_ptr result
 
 	try {
 		// First
-		IOLoginDataLoad::loadPlayerFirst(player, result);
-
-		// Experience load
-		IOLoginDataLoad::loadPlayerExperience(player, result);
-
-		if (disableIrrelevantInfo) {
+		IOLoginDataLoad::loadPlayerBasicInfo(player, result);
+		if (loadBasicInfoOnly) {
 			return true;
 		}
 

diff --git a/src/io/iologindata.hpp b/src/io/iologindata.hpp
@@ -20,9 +20,9 @@ class IOLoginData {
 	static bool gameWorldAuthentication(const std::string &accountDescriptor, const std::string &sessionOrPassword, std::string &characterName, uint32_t &accountId, bool oldProcotol, const uint32_t ip);
 	static uint8_t getAccountType(uint32_t accountId);
 	static void updateOnlineStatus(uint32_t guid, bool login);
-	static bool loadPlayerById(std::shared_ptr<Player> player, uint32_t id, bool disableIrrelevantInfo = true);
-	static bool loadPlayerByName(std::shared_ptr<Player> player, const std::string &name, bool disableIrrelevantInfo = true);
-	static bool loadPlayer(std::shared_ptr<Player> player, DBResult_ptr result, bool disableIrrelevantInfo = false);
+	static bool loadPlayerById(std::shared_ptr<Player> player, uint32_t id, bool loadBasicInfoOnly = true);
+	static bool loadPlayerByName(std::shared_ptr<Player> player, const std::string &name, bool loadBasicInfoOnly = true);
+	static bool loadPlayer(std::shared_ptr<Player> player, DBResult_ptr result, bool loadBasicInfoOnly = false);
 	static bool savePlayer(std::shared_ptr<Player> player);
 	static uint32_t getGuidByName(const std::string &name);
 	static bool getGuidByNameEx(uint32_t &guid, bool &specialVip, std::string &name);