hohav / peppi

Rust parser for Slippi SSBM replay files
MIT License
37 stars 9 forks source link

Updated naming conventions #54

Closed Walnut356 closed 8 months ago

Walnut356 commented 1 year ago

Some terms are modified to disambiguate data type (airborne -> is_airborne, hitlag -> hitlag_remaining)

Some are changed to be more in line with what someone unfamiliar with the API would most likely try to type in when looking for a variable (damage -> percent)

I basically had 3 rules for consistency:

I also took the liberty of adding descriptive comments to many of the states in the action states enum, based off of the ssbm datasheet.

All tests pass, hard-coded test strings were updated by hand rather than re-generated-and-replaced to prove that there were no logic changes.

NickCondron commented 1 year ago

I made a PR #56 that will allow you to add doc comments to the constants within pseudo_enum/pseudo_bitmask

hohav commented 8 months ago

The enum-related parts of this PR would no longer belong in peppi but in ssbm-data. The field renames I mostly disagree with, except damage->percent: I still think the latter is a terrible name, but it's used widely enough in the community that I'm willing to accept it in the interests of pragmatism.

More generally, it would be better to open an issue to discuss a sweeping change like this before submitting a PR.