8000 Generate GC base separately for each game by xPaw · Pull Request #180 · SteamRE/SteamKit · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Generate GC base separately for each game #180

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Oct 26, 2015
Merged

Generate GC base separately for each game #180

merged 6 commits into from
Oct 26, 2015

Conversation

xPaw
Copy link
Member
@xPaw xPaw commented Oct 23, 2015

I'm not going to regenerate them because it destroys them due to line endings (honk honk, someone doesn't want to normalize them)

@voided
Copy link
Member
voided commented Oct 24, 2015

I don't think I mind line endings on dumped/generated files at this point since SteamDB maintains a better record of history than we do.

@xPaw
Copy link
Member Author
xPaw commented Oct 24, 2015

Ok, I've regenerated everything and removed no longer existing protos, and also the non-generated EMsg enums for CSGO and TF2 that were super outdated.

MsgGCHdrProtoBuf in gamecoordinator.steamd uses Dota's CMsgProtoBufHeader, I've kept it that way, but should it stay that way?

To make it easier to review this PR, add ?w=1 to the url.

@Netshroud
Copy link

I'd assume the CMsgProtoBufHeader would be consistent across all GCs and the CMs, since it's in steammessages_base.proto and it appears to be a common structural definition.

If so, MsgGCHdrProtoBuf could, and perhaps should, just use SteamKit2.Internal.CMsgProtoBufHeader. Otherwise we could make it generic and the consumer would specify their header proto on a per-message / per-GC basis, but that seems like a lot of hassle for no value.

@xPaw
Copy link
Member Author
xPaw commented Oct 24, 2015

Dota's CMsgProtoBufHeader might not be compatible because it has 2 fields that steamclient doesn't and they seem directly related to GC stuff:

    optional .GCProtoBufMsgSrc gc_msg_src = 200 [default = GCProtoBufMsgSrc_Unspecified];
    optional uint32 gc_dir_index_source = 201;

Also, while I'm cleaning up proto stuff, is there any point in having protos in the repo that aren't used for anything? (portal, dod, etc).

@voided
Copy link
Member
voided commented Oct 24, 2015

I'm not sure if relying on a specific game's header is the right approach either.

One idea that crossed my mind is maintaining a copy of our own .proto with a copy of whatever header looks most up to date, and simply using that. This allows us to control our own field changes.

If some day games diverge enough to have differing header fields, we can look into allowing consumers to specify what header they want.

@voided
Copy link
Member
voided commented Oct 25, 2015

LGTM unless @Netshroud has any other ideas.

@Netshroud Netshroud merged commit d95bb5c into SteamRE:master Oct 26, 2015
@xPaw xPaw deleted the separate-protos branch October 28, 2015 17:05
@voided voided mentioned this pull request Dec 2, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@xPaw @voided @Netshroud
0