Adds coherence to the upgrade names as a first step.

[EHadoux]

I started with giving coherence to all the upgrade names such that:
Upgrade{Race}{Type}Level{n}.
This impacts BasicCommandEvent.ability_name. This change reflects UpgradeCompleteEvent names for the upgrades.
However, some names have to be changed in the later, like ShieldWall or PunisherGrenades because the name in ability_lookup.csv (and thus in BasicCommandEvent.ability_name) is more suitable.
Where can I do that?
Maybe do not merge this pull-request until this and further tests have been done.

I also fixed a typo for the file to be displayed properly in Github (with the nice table and stuff like that).

[GraylinKim]

You'll notice that the change in names has caused some tests to fail, if it is necessary to change tests to keep them passing with these changes please do so. Because these sorts of changes are not backwards compatible it would be far better if they were done in one large, complete, update. I'll hold on merging anything to master until we have something that resembles a complete solution.

There is currently no mapping for UpgradeCompleteEvent names. You will need to create one, load it with the rest of the data, and add logic that fixes the names to the UpgradeCompleteEvent processing. Normally when I do such things I will leave the original name intact on the event and have a separate field with the corrected or normalized value.

[EHadoux]

You'll notice that the change in names has caused some tests to fail, if it is necessary to change tests to keep them passing with these changes please do so

Yep, the log on Travis tells that the problem come from the Coveralls installation. No sc2reader tests have failed.

Because these sorts of changes are not backwards compatible it would be far better if they were done in one large, complete, update. I'll hold on merging anything to master until we have something that resembles a complete solution.

I completely agree with that, hence my question you answer to after.

Normally when I do such things I will leave the original name intact on the event and have a separate field with the corrected or normalized value.

I'll do this that way so. Thanks :)

[EHadoux]

I've test it against all the replays from the last DreamHack.

[EHadoux]

I'm running it on all the WCS replays. Please wait until it is finished to merge.

[EHadoux]

This commit passes all the replays from the WCS.

[GraylinKim]

Can you talk about what sort of testing your "running it on all the WCS replays" entails? Is there some specific script you are running to make sure all the mappings make sense or are you just trying to weed out any unrelated bugs in the upgrade handling?

I have two remaining concerns re: your pull request.

1. I think it would be good to move the normalized lookup to the data package along with the other lookups. Even better would be to create an UpgradeClass in the style of the Ability class and add methods to the Build class for looking them up. This way we could include upgrade cost data in the project as well. If you aren't interested in going that far that's fine, just thought I'd mention it while we are here.
2. These normalized names have never been part of the documentation. If you had some ideas on how it might be best to include the names that sc2reader uses in the documentation I'd like to hear them. I think having a couple pages listing out what strings people should look for in game data would be much better than having them browse the source. Since we are making changes to this layer now it'd be nice if we could supply documentation as well. I think having a couple pages listing out what strings people should look for in game data would be much better than having people search through the data files in our source tree.

Otherwise, thanks for the effort @EHadoux. I'll take a look at your pull request in more detail in the next couple days.

[EHadoux]

Can you talk about what sort of testing your "running it on all the WCS replays" entails? Is there some specific script you are running to make sure all the mappings make sense or are you just trying to weed out any unrelated bugs in the upgrade handling?

You're right I forgot that. I have tested that every finishing upgrade matches an upgrade that have been launched earlier.

I think it would be good to move the normalized lookup to the data package along with the other lookups.

I've tried to do that but I didn't manage to find a way to get the dict from outside this file (I'm rather new at Python).

Even better would be to create an UpgradeClass in the style of the Ability class and add methods to the Build class for looking them up. This way we could include upgrade cost data in the project as well. If you aren't interested in going that far that's fine, just thought I'd mention it while we are here.

No problem, I'll try that but I cannot guarantee you a result. It will worth the effort but maybe in a second time.

These normalized names have never been part of the documentation. If you had some ideas on how it might be best to include the names that sc2reader uses in the documentation I'd like to hear them. I think having a couple pages listing out what strings people should look for in game data would be much better than having them browse the source. Since we are making changes to this layer now it'd be nice if we could supply documentation as well. I think having a couple pages listing out what strings people should look for in game data would be much better than having people search through the data files in our source tree

Ok, i'll add that.

[EHadoux]

I will definitely add an upgrade object to have the time until completion because you cannot really know when an update is started if it is queued (unless you know some way). However, i'll do this a bit after, in an other pull request.

[coveralls]

Coverage increased (+0.08%) when pulling 33b011f602b62c5c452c7a23c064b50b8ca8bcde on MakozFriends:hotfix/upgradenames into 1a6220b on GraylinKim:master.

[EHadoux]

Unless other comments, I have no other commit to do in this pull request.

[GraylinKim]

Hi, sorry, not ignoring you. I am currently moving to a new apartment and can't look at this right now. I'll review and merge as soon as possible, thanks for the work!

[EHadoux]

No worries, thank you :)

[EHadoux]

Hi, feel free to change some stuffs in the docs to comply with your standards :)

[coveralls]

Coverage increased (+0.08%) when pulling c333e6e59d5465a76c3b802dfe1b8c99bf983c50 on MakozFriends:hotfix/upgradenames into 1a6220b on GraylinKim:master.

[coveralls]

Coverage increased (+0.39%) when pulling a312c9a on MakozFriends:hotfix/upgradenames into c860d8e on GraylinKim:master.

[EHadoux]

I just rebase the branch with the last commits from @dsjoerg.
However, I'm not sure about the doc (the table), I cannot compile it ATM.