Skip to content
This repository has been archived by the owner on Apr 29, 2020. It is now read-only.

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

Open
2 tasks done
Hokuten85 opened this issue Nov 15, 2019 · 5 comments
Open
2 tasks done

Comments

@Hokuten85
Copy link
Contributor

I have:

  • 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
  • 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;

@SirGouki
Copy link

SirGouki commented Jan 9, 2020

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)

@Hokuten85
Copy link
Contributor Author

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.

@KnowOne134
Copy link
Contributor

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

@TeoTwawki
Copy link
Member

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

TeoTwawki added a commit that referenced this issue Feb 6, 2020
 change amount values from uint8 to uint32 fixes issue #6282
Hokuten85 pushed a commit to Hokuten85/Ivalice-old that referenced this issue Mar 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants