Skip to content

Commit

Permalink
Fixed market offers duplication money and items (#521)
Browse files Browse the repository at this point in the history 
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)
  • Loading branch information
@dudantas
dudantas authored Oct 5, 2022
1 parent 4e49d5b commit 9dd8a10
Show file tree
Hide file tree
Showing 13 changed files with 101 additions and 86 deletions.
2 changes: 1 addition & 1 deletion data/migrations/0.lua
View file
Original file line number Diff line number Diff line change
@@ -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`,
Expand Down
7 changes: 4 additions & 3 deletions data/migrations/1.lua
View file
Original file line number Diff line number Diff line change
@@ -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
5 changes: 5 additions & 0 deletions data/migrations/2.lua
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
-- return true = There are others migrations file
-- return false = This is the last migration file
function onUpdateDatabase()
return false
end
6 changes: 3 additions & 3 deletions schema.sql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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` (
Expand Down Expand Up @@ -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,
Expand All @@ -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`),
Expand Down
6 changes: 3 additions & 3 deletions src/creatures/creatures_definitions.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ struct RecentPvPKillEntry {
};

struct MarketOffer {
uint32_t price;
uint64_t price;
uint32_t timestamp;
uint16_t amount;
uint16_t counter;
Expand All @@ -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;
Expand All @@ -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;
Expand Down
77 changes: 51 additions & 26 deletions src/game/game.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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";
Expand Down Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions src/game/game.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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; }

Expand Down Expand Up @@ -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;
Expand Down
14 changes: 7 additions & 7 deletions src/io/iomarket.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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");
Expand All @@ -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"));
Expand Down Expand Up @@ -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) {
Expand All @@ -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 << ')';
Expand All @@ -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 ("
Expand All @@ -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;
}

Expand Down
4 changes: 2 additions & 2 deletions src/io/iomarket.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/lua/functions/creatures/npc/npc_functions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
39 changes: 16 additions & 23 deletions src/lua/functions/creatures/player/guild_functions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
9 changes: 1 addition & 8 deletions src/lua/functions/creatures/player/player_functions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Loading

2 comments on commit 9dd8a10

@travisani
Copy link
Contributor

@travisani travisani commented on 9dd8a10 Oct 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eu testei aqui, uma venda ao valor de 999999999999 gps, deu certo, contudo não constou no historico de compra/venda do item no market.

A compra no valor:
image

O item chegando:
image

O Historico de compra/venda:
image

E parece que valores como 9.999.999.999.999 GPS bugam o balance descontando errado (não testei a fundo ainda)

@dudantas
Copy link
Contributor Author

@dudantas dudantas commented on 9dd8a10 Oct 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eu testei aqui, uma venda ao valor de 999999999999 gps, deu certo, contudo não constou no historico de compra/venda do item no market.

A compra no valor: image

O item chegando: image

O Historico de compra/venda: image

E parece que valores como 9.999.999.999.999 GPS bugam o balance descontando errado (não testei a fundo ainda)

Quanto ao bug do balance, eu testei, e até onde me lembro, retira o valor correto, pois a função que remove e adiciona dinheiro também é uint64_t e chega no limite de 999,999,999,999
Mas se você encontrar alguma incongruência, é só dizer.

PS: Por favor, não utilizar os commits para reportar problemas. Devemos testar enquanto estiver em pr, caso encontre algum problema depois de mergeado, abra um issue, pois dificilmente iremos ir nos commits acompanhar conversas, o local correto de acompanhar os problemas é nos issues, assim evita que a gente acabe esquecendo de resolver algo...

Please sign in to comment.