Bug in trading coins between players

[Windcatcher]

I've verified this in 0.5.5, and the code in 0.5.8 is the same. It also looks like it will rear its head in 0.6.1, though I haven't tried it.

- Player 1 hands a copper piece to player 2
- Player 2 clicks accept
- Player 1 clicks accept

Player 1 has the money deducted, but player 2 never receives it. The problem has to do with who accepts the trade first. When player 2 accepts first, the trade isn't completed until player 1 also accepts. The server code calls FinishTrade() for player 1 first, since it is his acceptance that caused the trade to finally complete. The problem is that FinishTrade() calls trade->Reset() before it completes, which zeroes out the coin amounts. Therefore, when the server gets around to calling FinishTrade() for player 2, the coin amounts have been zeroed, and player 2 never gets the money.

Basically, the person who first accepts the trade can never receive money in the trade. The solution, as I see it, is that trade->Reset() should be called separately from FinishTrade(), once for each player, after all calls to FinishTrade() have completed.

[fathernitwit]

hey Windcatcher,

thanks for the detailed report... good to see your still lurking around somewhere.

The code which calls FinishTrade() allready takes care of calling Reset right after both finishes, so that code is fine, the only thing which needed adjusting was the code to trade with NPCs, which needed a Reset after the finish. Then you can kill the Reset in Finish Trade:


Index: trading.cpp
================================================== ================
--- trading.cpp (revision 483)
+++ trading.cpp (working copy)
@@ -428,8 +428,7 @@
// Money @merth: look into how NPC's receive cash
this->AddMoneyToPP(other->trade->cp, other->trade->sp, other->trade->gp,
other->trade->pp, true);

- // Clear trade inventory
- trade->Reset();
+ //Do not reset the trade here, done by the caller.
}

bool Client::CheckTradeLoreConflict(Client* other)
Index: client_packet.cpp
================================================== =================
--- client_packet.cpp (revision 483)
+++ client_packet.cpp (working copy)
@@ -2966,10 +2967,12 @@
}
}
else if(with){
+ //trading with an NPC
EQZonePacket* outapp = new EQZonePacket(OP_FinishTrade,0);
QueuePacket(outapp);
safe_delete(outapp);
FinishTrade(with->CastToNPC());
+ trade->Reset();
}

return;

[SCROFT]

Was this ever fixed?

I have tried the work around in the first post and still the coins are lost in the transfer.

Trade between 2 Player Chars
Windows server
MySql 4.x
7.0 and PEQ via DRP Server Pack

[WARR10R]

I have found that the transfer only shows after you zone.
So it is kind of working. You just need to zone after the transfer.
Then the money should appear in your inventory.

[SCROFT]

thanks for the work around... will give that a try.