hohav / peppi

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

Derive PartialOrd/Ord/PartialEq for more types #6

Closed NickCondron closed 2 years ago

NickCondron commented 2 years ago

A couple times using this library I've expected a trait to be implemented for a type when it wasn't. For example the Port type in src/model/primitives.rs doesn't implement PartialOrd/Ord, so trying to compare the port of two players to determine which comes first in the Frame<N> struct is tedious.

I don't know when ParitalOrd is sufficient or when you want PartialOrd & Ord. Same goes with ParitalEq and Eq. Seems like some types have just PartialEq when they could also have Eq. I don't know if there's a rhyme or reason why things are the way they are.

See also: https://rust-lang.github.io/api-guidelines/interoperability.html#c-common-traits

I'd be willing to go through and add some of these to the derive statements, but it would help to hear your input.

hohav commented 2 years ago

Closed by 8f810210868dc595de2ee66b0c72b93e3f61d72f & 961d95beea86b7d2800ee33f1e0d5f60f186454e. FYI there are a few structs that still don't implement Eq because they contain floats, which are PartialEq but not Eq.