Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skipping frames on streaming replay panics. #51

Closed
Kered13 opened this issue Jan 26, 2023 · 2 comments
Closed

Skipping frames on streaming replay panics. #51

Kered13 opened this issue Jan 26, 2023 · 2 comments

Comments

@Kered13
Copy link

Kered13 commented Jan 26, 2023

This bug is similar to #19, however this bug effects perfectly valid replays that should parse. The issue is on the same line, de.rs:982. For streaming replays length is 0, which guarantees an underflow and panic. The solution is straightforward: If length is 0, then do not skip directly to GameEnd, instead keep processing events normally, but just don't parse the frame events.

I also noticed that there is no unit testing for streaming replays at all, this should be fixed. I have attached a streaming replay that can be used for unit testing. It is identical to game.slp, except the length field has been modified to 0.

streaming_game.zip

@NickCondron
Copy link
Contributor

#41 addresses this. It also adds a unit test for this case. It removes the panic, but it will return a Result::Error if you attempt to skip frames on a corrupt/in-progress replay. In case the raw length is 0, it might be better to skip frame events as they're read like you suggest. However, that would be a more involved change to the code and is a subtle change in semantics of the skip. Maybe your suggestion is better I'll have to think about it.

@hohav
Copy link
Owner

hohav commented Jan 3, 2024

Closed via #41.

@hohav hohav closed this as completed Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants