search

project-slippi / slippi-js

Parse slp files and compute stats
GNU Lesser General Public License v3.0
148 stars 79 forks source link

Hard crash when outputting Slp files with only port 2 and 3 in use #96

Closed dragonbane0 closed 2 years ago

dragonbane0 commented 2 years ago

As soon as the SlpFile class is used to dump slp files with port 1 disabled and port 2 and 3 (for example) in use you experience a crash as soon as the game starts.

The crash point occurs here: https://github.com/project-slippi/slippi-js/blob/beb20613a33a387481c93623a5ce5c4b9e470bc3/src/utils/slpFile.ts#L149

The this.metadata.players array only contains playerIndex 0 and 1 when it should be 1 and 2 in this case.

This seems to be the offending line: https://github.com/project-slippi/slippi-js/blob/beb20613a33a387481c93623a5ce5c4b9e470bc3/src/utils/slpFile.ts#L127

Specifically it should be this.metadata.players[player.playerIndex] instead of this.metadata.players[i] to setup the array properly, so the later indexing during POST_FRAME_UPDATE doesn't fail.

My limited slippi-js knowledge doesn't tell me if any further changes might be needed to ensure data integrity.

NikhilNarayana commented 2 years ago

Nice catch, your suggested change (this.metadata.players[player.playerIndex]) is correct and all that needs to be done. Feel free to make a PR.

I am surprised this is the first error report we've gotten about this though cause tons of events have been using mirroring and not had issues.