ascendedguard / sc2replay-csharp

C# Library for parsing Starcraft 2 Replay files
MIT License
56 stars 20 forks source link

Behavior of Replay.Players property changed #23

Closed TheNickestNick closed 12 years ago

TheNickestNick commented 12 years ago

Old Behavior

Replay.Players was a List that only contained players that actually participated in the game.

New Behavior

Replay.Players is a Player[16], which contains null entries for players that don't exist in the game. Players are inserted to the array using a 1-based index (instead of 0-based), which means that Players[0] is always null.

Anybody know why this was changed? This unexpectedly broke my app.

ghost commented 12 years ago

I changed it to that to make my events parser be able to refer to players more efficiently and to be able to parse events for players who didn't show up in the old List (i.e. observers). All that's required to bring it back to a list is to use the array constructor for Lists, and it's a small array, so it's not a huge change in my opinion.

I can patch it back if it's absolutely necessary.

On a side note, if you don't need the events parser, there's an optional parameter in Replay.Parse now to turn that off so you can save parse time.

ascendedguard commented 12 years ago

Originally my intention was ClientList would show all clients connected to the game, including observers. Players would simply be the players playing the game. Observers you could deduce by removing any players from ClientList

I think a better implementation would be to change ClientList to include all players in their current form (marking as observer, referree, etc. as correct) and change Players back to simply include the players that are in-game.

I'm guessing Players[0] being null is a reference to the neutral player in-game, but extracting it in this class seems irrelevant and should probably be removed so the indexing is back to 0-based.

Edit: I'm out of town until Saturday night, so unfortunately I won't be able to make any changes/pulls myself until then.

ascendedguard commented 12 years ago

ClientList[] is now the Player[16], with proper PlayerId indexing (index 0 is always a neutral player in SC2). Players[] is now a properly sized Player[] with only the active players.

References between Players[] and ClientList[] are shared when appropriate, but this should keep everything working with original intentions while making ClientList a more useful structure (Spectators are correctly marked as so).

I've pushed the changes and tested on my end, so let me know if you have any issues.