The Wayback Machine - https://web.archive.org/web/20201015052800/https://github.com/SteamRE/SteamKit/issues/574
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

Steam2 Universes #574

Open
strafe opened this issue Jul 17, 2018 · 11 comments
Open

Steam2 Universes #574

strafe opened this issue Jul 17, 2018 · 11 comments
Labels

Comments

@strafe
Copy link

@strafe strafe commented Jul 17, 2018

Couple of questions about the SteamID class.

  1. Why must you supply an EUniverse when using the SetFromString method? I was under the impression the ID always contains the universe? xPaw's PHP implementation currently pulls the universe from the string.
  2. Why does the RenderSteam2 method automatically convert accounts that are in universe 1 to universe 0? This logic seems backwards as universe 0 is invalid and is only thought to be correct due to a bug(?). Again xPaw's PHP implementation corrects this the other way.

Thank you.

@yaakov-h yaakov-h added the question label Jul 18, 2018
@yaakov-h
Copy link
Member

@yaakov-h yaakov-h commented Jul 18, 2018

I believe (2) is for backwards compatibility, to match our old behaviour - and Steam's old behaviour, and Source's old behaviour...

I'm not sure about (1). I assume it was that way in the original C++ code that was ported, but I'd have to check.

@strafe
Copy link
Author

@strafe strafe commented Jul 18, 2018

I believe (2) is for backwards compatibility, to match our old behaviour - and Steam's old behaviour, and Source's old behaviour...

I see, don't think you think it would make more sense to provide the correct behaviour? Supposedly according to some posts (which I won't treat as gospel) any game that incorrectly used 0 instead of 1 now uses SteamID 3 internally instead and any games that didn't (such as CS:GO) still use it.

I'm not sure about (1). I assume it was that way in the original C++ code that was ported, but I'd have to check.

I see, I'm not familiar with the history of SteamKit at all.


Is the project open to changing behaviour with these things in at least some way (parameters to select behaviour) or are they too large?

@yaakov-h
Copy link
Member

@yaakov-h yaakov-h commented Jul 18, 2018

We tend to make breaking changes fairly regularly. 😢

@strafe
Copy link
Author

@strafe strafe commented Jul 18, 2018

We tend to make breaking changes fairly regularly. cry

Ah good, not sure if this is just stalled now then. Maybe @xPaw or @asherkin could chime in.

@JustArchi
Copy link
Contributor

@JustArchi JustArchi commented Jul 18, 2018

I don't see why we can't deprecate existing function and code new one with one less argument.

As of 2: you could do the same by adding another function with one more argument such as EUniverse invalid = EUniverse.Public or something similar.

@xPaw
Copy link
Contributor

@xPaw xPaw commented Jul 18, 2018

My PHP implementation is based on inspecting multiple Valve's implementations (steamworks sdk, 2007 source leak, a recent private leak from the source engine for partners, and looking at steamclient in IDA).

All these implementations sadly have discrepancies on Valve's side, so what I have there I think is the most reasonable approach to handling it.

@psychonic
Copy link
Member

@psychonic psychonic commented Jul 18, 2018

any game that incorrectly used 0 instead of 1 now uses SteamID 3 internally instead and any games that didn't (such as CS:GO) still use it.

That's not 100% accurate, but it's close. Older third-party Source games and Valve's older SDK bases still use the 0.

@asherkin
Copy link
Member

@asherkin asherkin commented Jul 18, 2018

Gut tells me that parsing should take 0 or 1 as Public (and drop the override), and that rendering should default to emitting Public as 1 but allow overriding to 0 for only Steam2. It is a breaking change on rendering, but I doubt it'll actually break anyone's real code.

@yaakov-h
Copy link
Member

@yaakov-h yaakov-h commented Jul 18, 2018

I bet someone is using that as a DB key somewhere... but we did already break the default to render Steam3 instead of Steam2.

@yaakov-h
Copy link
Member

@yaakov-h yaakov-h commented May 23, 2019

Revisiting this.

We could just ignore it, or:

  • Remove EUniverse from SetFromString, and infer it from the first digit.
  • Drop the boolean parameter from Render(), only render Steam3.
  • Replace private RenderSteam2() with public RenderSteam2(SteamIDCompatibility) where SteamIDCompatibility is either a bool, or an enum with two values along the lines of Pre2009 and Post2009.

Thoughts?

@strafe
Copy link
Author

@strafe strafe commented May 24, 2019

@yaakov-h I'm not qualified to comment on the best solution, however, it would be great if this was not ignored and SteamKit offered the objectively correct behaviour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
6 participants
You can’t perform that action at this time.