Skip to content

Conversation

@Talv
Copy link
Contributor

@Talv Talv commented Nov 27, 2019

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep this list in sorted order because it make it easier to spot duplicates.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A double ternary if is not really Pythonic. What about:

if replay.base_build >= 77379:
    read_bits = 16
elif replay.base_build >= 70154:
    read_bits = 13
else:
    read_bits = 9
data.read_bits(read_bits)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might not be Pythonic, but it's consistent with rest of the code. So I'd rather keep it as is.

Unfortunately decoding functions are already hardly readable. And if there's something to be done with it, it should be tackled in separate PR IMO.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same.

@StoicLoofah StoicLoofah merged commit 7a1afe6 into ggtracker:upstream Nov 28, 2019
@StoicLoofah
Copy link
Collaborator

Thanks for doing this! Now I need to go add Mengsk support elsewhere =P

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

Successfully merging this pull request may close these issues.

3 participants