EmbarkStudios / discord-sdk

An open implementation of the Discord Game SDK in Rust
Apache License 2.0
150 stars 10 forks source link

Make the timestamp system in Activity closer to that of the Discord SDK, and more user friendly #10

Closed ewpratten closed 3 years ago

ewpratten commented 3 years ago

Checklist

Description of Changes

After working with this crate in a recent project, the largest headache I encountered was the lack of ability to set just a start timestamp on activity (to get the (xx:xx elapsed) message in my status).

This PR adds two new methods to ActivityBuilder and slightly changes IntoTimestamp and Timestamps to add this ability.

I have added ActivityBuilder::start_timestamp and ActivityBuilder::end_timestamp which both set only their respective timestamp values when called. I have also deprecated ActivityBuilder::timestamps (although this can be undone), and changed its code to just call start_timestamp and end_timestamp. (Important note, calling both methods together will set a time range, like timestamps does)

Both the start_timestamp and end_timestamp now take a generic T: IntoTimestamp instead of an impl IntoTimestamp since the Rust compiler seems to dislike the old way, throwing errors when the method is called.

I have added an IntoTimestamp impl for i64, so crate users can easily enter a unix timestamp as an integer (more useful for testing than anything).

Finally, to make setting only one timestamp at a time possible, I have added serde annotations to Timestamps to skip serialization of empty fields.

I'll be keeping an eye on this PR for any requested changes, or comments.

Related Issues

None

Jake-Shadle commented 3 years ago

I don't think there is any need to deprecate timestamps, there are situations where where having a single method can be more convenient compared to 2 separate builder methods. Everything else looks good though (well, other than the rustfmt issues)!

ewpratten commented 3 years ago

Alright. Both of those should be addressed now.