AttributeError when trying to load replay

[stefanpantic]

I get this error whenever I try to load a Replay. I tried reinstalling and installing directly from GitHub but the error persists. Any ideas on what could be causing this?

  File "<stdin>", line 1, in <module>
  File "/home/stefan/Documents/ML/env/lib/python3.6/site-packages/sc2reader/factories/sc2factory.py", line 85, in load_replay
    return self.load(Replay, source, options, **new_options)
  File "/home/stefan/Documents/ML/env/lib/python3.6/site-packages/sc2reader/factories/sc2factory.py", line 137, in load
    return self._load(cls, resource, filename=filename, options=options)
  File "/home/stefan/Documents/ML/env/lib/python3.6/site-packages/sc2reader/factories/sc2factory.py", line 146, in _load
    obj = cls(resource, filename=filename, factory=self, **options)
  File "/home/stefan/Documents/ML/env/lib/python3.6/site-packages/sc2reader/resources.py", line 312, in __init__
    engine.run(self)
  File "/home/stefan/Documents/ML/env/lib/python3.6/site-packages/sc2reader/engine/engine.py", line 177, in run
    for new_event in (event_handler(event, replay) or []):
  File "/home/stefan/Documents/ML/env/lib/python3.6/site-packages/sc2reader/engine/plugins/context.py", line 57, in handleTargetUnitCommandEvent
    self.last_target_ability_event[event.player.pid] = event
AttributeError: 'NoneType' object has no attribute 'pid'

[dsjoerg]

Is this a standard ladder replay? If not, that could be the problem. I'm sure there are types of replays that sc2reader needs to be tweaked to be able to handle.

[StoicLoofah]

Yeah, I have run into issues like this before, though it does tend to be replay-specific. Can you provide a download link for an example of a problematic replay?

[stefanpantic]

I've tried several from here and they all gave the same error.
http://blzdistsc2-a.akamaihd.net/ReplayPacks/4.1.2.60604-20171220_014559-1.zip

I forgot to mention my client version (4.1.2), and Python version (3.6.5).

[StoicLoofah]

@stefanpantic I tried opening the file linked, and it's requesting a password to open the zip file. Any help on that?

[stefanpantic]

My bad i forgot to include that. It's"iagreetotheeula".

[StoicLoofah]

Okay, so I was able to diagnose where we're not getting the data we need. However, I have no idea how to fix it.

On the replay that I checked, the event.player == None, but event.pid == 1. When I checked, both replay.player and replay.players were empty.

Those are supposed to be populated here

sc2reader/sc2reader/resources.py

Lines 384 to 386 in cc364ff

	def load_players(self):
	# If we don't at least have details and attributes_events we can go no further
	if 'replay.details' not in self.raw_data:

but replay.details does not appear in self.raw_data.

that is supposed to be loaded at

sc2reader/sc2reader/resources.py

Line 274 in cc364ff

self._read_data(data_file, self._get_reader(data_file))

which ends up invoking this

sc2reader/sc2reader/utils.py

Lines 138 to 139 in cc364ff

	try:
	file_data = archive.read_file(data_file, force_decompress=True)

and there, file_data == None. It doesn't even hit the exception, though when I checked recovery_attempt() == None.

Note that there is real data here in replays that parse correctly (though it did hit the recovery_attempt case on the replay I tried), so I think this is the gap.

This is far beyond the parts of sc2reader that I have ever had to deal with, so I really have no idea what's going on here. Calling on @GraylinKim to see if he has any memory of what's going on here. Otherwise, I'm not sure where to go from here =(

[cclauss]

except Exception as e: is a bad practice so, would it be possible to replace Exception with ImportError (not likely to happen here) and then do a few trial runs and catch each exception that actually occurs in its own except block?

try:
    file_data = archive.read_file(data_file, force_decompress=True)
except AError as e:
    file_data = recovery_attempt()
    if file_data is None:
        raise
except BError as e:
    file_data = recovery_attempt()
    if file_data is None:
        raise
except CError as e:
    file_data = recovery_attempt()
    if file_data is None:
        raise

This way we can understand what is actually going wrong before we propose that the solution is file_data = recovery_attempt(). Once you understand the problem and have an effective solution, you can condense it back down to:

try:
    file_data = archive.read_file(data_file, force_decompress=True)
except (AError, BError, CError) as e:
    file_data = recovery_attempt()
    if file_data is None:
        raise

But we should avoid sweeping helpful information under the rug with except Exception as e:.

[StoicLoofah]

@cclauss definitely good practice if we can narrow in on the exception types, and would definitely encourage anyone to take a shot at it!

For this specific issue, though, i'll clarify that we aren't even hitting the exception: the function is actually running without an error. The problem is that it's returning None rather than any data. As to how it's coming to that, I have no idea =(

[Gusgus01]

@StoicLoofah I dug into it a bit further :)

If you load those replays up in Starcraft 2, the players don't have names. I'm not sure if at some point they had names and even the Starcraft client can't parse them now or they never had names.

Also peeking into the MPQArchive using mpyq, I see that the usual replay.details file is missing which would explain why replay.details does not appear in self.raw_data. However, there is a replay.details.backup file in the archive, but it would appear to be missing the player name. Also, the current BitPackedDecoder available in decoders.py manages to mostly parse the data, but not fully.

Should the package support using replay.details.backup? If so I can dig into this further and try to figure out the parsing more, but it appears there won't be a player name either way and it looks to be missing other data as well. A better course of action if we want to support these replay files might be to make it fall back to using the pid instead or some default string for each player.

[Necdilzor]

but it appears there won't be a player name either way and it looks to be missing other data as well. A better course of action if we want to support these replay files might be to make it fall back to using the pid instead or some default string for each player.

These replays were anonymized by Blizzard, removing every player's specific data. When you load them using the sc2client-api you can't see their names either and it defaults to "1" and "2" as player names.

[cclauss]

Can we please consider something along the lines of #62?

[StoicLoofah]

@Gusgus01 thanks for looking more deeply into this! My short answer is that I would love to have a good solution for this but don't have a strong opinion on how to do it.

In the general case, it sounds like we won't stumble across replays with this problem in the wild: they just happen to be how Blizzard treated this particular data set, but since they are official, that makes them pretty relevant.

As I mentioned above, I really don't understand how the archive reading logic works: most of my tweaks to sc2reader start assuming that it was unpacked properly, then working to massage that into the python class structure. I would be happy to look at any PRs that manage to avoid crashes. From there, we can talk more about how we want to fill in details e.g. using pids or placeholder data.