darkstar icon indicating copy to clipboard operation
darkstar copied to clipboard

Trade confirmation doesn't work with amounts over 255(applicable to Gil trades)

Open Hokuten85 opened this issue 5 years ago • 5 comments

I have:

  • [x] searched existing issues (http://github.com/darkstarproject/darkstar/issues/) to see if the issue I am posting has already been addressed or opened by another contributor
  • [x] checked the commit log to see if my issue has been resolved since my server was last updated

Client Version (type /ver in game) : 0.0.8319

Source Branch (master/stable) : master

Additional Information (Steps to reproduce/Expected behavior) : Should be able to reproduce this by attempting to confirmSlot or confirmItem on a trade involving Gil with an amount over 255, and then call trade:confirmTrade(). All of the amount parameters in the functions involved are defined as uint8.

src/map/trade_container.cpp getConfirmedStatus setConfirmedStatus

src/map/trade_container.h getConfirmedStatus setConfirmedStatus std::vector m_confirmed;

Hokuten85 avatar Nov 15 '19 14:11 Hokuten85

The reason is the data type UINT8, range is between 0 and 255. Type needs to be changed and proper value checking should be implemented. I would do it, but I'm trying to update everything right now, and I have to set things back up for a client for darkstar.

I looked at the source code for the version I already had (I last downloaded a zip around mar2018) and everything that involves quantities for items does use UINT32 instead of UINT8, but anything that says amount uses UINT8... could this be the problem? ( I only took a quick look)

SirGouki avatar Jan 09 '20 06:01 SirGouki

Yep. I've corrected this on my own server, but due to all the other funky customizations I've made it makes pull requests difficult.

Hokuten85 avatar Jan 09 '20 14:01 Hokuten85

Clone dsp again, make a new branch and pull request. Or change branches on your own server to newest dsp

KnowOne134 avatar Jan 09 '20 14:01 KnowOne134

in getConfirmedStatus that's the slot ID not the amount, so that one should be uint8 still.

TeoTwawki avatar Jan 16 '20 00:01 TeoTwawki

https://github.com/DarkstarProject/darkstar/pull/6382/files?utf8=%E2%9C%93&diff=unified&w=1

TeoTwawki avatar Jan 16 '20 00:01 TeoTwawki