Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Balance64 to WalletInfoCallback #607

Merged
merged 2 commits into from
Nov 4, 2018
Merged

Add Balance64 to WalletInfoCallback #607

merged 2 commits into from
Nov 4, 2018

Conversation

JustArchi
Copy link
Contributor

This can be needed if int won't be enough to display the account balance, i.e. when balance reaches more than 21474836.47 in local currency.

Unlikely to happen, but won't hurt to add considering Steam provides this.

@voided
Copy link
Member

voided commented Nov 3, 2018

LongBalance maybe? To match convention with things like Array.LongLength.

@JustArchi
Copy link
Contributor Author

Sure, why not, if you consider it better.

@codecov-io
Copy link

codecov-io commented Nov 3, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@141aa3b). Click here to learn what that means.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #607   +/-   ##
=========================================
  Coverage          ?   23.46%           
=========================================
  Files             ?       85           
  Lines             ?     8675           
  Branches          ?      719           
=========================================
  Hits              ?     2036           
  Misses            ?     6508           
  Partials          ?      131
Impacted Files Coverage Δ
...t2/SteamKit2/Steam/Handlers/SteamUser/Callbacks.cs 3.03% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 141aa3b...208e65c. Read the comment docs.

/// <summary>
/// Gets the balance of the wallet, in cents.
/// Gets the balance of the wallet as a 32-bit integer, in cents.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know that stating data type in the doc is a bit excessive and unneeded, but I wanted to point out the difference between those two for people that might not recall at first. I think it's OK in this case where consumer might wonder which one to use.

Copy link
Member

Choose a reason for hiding this comment

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

Do we know what actually happens to this field when an account has a balance of $42m?

Copy link
Member

@xPaw xPaw Nov 4, 2018

Choose a reason for hiding this comment

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

Is it in USD or users currency?

Because with vietnamese dong a game can cost 1.300.000₫ which is 130000000 cents.

This https://store.steampowered.com/sub/252061/?cc=vn costs 1359250000 cents in dong.

Copy link
Member

Choose a reason for hiding this comment

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

Well do we know what happens in that scenario too?

Copy link
Member

@xPaw xPaw Nov 4, 2018

Choose a reason for hiding this comment

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

No clue, but I pointed it out to ease the checking it out if anyone wants to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't check myself but very likely it caps at int.MaxValue and stays there. This would be the solution that makes the most sense in terms of backwards compatibility.

Copy link
Member

Choose a reason for hiding this comment

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

@JustArchi Just saw your comment - yeah, but it's Valve. I'd be quite unsurprised if it's just the lower 32 bits of the actual balance, and wraps around to the negatives.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yaakov-h you're totally right and it also wouldn't shock me, but I'm not rich enough to check 😀.

And BTW yes, this balance is in local currency (ECurrencyCode) which is user's local currency, if supported by Steam. It counts from full cents or other smallest monetary value.

@yaakov-h yaakov-h added this to the 2.2.0 milestone Nov 4, 2018
@yaakov-h
Copy link
Member

yaakov-h commented Nov 4, 2018

I think "in cents" in the xmldoc is slightly misleading, because with some ECurrencyCode values there are no cents... but that's how the old xmldoc was, so ¯\(ツ)/¯.

@xPaw
Copy link
Member

xPaw commented Nov 4, 2018

because with some ECurrencyCode values there are no cents

They're all still multiplied by 100, aren't they?

@yaakov-h
Copy link
Member

yaakov-h commented Nov 4, 2018

I don't think Yen has a hundredth value.

@yaakov-h yaakov-h merged commit b018ab8 into SteamRE:master Nov 4, 2018
@JustArchi
Copy link
Contributor Author

JustArchi commented Nov 4, 2018

It depends on the currency. For "traditional" western currencies like USD, CAD, PLN, EUR, the minimum monetary value is 0.01 of that currency and this amount is what Steam uses as integer. In other words, 10 USD will have ECurrencyCode of USD and 1000 as a balance (and balance64).

Not all currencies follow traditional way though, yen is a good example of that, there is no 0.01th of yen. I'm actually wondering now if Valve decided to go easy route and just record everything as * 100, or there is per-currency-parsing where yen balance should not be divided by 100 to present actual one.

If somebody is interested in checking that out, I'd be interested in knowing that as well, because that routine has to be included in the Steam client itself. If there is more logic to that, it could be a good idea to reverse-engineer it and add directly in SK2's callback for easier access to actual balance. Right now this is just a wrapper over protobuf message - works fine, but the logic is expected from the consumer.

@JustArchi JustArchi deleted the patch-2 branch November 4, 2018 23:49
@yaakov-h
Copy link
Member

yaakov-h commented Nov 5, 2018

I'm actually wondering now if Valve decided to go easy route and just record everything as * 100, or there is per-currency-parsing where yen balance should not be divided by 100 to present actual one.

From IRC:

08:45 <+xPaw>  Netshroud, https://github.com/SteamDatabase/SteamTracking/blob/85cd023702c77d200245f14bbc040e11003bf7f7/Scripts/Community/global.js#L307
08:45 <+xPaw>  they store all currencies as 'cents', even if said currency has no hundreds
08:46 <Netshroud>  so they store a hundredth of a yen? O_o
08:47 <+xPaw>  its multiplied for storage
08:47 <+xPaw>  divided for display
08:47 <Netshroud>  bWholeUnitsOnly
08:48 <Netshroud>  oh come on
08:48 <+xPaw>  yeah thats solely for display
08:48 <Netshroud>  that doesn't affect multiplier, just the trailing ".00"
08:48 <Netshroud>  VALVE!!

@JustArchi
Copy link
Contributor Author

I was afraid they might go this route... 😀

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants