barrett777 / Heroes.ReplayParser

A C# library for parsing Heroes of the Storm replay files (.StormReplay)
MIT License
223 stars 95 forks source link

ShouldParseDetailedBattleLobby flag causes some replay parses to throw exception #101

Closed Zemill closed 4 years ago

Zemill commented 4 years ago

Replay to Test with

There is an issue with the parser. Default/Minimal parsing returns a result for the linked replay, but when I run Medium or Full, it throws an exception.

I have narrowed it down to ShouldParseDetailedBattleLobby flag causing the exception.

barrett777 commented 4 years ago

This should be fixed in version 1.2.8, which will be on NuGet shortly

Thanks @koliva8245 !

Zemill commented 4 years ago

I found another old replay that fails through here. Fails on the ExtendedBattleTagParsingOld(replay, bitReader); method.

Build 44124

replay-24309914.zip

I actually have a long list of replays that have been failing today (maybe someone started uploading a bunch of old replays). I spot checked a few of them and they are build 44124. I am not familiar with the decisions for the different if logical statements based on builds, so I will leave that to you

Maybe we just increase that value on the if statement for GetBattleTags(replay, bitReader);

barrett777 commented 4 years ago

The battlelobby file is the only undocumented part of the replay file, so it is hardest to parse successfully. You could try creating your own parse options with everything enabled except for battlelobby parsing, and see if the parsed replay object has all the data you need

barrett777 commented 4 years ago

@koliva8245 Could you look at the previous comment and zip file for battlelobby parsing error?

Zemill commented 4 years ago

@koliva8245 So I found some new replays under build 75132 that fail. I tested 5. I am not sure its consistent across all of them at that build number. I have other replays in my DB for the same game version with valid detailed scores.

I am wondering if we shouldn't come at this a different way. Instead of trying to find all builds or situations where this occurs, just put in some logic to detect a failed DetailedBattleLobby parse, and fall back to the default. This way you still get a valid end result, even if you don't get all of the data.

I am probably going to create a secondary loop for this condition on my side, but might be something to think about for the main parser. replays.zip

barrett777 commented 4 years ago

That's a good idea, we already do this for game events and tracker events: https://github.com/barrett777/Heroes.ReplayParser/blob/master/Heroes.ReplayParser/DataParser.cs#L187-L195