bevy_core: Derive useful traits on FrameCount

[snendev]

Objective

I am emboldened by my last small PR and am here with another.

• Describe the objective or issue this PR addresses.

It would be nice if FrameCount could be used by downstream plugins that want to use frame data. The example that I have in mind is leafwing_input_playback which has a duplicate implementation of FrameCount used in several structs which rely on those derives (or otherwise the higher-level structs would have to implement these traits manually). That crate, using FrameCount, tracks input frames and timestamps and enables various playback modes.

I am aware that bevy org refrains from deriving lots of unnecessary stuff on bevy types to avoid compile time creep. It is worth mentioning the (equally reasonable) alternative that downstream crates should implement some FrameCount themselves if they want special behavior from it.

Solution

• Describe the solution used to achieve the objective above.

I added derives for PartialEq, Eq, PartialOrd, Ord and implementations for serde::{Deserialize, Serialize} to FrameCount.

Testing

Manually confirmed that the serde implementation works, but that's all. Let me know if I should do more here.

[snendev]

Didn't quite do my due diligence here, my bad. bevy_core doesn't use serde's derive feature and also has a specific module for serde implementations, so I added a manual implementation there (that I hope is done correctly, I'm not super familiar).

Going to quickly edit the PR body to reflect these changes, double check that the serde implementation works correctly, and edit the PR body with that info as well

[bushrat011899]

I agree with the proposed changes, but I just have a couple small concerns around testing and the exact implementation. Thanks for contributing!

[alice-i-cecile]

I'd like to see a comment in the code base about why you chose to use a manual serde impl (a decision I agree with!) but once that's done this LGTM :)

[snendev]

I'd like to see a comment in the code base about why you chose to use a manual serde impl

I can't really provide any reasoning -- I was just following the existing pattern in the crate. The traits were implemented manually for Name so serde's derive feature isn't included at this point... changing a dependency seemed like a "bigger" change than this PR was intended to be. I'm happy to add a comment if that's helpful, maybe near the top of the file. Is there a good explanation I should use?

[alice-i-cecile]

There's my understanding of the motivation here :) I think on the balance I probably prefer to just swap to derives, but I don't mind it. Name needs a custom impl because it's effectively just storing a string but has perf-optimized internals.

[snendev]

got it, thanks a bunch alice! makes sense to me

[bushrat011899]

Lovely that assert_tokens tests serialisation and deserialisation at the same time.

[bushrat011899]

Good call to test this as well!