Fixed market offers duplication money and items (#521)

Resolves #517
Resolves #520
Resolves #538

Global sync pr: https://github.com/opentibiabr/otservbr-global/pull/797/files

Changed max market offer price from uint32_t to uint64_t (from 999999999 to 999999999999 client limitation)

data/migrations/0.lua

@@ -1,5 +1,5 @@
 function onUpdateDatabase()
-	Spdlog.info("Updating database to version 19 (Prey system rework + Task hunting system)")
+	Spdlog.info("Updating database to version 1 (Prey system rework + Task hunting system)")
 	db.query([[
 		ALTER TABLE `players`
 			DROP `prey_stamina_1`,

data/migrations/1.lua

@@ -1,5 +1,6 @@
--- return true = There are others migrations file
--- return false = This is the last migration file
 function onUpdateDatabase()
-    return false
+	Spdlog.info("Updating database to version 2 (Fix market price size)")
+	db.query("ALTER TABLE `market_history` CHANGE `price` `price` BIGINT(20) UNSIGNED NOT NULL DEFAULT '0';")
+	db.query("ALTER TABLE `market_offers` CHANGE `price` `price` BIGINT(20) UNSIGNED NOT NULL DEFAULT '0';")
+	return true
 end

data/migrations/2.lua

@@ -0,0 +1,5 @@
+-- return true = There are others migrations file
+-- return false = This is the last migration file
+function onUpdateDatabase()
+    return false
+end

schema.sql

@@ -7,7 +7,7 @@ CREATE TABLE IF NOT EXISTS `server_config` (
     CONSTRAINT `server_config_pk` PRIMARY KEY (`config`)
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8;
 
-INSERT INTO `server_config` (`config`, `value`) VALUES ('db_version', '1'), ('motd_hash', ''), ('motd_num', '0'), ('players_record', '0');
+INSERT INTO `server_config` (`config`, `value`) VALUES ('db_version', '2'), ('motd_hash', ''), ('motd_num', '0'), ('players_record', '0');
 
 -- Table structure `accounts`
 CREATE TABLE IF NOT EXISTS `accounts` (
@@ -423,7 +423,7 @@ CREATE TABLE IF NOT EXISTS `market_history` (
     `sale` tinyint(1) NOT NULL DEFAULT '0',
     `itemtype` int(10) UNSIGNED NOT NULL,
     `amount` smallint(5) UNSIGNED NOT NULL,
-    `price` int(10) UNSIGNED NOT NULL DEFAULT '0',
+    `price` bigint(20) UNSIGNED NOT NULL DEFAULT '0',
     `expires_at` bigint(20) UNSIGNED NOT NULL,
     `inserted` bigint(20) UNSIGNED NOT NULL,
     `state` tinyint(1) UNSIGNED NOT NULL,
@@ -443,7 +443,7 @@ CREATE TABLE IF NOT EXISTS `market_offers` (
     `amount` smallint(5) UNSIGNED NOT NULL,
     `created` bigint(20) UNSIGNED NOT NULL,
     `anonymous` tinyint(1) NOT NULL DEFAULT '0',
-    `price` int(10) UNSIGNED NOT NULL DEFAULT '0',
+    `price` bigint(20) UNSIGNED NOT NULL DEFAULT '0',
     INDEX `sale` (`sale`,`itemtype`),
     INDEX `created` (`created`),
     INDEX `player_id` (`player_id`),

src/creatures/creatures_definitions.hpp

@@ -672,7 +672,7 @@ struct RecentPvPKillEntry {
 };
 
 struct MarketOffer {
-	uint32_t price;
+	uint64_t price;
 	uint32_t timestamp;
 	uint16_t amount;
 	uint16_t counter;
@@ -696,7 +696,7 @@ struct MarketOfferEx {
 	uint32_t id;
 	uint32_t playerId;
 	uint32_t timestamp;
-	uint32_t price;
+	uint64_t price;
 	uint16_t amount;
 	uint16_t counter;
 	uint16_t itemId;
@@ -706,7 +706,7 @@ struct MarketOfferEx {
 
 struct HistoryMarketOffer {
 	uint32_t timestamp;
-	uint32_t price;
+	uint64_t price;
 	uint16_t itemId;
 	uint16_t amount;
 	MarketOfferState_t state;

src/game/game.cpp

@@ -1952,7 +1952,7 @@ ReturnValue Game::internalRemoveItem(Item* item, int32_t count /*= -1*/, bool te
 {
 	Cylinder* cylinder = item->getParent();
 	if (cylinder == nullptr) {
-		SPDLOG_DEBUG("{} - Cylinder is nullpt", __FUNCTION__);
+		SPDLOG_DEBUG("{} - Cylinder is nullptr", __FUNCTION__);
 		return RETURNVALUE_NOTPOSSIBLE;
 	}
 	Tile* fromTile = cylinder->getTile();
@@ -7642,6 +7642,10 @@ void removeOfferItems(Player &player, DepotLocker &depotLocker, const ItemType &
 		auto [itemVector, itemMap] = player.requestLockerItems(&depotLocker);
 		uint32_t count = 0;
 		for (auto item : itemVector) {
+			if (itemType.id != item->getID()) {
+				continue;
+			}
+
 			if (itemType.stackable) {
 				uint16_t removeCount = std::min<uint16_t>(removeAmount, item->getItemCount());
 				removeAmount -= removeCount;
@@ -7676,67 +7680,88 @@ void removeOfferItems(Player &player, DepotLocker &depotLocker, const ItemType &
 	}
 }
 
-void Game::playerCreateMarketOffer(uint32_t playerId, uint8_t type, uint16_t itemId, uint16_t amount, uint32_t price, bool anonymous)
+bool checkCanInitCreateMarketOffer(Player *player, uint8_t type, const ItemType &it, uint16_t amount, uint64_t price, std::string offerStatus)
 {
-	// Before creating the offer we will compare it with the RETURN VALUE ERROR
-	std::string offerStatus = "No error.";
-	if (price == 0 || price > 999999999) {
-		offerStatus = "Failed to process price";
-		return;
-	}
-
-	if (type != MARKETACTION_BUY && type != MARKETACTION_SELL) {
-		offerStatus = "Failed to process type";
-		return;
-	}
-
-	Player *player = getPlayerByID(playerId);
 	if (!player) {
 		offerStatus = "Failed to load player";
-		return;
+		return false;
 	}
 
 	if (!player->isInMarket()) {
 		offerStatus = "Failed to load market";
-		return;
+		return false;
+	}
+
+	if (price == 0) {
+		SPDLOG_ERROR("{} - Player with name {} selling offer with a invalid price", __FUNCTION__, player->getName());
+		offerStatus = "Failed to process price";
+		return false;
+	}
+
+	if (price > 999999999999) {
+		SPDLOG_ERROR("{} - Player with name {} is trying to sell an item with a higher than allowed value", __FUNCTION__, player->getName());
+		offerStatus = "Player is trying to sell an item with a higher than allowed value";
+		return false;
+	}
+
+	if (type != MARKETACTION_BUY && type != MARKETACTION_SELL) {
+		offerStatus = "Failed to process type";
+		return false;
 	}
 
 	// Check market exhausted
 	if (player->isMarketExhausted()) {
 		player->sendCancelMessage(RETURNVALUE_YOUAREEXHAUSTED);
 		g_game().addMagicEffect(player->getPosition(), CONST_ME_POFF);
 		offerStatus = "Market exhausted";
-		return;
+		return false;
 	}
 
-	const ItemType &it = Item::items[itemId];
 	if (it.id == 0 || it.wareId == 0) {
 		offerStatus = "Failed to load offer or item id";
-		return;
+		return false;
 	}
 
 	if (amount == 0 || !it.stackable && amount > 2000 || it.stackable && amount > 64000) {
 		SPDLOG_ERROR("{} - Player: {} invalid offer amount: {}", __FUNCTION__, player->getName(), amount);
 		offerStatus = "Failed to load amount";
-		return;
+		return false;
 	}
 	SPDLOG_DEBUG("{} - Offer amount: {}", __FUNCTION__, amount);
 
 	if (g_configManager().getBoolean(MARKET_PREMIUM) && !player->isPremium()) {
 		player->sendTextMessage(MESSAGE_MARKET, "Only premium accounts may create offers for that object.");
 		offerStatus = "Only premium can create offers";
-		return;
+		return false;
 	}
 
 	const uint32_t maxOfferCount = g_configManager().getNumber(MAX_MARKET_OFFERS_AT_A_TIME_PER_PLAYER);
 	if (maxOfferCount != 0 && IOMarket::getPlayerOfferCount(player->getGUID()) >= maxOfferCount) {
 		offerStatus = "Excedeed max offer count";
+		return false;
+	}
+
+	return true;
+}
+
+void Game::playerCreateMarketOffer(uint32_t playerId, uint8_t type, uint16_t itemId, uint16_t amount, uint64_t price, bool anonymous)
+{
+	// Initialize variables
+	// Before creating the offer we will compare it with the RETURN VALUE ERROR
+	std::string offerStatus = "No error.";
+	Player *player = getPlayerByID(playerId);
+	const ItemType &it = Item::items[itemId];
+
+	// Make sure everything is ok before the create market offer starts
+	if (!checkCanInitCreateMarketOffer(player, type, it, amount, price, offerStatus)) {
+		SPDLOG_ERROR("{} - Player {} had an error on init offer on the market, error code: {}", __FUNCTION__, player->getName(), offerStatus);
 		return;
 	}
 
-	uint64_t calcFee = (price / 100.) *amount;
-	uint32_t minFee = std::min<uint32_t> (100000, calcFee);
-	uint32_t fee = std::max<uint32_t> (20, minFee);
+	uint64_t calcFee = (price / 100) * amount;
+	uint64_t minFee = std::min<uint64_t>(100000, calcFee);
+	uint64_t fee = std::max<uint64_t>(20, minFee);
+
 	if (type == MARKETACTION_SELL) {
 		if (fee > (player->getBankBalance() + player->getMoney())) {
 			offerStatus = "Fee is greater than player money";
@@ -7829,7 +7854,7 @@ void Game::playerCancelMarketOffer(uint32_t playerId, uint32_t timestamp, uint16
 	}
 
 	if (offer.type == MARKETACTION_BUY) {
-		player->setBankBalance( player->getBankBalance() + static_cast<uint64_t>(offer.price) * offer.amount);
+		player->setBankBalance( player->getBankBalance() + offer.price * offer.amount);
 		// Send market window again for update stats
 		player->sendMarketEnter(player->getLastDepotId());
 	} else {

src/game/game.h

@@ -379,7 +379,7 @@ class Game
 		void playerBrowseMarket(uint32_t playerId, uint16_t itemId);
 		void playerBrowseMarketOwnOffers(uint32_t playerId);
 		void playerBrowseMarketOwnHistory(uint32_t playerId);
-		void playerCreateMarketOffer(uint32_t playerId, uint8_t type, uint16_t itemId, uint16_t amount, uint32_t price, bool anonymous);
+		void playerCreateMarketOffer(uint32_t playerId, uint8_t type, uint16_t itemId, uint16_t amount, uint64_t price, bool anonymous);
 		void playerCancelMarketOffer(uint32_t playerId, uint32_t timestamp, uint16_t counter);
 		void playerAcceptMarketOffer(uint32_t playerId, uint32_t timestamp, uint16_t counter, uint16_t amount);
 		void playerStoreOpen(uint32_t playerId, uint8_t serviceType);
@@ -468,7 +468,7 @@ class Game
 
 		void sendOfflineTrainingDialog(Player* player);
 
-		const std::map<uint16_t, uint32_t>& getItemsPrice() const { return itemsPriceMap; }
+		const std::map<uint16_t, uint64_t>& getItemsPrice() const { return itemsPriceMap; }
 		const phmap::flat_hash_map<uint32_t, Player*>& getPlayers() const { return players; }
 		const std::map<uint32_t, Npc*>& getNpcs() const { return npcs; }
 
@@ -656,7 +656,7 @@ class Game
 		std::string motdHash;
 		uint32_t motdNum = 0;
 
-		std::map<uint16_t, uint32_t> itemsPriceMap;
+		std::map<uint16_t, uint64_t> itemsPriceMap;
 		uint16_t itemsSaleCount;
 
 		std::vector<ItemClassification*> itemsClassifications;

src/io/iomarket.cpp

@@ -44,7 +44,7 @@ MarketOfferList IOMarket::getActiveOffers(MarketAction_t action, uint16_t itemId
 	do {
 		MarketOffer offer;
 		offer.amount = result->getNumber<uint16_t>("amount");
-		offer.price = result->getNumber<uint32_t>("price");
+		offer.price = result->getNumber<uint64_t>("price");
 		offer.timestamp = result->getNumber<uint32_t>("created") + marketOfferDuration;
 		offer.counter = result->getNumber<uint32_t>("id") & 0xFFFF;
 		if (result->getNumber<uint16_t>("anonymous") == 0) {
@@ -74,7 +74,7 @@ MarketOfferList IOMarket::getOwnOffers(MarketAction_t action, uint32_t playerId)
 	do {
 		MarketOffer offer;
 		offer.amount = result->getNumber<uint16_t>("amount");
-		offer.price = result->getNumber<uint32_t>("price");
+		offer.price = result->getNumber<uint64_t>("price");
 		offer.timestamp = result->getNumber<uint32_t>("created") + marketOfferDuration;
 		offer.counter = result->getNumber<uint32_t>("id") & 0xFFFF;
 		offer.itemId = result->getNumber<uint16_t>("itemtype");
@@ -99,7 +99,7 @@ HistoryMarketOfferList IOMarket::getOwnHistory(MarketAction_t action, uint32_t p
 		HistoryMarketOffer offer;
 		offer.itemId = result->getNumber<uint16_t>("itemtype");
 		offer.amount = result->getNumber<uint16_t>("amount");
-		offer.price = result->getNumber<uint32_t>("price");
+		offer.price = result->getNumber<uint64_t>("price");
 		offer.timestamp = result->getNumber<uint32_t>("expires_at");
 
 		MarketOfferState_t offerState = static_cast<MarketOfferState_t>(result->getNumber<uint16_t>("state"));
@@ -235,7 +235,7 @@ MarketOfferEx IOMarket::getOfferByCounter(uint32_t timestamp, uint16_t counter)
 	offer.amount = result->getNumber<uint16_t>("amount");
 	offer.counter = result->getNumber<uint32_t>("id") & 0xFFFF;
 	offer.timestamp = result->getNumber<uint32_t>("created");
-	offer.price = result->getNumber<uint32_t>("price");
+	offer.price = result->getNumber<uint64_t>("price");
 	offer.itemId = result->getNumber<uint16_t>("itemtype");
 	offer.playerId = result->getNumber<uint32_t>("player_id");
 	if (result->getNumber<uint16_t>("anonymous") == 0) {
@@ -246,7 +246,7 @@ MarketOfferEx IOMarket::getOfferByCounter(uint32_t timestamp, uint16_t counter)
 	return offer;
 }
 
-void IOMarket::createOffer(uint32_t playerId, MarketAction_t action, uint32_t itemId, uint16_t amount, uint32_t price, bool anonymous)
+void IOMarket::createOffer(uint32_t playerId, MarketAction_t action, uint32_t itemId, uint16_t amount, uint64_t price, bool anonymous)
 {
 	std::ostringstream query;
 	query << "INSERT INTO `market_offers` (`player_id`, `sale`, `itemtype`, `amount`, `price`, `created`, `anonymous`) VALUES (" << playerId << ',' << action << ',' << itemId << ',' << amount << ',' << price << ',' << time(nullptr) << ',' << anonymous << ')';
@@ -267,7 +267,7 @@ void IOMarket::deleteOffer(uint32_t offerId)
 	Database::getInstance().executeQuery(query.str());
 }
 
-void IOMarket::appendHistory(uint32_t playerId, MarketAction_t type, uint16_t itemId, uint16_t amount, uint32_t price, time_t timestamp, MarketOfferState_t state)
+void IOMarket::appendHistory(uint32_t playerId, MarketAction_t type, uint16_t itemId, uint16_t amount, uint64_t price, time_t timestamp, MarketOfferState_t state)
 {
 	std::ostringstream query;
 	query << "INSERT INTO `market_history` (`player_id`, `sale`, `itemtype`, `amount`, `price`, `expires_at`, `inserted`, `state`) VALUES ("
@@ -294,7 +294,7 @@ bool IOMarket::moveOfferToHistory(uint32_t offerId, MarketOfferState_t state)
 		return false;
 	}
 
-	appendHistory(result->getNumber<uint32_t>("player_id"), static_cast<MarketAction_t>(result->getNumber<uint16_t>("sale")), result->getNumber<uint16_t>("itemtype"), result->getNumber<uint16_t>("amount"), result->getNumber<uint32_t>("price"), time(nullptr), state);
+	appendHistory(result->getNumber<uint32_t>("player_id"), static_cast<MarketAction_t>(result->getNumber<uint16_t>("sale")), result->getNumber<uint16_t>("itemtype"), result->getNumber<uint16_t>("amount"), result->getNumber<uint64_t>("price"), time(nullptr), state);
 	return true;
 }
 

src/io/iomarket.h

@@ -41,11 +41,11 @@ class IOMarket
 		static uint32_t getPlayerOfferCount(uint32_t playerId);
 		static MarketOfferEx getOfferByCounter(uint32_t timestamp, uint16_t counter);
 
-		static void createOffer(uint32_t playerId, MarketAction_t action, uint32_t itemId, uint16_t amount, uint32_t price, bool anonymous);
+		static void createOffer(uint32_t playerId, MarketAction_t action, uint32_t itemId, uint16_t amount, uint64_t price, bool anonymous);
 		static void acceptOffer(uint32_t offerId, uint16_t amount);
 		static void deleteOffer(uint32_t offerId);
 
-		static void appendHistory(uint32_t playerId, MarketAction_t type, uint16_t itemId, uint16_t amount, uint32_t price, time_t timestamp, MarketOfferState_t state);
+		static void appendHistory(uint32_t playerId, MarketAction_t type, uint16_t itemId, uint16_t amount, uint64_t price, time_t timestamp, MarketOfferState_t state);
 		static bool moveOfferToHistory(uint32_t offerId, MarketOfferState_t state);
 
 		void updateStatistics();

src/lua/functions/creatures/npc/npc_functions.cpp

@@ -540,7 +540,7 @@ int NpcFunctions::luaNpcSellItem(lua_State* L)
 		}
 	}
 
-    uint32_t itemsPurchased = 0;
+	uint32_t itemsPurchased = 0;
 	uint8_t backpacksPurchased = 0;
 	uint8_t internalCount = it.stackable ? 100 : 1;
 	auto remainingAmount = static_cast<uint32_t>(amount);

src/lua/functions/creatures/player/guild_functions.cpp

@@ -80,34 +80,27 @@ int GuildFunctions::luaGuildGetMembersOnline(lua_State* L) {
 }
 
 int GuildFunctions::luaGuildGetBankBalance(lua_State* L) {
-  // guild:getBankBalance()
-  Guild* guild = getUserdata<Guild>(L, 1);
-  if (guild) {
-	lua_pushnumber(L, guild->getBankBalance());
-  } else {
-	lua_pushnil(L);
-  }
-  return 1;
+	// guild:getBankBalance()
+	Guild* guild = getUserdata<Guild>(L, 1);
+	if (guild) {
+		lua_pushnumber(L, guild->getBankBalance());
+	} else {
+		lua_pushnil(L);
+	}
+	return 1;
 }
 
 int GuildFunctions::luaGuildSetBankBalance(lua_State* L) {
-  // guild:setBankBalance(bankBalance)
-  Guild* guild = getUserdata<Guild>(L, 1);
-  if (!guild) {
-	lua_pushnil(L);
-	return 1;
-  }
+	// guild:setBankBalance(bankBalance)
+	Guild* guild = getUserdata<Guild>(L, 1);
+	if (!guild) {
+		lua_pushnil(L);
+		return 1;
+	}
 
-  int64_t balance = getNumber<int64_t>(L, 2);
-  if (balance < 0) {
-	reportErrorFunc("Invalid bank balance value.");
-	lua_pushnil(L);
+	guild->setBankBalance(getNumber<uint64_t>(L, 2));
+	pushBoolean(L, true);
 	return 1;
-  }
-
-  guild->setBankBalance(balance);
-  pushBoolean(L, true);
-  return 1;
 }
 
 int GuildFunctions::luaGuildAddRank(lua_State* L) {

src/lua/functions/creatures/player/player_functions.cpp

@@ -1535,14 +1535,7 @@ int PlayerFunctions::luaPlayerSetBankBalance(lua_State* L) {
 		return 1;
 	}
 
-	int64_t balance = getNumber<int64_t>(L, 2);
-	if (balance < 0) {
-		reportErrorFunc("Invalid bank balance value.");
-		lua_pushnil(L);
-		return 1;
-	}
-
-	player->setBankBalance(balance);
+	player->setBankBalance(getNumber<uint64_t>(L, 2));
 	pushBoolean(L, true);
 	return 1;
 }