TFS 1.X+ onTradeAccept bug?

[mackerel]

I was just getting familiar with new function Player:onTradeAccept(target, item, targetItem) and I believe there is a bug. I mean, it does work as it should with one exception.

The function is executing even when the trade has not been successful due to lack of space or similar.

Is there an easy fix for this?

 

[zbizu]

either writing a new creatureevent(onTradeSuccess) or moving event execution (game.cpp) lower (you need only this g_events->eventPlayerOnTradeAccept(player, tradePartner, playerTradeItem, partnerTradeItem)):

if (!g_events->eventPlayerOnTradeAccept(player, tradePartner, playerTradeItem, partnerTradeItem)) {
            internalCloseTrade(player);
            return;
        }

and rewrite if (!isSuccess) { statement to execute onTradeAccept when the trade is successful

 

[mackerel]

either writing a new creatureevent(onTradeSuccess) or moving event execution (game.cpp) lower (you need only this g_events->eventPlayerOnTradeAccept(player, tradePartner, playerTradeItem, partnerTradeItem)):

if (!g_events->eventPlayerOnTradeAccept(player, tradePartner, playerTradeItem, partnerTradeItem)) {
            internalCloseTrade(player);
            return;
        }

and rewrite if (!isSuccess) { statement to execute onTradeAccept when the trade is successful

But if you only use this:

!g_events->eventPlayerOnTradeAccept(player, tradePartner, playerTradeItem, partnerTradeItem)

You wont be able to use boolean return values within Lua

/edit
Actually, I think I am overthinking this lol. Thanks a lot

 

[zbizu]

if(isSuccess){

// execute event

} else {

// things from !isSuccess

}

now you don't need to return anything because return value (true/false) was used to allow or disallow trade, but here it's already executed so there's nothing else to do

 

[mackerel]

if(isSuccess){

// execute event

} else {

// things from !isSuccess

}

now you don't need to return anything because return value (true/false) was used to allow or disallow trade, but here it's already executed so there's nothing else to do

Yes , it's just that you could return false in your lua script to cancel the whole trade. It's not really useful for me anyway so its cool.

if someone needs it:

void Game::playerAcceptTrade(uint32_t playerId)
{
    Player* player = getPlayerByID(playerId);
    if (!player) {
        return;
    }

    if (!(player->getTradeState() == TRADE_ACKNOWLEDGE || player->getTradeState() == TRADE_INITIATED)) {
        return;
    }

    Player* tradePartner = player->tradePartner;
    if (!tradePartner) {
        return;
    }

    if (!canThrowObjectTo(tradePartner->getPosition(), player->getPosition())) {
        player->sendCancelMessage(RETURNVALUE_CREATUREISNOTREACHABLE);
        return;
    }

    player->setTradeState(TRADE_ACCEPT);

    if (tradePartner->getTradeState() == TRADE_ACCEPT) {
        Item* playerTradeItem = player->tradeItem;
        Item* partnerTradeItem = tradePartner->tradeItem;

        player->setTradeState(TRADE_TRANSFER);
        tradePartner->setTradeState(TRADE_TRANSFER);

        auto it = tradeItems.find(playerTradeItem);
        if (it != tradeItems.end()) {
            ReleaseItem(it->first);
            tradeItems.erase(it);
        }

        it = tradeItems.find(partnerTradeItem);
        if (it != tradeItems.end()) {
            ReleaseItem(it->first);
            tradeItems.erase(it);
        }

        bool isSuccess = false;

        ReturnValue tradePartnerRet = internalAddItem(tradePartner, playerTradeItem, INDEX_WHEREEVER, 0, true);
        ReturnValue playerRet = internalAddItem(player, partnerTradeItem, INDEX_WHEREEVER, 0, true);
        if (tradePartnerRet == RETURNVALUE_NOERROR && playerRet == RETURNVALUE_NOERROR) {
            playerRet = internalRemoveItem(playerTradeItem, playerTradeItem->getItemCount(), true);
            tradePartnerRet = internalRemoveItem(partnerTradeItem, partnerTradeItem->getItemCount(), true);
            if (tradePartnerRet == RETURNVALUE_NOERROR && playerRet == RETURNVALUE_NOERROR) {
                tradePartnerRet = internalMoveItem(playerTradeItem->getParent(), tradePartner, INDEX_WHEREEVER, playerTradeItem, playerTradeItem->getItemCount(), nullptr, FLAG_IGNOREAUTOSTACK, nullptr, partnerTradeItem);
                if (tradePartnerRet == RETURNVALUE_NOERROR) {
                    internalMoveItem(partnerTradeItem->getParent(), player, INDEX_WHEREEVER, partnerTradeItem, partnerTradeItem->getItemCount(), nullptr, FLAG_IGNOREAUTOSTACK);
                    playerTradeItem->onTradeEvent(ON_TRADE_TRANSFER, tradePartner);
                    partnerTradeItem->onTradeEvent(ON_TRADE_TRANSFER, player);
                    g_events->eventPlayerOnTradeAccept(player, tradePartner, playerTradeItem, partnerTradeItem);
                    isSuccess = true;
                }
            }
        }

        if (!isSuccess) {
            std::string errorDescription;

            if (tradePartner->tradeItem) {
                errorDescription = getTradeErrorDescription(tradePartnerRet, playerTradeItem);
                tradePartner->sendTextMessage(MESSAGE_EVENT_ADVANCE, errorDescription);
                tradePartner->tradeItem->onTradeEvent(ON_TRADE_CANCEL, tradePartner);
            }

            if (player->tradeItem) {
                errorDescription = getTradeErrorDescription(playerRet, partnerTradeItem);
                player->sendTextMessage(MESSAGE_EVENT_ADVANCE, errorDescription);
                player->tradeItem->onTradeEvent(ON_TRADE_CANCEL, player);
            }
        }

        player->setTradeState(TRADE_NONE);
        player->tradeItem = nullptr;
        player->tradePartner = nullptr;
        player->sendTradeClose();

        tradePartner->setTradeState(TRADE_NONE);
        tradePartner->tradeItem = nullptr;
        tradePartner->tradePartner = nullptr;
        tradePartner->sendTradeClose();
    }
}

 

[zbizu]

https://github.com/otland/forgottenserver/pull/2970/files