pid issues in pre-HotS branch

[dsjoerg]

ggtracker.com/matches/1471849 the APM for Aicy is 9!

replay = sc2reader.load_replay("http://ggtracker.com/matches/1471849/replay")

print replay.release_string 1.5.4.24540

print replay.players[0] Player 1 - Acey (Protoss) print replay.players[1] Player 4 - Aicy (Zerg)

print replay.player[1] Player 1 - Acey (Protoss) print replay.player[4] Player 4 - Aicy (Zerg)

[ (person.pid, person.is_observer, len(person.events)) for person in replay.people ] [(1, False, 8110), (4, False, 5675), (0, True, 6601), (2, True, 865), (3, True, 1803)]

print replay.events[47], replay.events[47].pid 00.01 Acey Ability (0x1720) - MorphDrone 1 print replay.events[69], replay.events[69].pid 00.03 TiDragOnflY Ability (0x14e0) - TrainProbe 0

The event pids think that the players are pids 0 and 1. However the player data structure thinks the player pids are 1 and 4.

[dsjoerg]

OK, I added a breaking test to ggtracker / sc2reader that will pass when the problem is fixed

[dsjoerg]

For what it's worth, using HEAD of GraylinKim/sc2reader master, this replay is handled properly:

>>> import sc2reader
>>> replay = sc2reader.load_replay("http://ggtracker.com/matches/1471849/replay")
>>> player_pids = set( [ player.pid for player in replay.players ] )
>>> efilter = lambda e: e.name == "AbilityEvent"
>>> abilityevents = filter(efilter, replay.events)
>>> ability_pids = set( [ event.pid for event in abilityevents ] )
>>> player_pids
set([1, 2])
>>> ability_pids
set([1, 2])

[dsjoerg]

Also interesting, the player pids are different according to GraylinKim/sc2reader master: 1,2 vs 0,1 for the newer version

[dsjoerg]

Having looked a little closer, it seems like the old load_players() algorithm that's at GraylinKim/sc2reader master HEAD uses a different technique than the new load_players() algorithm in GraylinKim/sc2reader hots HEAD. And at least for this replay file, the old algorithm works and the new one does not.

Any reason not to revert to the old one, at least for WoL replays?

[GraylinKim]

Thanks for all the information @dsjoerg. The problem isn't with the new load_players algorithm though. If that was broken then you would see the wrong races assigned to the wrong players.

Instead, the problem seems to be this:

• in WoL, event.pid is the player number (1,2,3,4)
• in HotS, event.pid is the index in the player array (which excludes observers and starts from 0)

I think this can be cleanly reconciled but I'll have to take a closer look. The reason I am trying to modify existing code to accommodate both instead of adding on code specific to HotS is that there needs to be consistency in accessing data between HotS and WoL. Having one player indexing method in HotS and one on WoL seems like it is exposing unnecessary complexity.

[dsjoerg]

OK, these issues are then too complex for me to really get involved, beyond finding bugs. Do remember that in the newer version of the code, you added the following in readers.py, so the event pids you're seeing are not the raw pids from the file:

POFFSET=-1
if pid != 16: pid+=self.POFFSET

[dsjoerg]

Ah never mind, I see what you're doing there -- you're adjusting pids for pre-HotS replays to be zero-based as well.

[GraylinKim]

Weird that commits from forks now show up here.

After some Not-Totally-Comprehensive testing it seems that it was always the player index in WoL as well. WoL just happened to have player numbers ordered according to their indexes so everything lined up for us. The pushed patch should work correctly in both expansions. I'm sure you'll notify me if it doesn't.

[GraylinKim]

Fixed in HotS branch now.