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
411 stars 85 forks source link

Unit Composition plugin #164

Open StoicLoofah opened 10 years ago

StoicLoofah commented 10 years ago

Hopefully it's fairly straightforward about what the approach. I have tested it some (and actually looked at the replay to verify), but I'm thinking there are probably still edge cases out there to consider.

Graylin, I'm thinking that you probably have a better sense for what these edge cases are regarding unit types and such.

Hopefully this plugin is generally useful for others. I know other sites are already extracting compositions in their own way, but it would be kind of nice to have a standard method for doing it.

StoicLoofah commented 10 years ago

One other approach I tried was to actually go event by event. It mostly worked but ended up messy, and I switched to this approach at your suggestion. If this approach seems bad, I can always fall back to that

StoicLoofah commented 10 years ago

Now that I think about it, it might also be arguable that the out should key on unit types, not strings. I ended up doing it this way because this is ultimately what I want, but send it back if you want that changed and think users should rely on their own packages (like spawningtool) to get it the rest of the way.

coveralls commented 10 years ago

Coverage Status

Coverage decreased (-0.27%) when pulling e24a0420df430f9f8b847a92fa1ff21d4b11cf47 on StoicLoofah:composition-plugin-history into 52e5be94a0c0177b6cd1549f28358b0d0580bae7 on GraylinKim:master.

GraylinKim commented 10 years ago

You broke the build! Likely it is because the code isn't Python3 compatible; all new contributions need compatibility. I advise putting the following at the top of every file so that you can catch many of these issues while running python2.7:

from __future__ import absolute_import, print_function, unicode_literals, division

You also can't use iteritems() anymore. It is just items() now. There may be other issues but those are the ones I can see from the diff.

As for the plugin itself, I'll have think on it a bit more. My current thoughts though:

I'm trying to work on the upcoming patch so it may be a while before I really get around to this; fyi.

coveralls commented 10 years ago

Coverage Status

Coverage decreased (-0.36%) when pulling 85eb7c2e5324f616e1b0d289d33ce0b576bb4859 on StoicLoofah:composition-plugin-history into 52e5be94a0c0177b6cd1549f28358b0d0580bae7 on GraylinKim:master.

StoicLoofah commented 10 years ago

Thanks for the heads up on python 3. I tried to tackle all of the ones I could easily from my python2.7 environment. In addition to the things you mentioned, I also needed to use // for some int division.

With regards to polling, the other approaches I considered were to either update a new composition at every change or to add a function that would spit out the composition at a given time. I didn't get with the former because that would take a lot of space, and I didn't do the latter because it didn't fit well with what I'm hoping to do with the data and, if used, would be much less efficient. Even so, I'm open to ideas or other ways of getting it.

If you think a white list would be better, I'm down for that, too. I actually have mappings and filters for getting unit types into readable types at each level of my application, so I really have no idea what the best approach is.

No rush on reviewing: I have procrastinated on writing this forever anyways. Let me know if you have thoughts on other things you would like to see me fix on this pull request.