search

GraylinKim / sc2reader

A python library that extracts data from various Starcraft II resources to power tools and services for the SC2 community. Who doesn't want to hack on the games they play?
http://sc2reader.readthedocs.org
MIT License
413 stars 85 forks source link

Adding support for the 36442 base build #187

Open StoicLoofah opened 9 years ago

StoicLoofah commented 9 years ago

@GraylinKim I feel like an idiot: I had no idea how the different numbers in the s2protocol file corresponded to the types, and then I realized it was just a dictionary into the types. After that, it was easy to figure out.

For posterity (and anyone else who might want to contribute), I:

  1. took a diff between the last 2 s2protocol builds
  2. for new/different field, I went into sc2reader/readers.py and found where it fit in (probably just below the previous field)
  3. for that field, I took the number type from s2protocol, found another field with the same type, searched that field in readers.py, and copied that type for the new line and added the base_build check as well
  4. None of the fields were marked as optional, but if they were, I would have handled that
  5. for the event changes, I copied the last GameEventsReader and updated its parent. I tweaked it appropriately
  6. I registered the new GameEventsReader in sc2reader/resources.py

and it seemed like that worked.

Let me know if there's anything that I might have missed. I'm popping this into production for lotv.spawningtool.com right now, so I will let you know if I see any other errors.

GraylinKim commented 9 years ago

I don't have any newer replays myself but you figured it out pretty well. Fix the comment above and I'd be happy to merge.

StoicLoofah commented 9 years ago

Updated with the condition as you mentioned. If you want to play around with it yourself, here's a replay on this build http://lotv.spawningtool.com/903/download/

StoicLoofah commented 9 years ago

I have been using my own fork in production, and it has been working here. @GraylinKim poking if you want to merge this into your branch?