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

Several improvements #61

Closed Veritasimo closed 12 years ago

Veritasimo commented 12 years ago

I've fixed a tiny amount of stuff in resources to allow Maps to load correctly.

I've also added correct location data to CameraMovementEvent.

GraylinKim commented 12 years ago

Hey @Veritasimo, thanks for the patches.

I just got back from vacation so I might not be able to review this right away. Just wanted to give you a heads up so you'd know I wasn't ignoring this.

GraylinKim commented 12 years ago

Also, it seems like the only differences are the line endings. You might want to see if there is a setting in Github Windows that lets you commit with unix line endings instead of the native Windows ones.

On the command line the feature is known as auto_crlf; github might refer to their GUI options similarly.

Veritasimo commented 12 years ago

Thanks. I've applied that to my config. I generally use Mercurial so I'm a bit of a git newbie.

GraylinKim commented 12 years ago

Hey, sorry for the extremely long delay @Veritasimo. I've pulled in the first commit as ed15354f.

I am not sure If I want to pull in the zoom/rotate changes or not. How certain are you that they are correct? Did you try to read meaningful numbers out?

Bonus: Why was that useful to you? I am curious.

GraylinKim commented 12 years ago

I should also mention that I modified your patch so that self.file was not set.

This decision was made because we want the client to be able to free the resources after loading without us holding on to dead references. The filename is saved such that you could open the resource again if you wanted. There may be cases where the resource can't be reopened and you need access to it, but I think that can better be solved in the client code, not the library.

If you really think keeping a reference is a good idea I am open to appeals.

Veritasimo commented 12 years ago

I'm fairly sure the zoom/rotate changes are correct. They match the comparisons I did with SC2Gears at least. I included them for the sake of completeness. They don't serve any useful purpose that I can think of. The camera co-ordinates I'm currently using in an anti-cheat application with some success.

I have no objections to your resource changes.

GraylinKim commented 12 years ago

We're now using (in the v15 branch) sc2replay-csharp project's camera event parse code. Their approach is slightly different. I have't tried verifying the zoom/rotate numbers but everything else we pulled from them was very solid.

Take a look if you are interested, I am closing this for now.