Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type 'core::time::Duration' does not support ReflectValue serialization #3277

Closed
shelbyd opened this issue Dec 7, 2021 · 5 comments
Closed
Labels
A-Reflection Runtime information about types C-Usability A targeted quality-of-life change that makes Bevy easier to use

Comments

@shelbyd
Copy link

shelbyd commented Dec 7, 2021

What problem does this solve or what need does it fill?

I'm building a general purpose plugin for doing rollback networking for bevy and a game using that. To support clients reconnecting after disconnect, I need to serialize relevant state and transmit it to reconnecting clients. It's common for Timers to be relevant for gameplay logic, and those require serializing Durations.

What solution would you like?

Implement ReflectValue serialization for Duration and other core types.

What alternative(s) have you considered?

To unblock myself, I can reimplement Timer and Duration with a custom MyDuration type that derives Reflect. Doing that is not ideal for a myriad of reasons, but probably necessary to continue progress on my project.

@shelbyd shelbyd added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Dec 7, 2021
@alice-i-cecile alice-i-cecile added A-Core A-Reflection Runtime information about types C-Usability A targeted quality-of-life change that makes Bevy easier to use and removed C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Dec 7, 2021
@alice-i-cecile
Copy link
Member

This is a very reasonable requirement. If you're comfortable with it, this should be a fairly straightforward PR.

@shelbyd
Copy link
Author

shelbyd commented Dec 7, 2021

If you can point me to the relevant file(s), I'm happy to do it myself.

@alice-i-cecile
Copy link
Member

Usually, we'd just use a #[derive(Reflect)] macro invocation (as seen in the examples), but that won't work here, as we need to implement the trait for a foreign type.

Similarly, we can't put this in bevy_core with the other time logic, since we don't own the Reflect trait there.

I would probably start a new file (time.rs) in this folder and implement the trait there for Duration and Instant.

The other examples in that folder should provide a nice guide, but feel free to reach out here or on your PR if you run into difficulties.

@cart
Copy link
Member

cart commented Dec 8, 2021

Duration is an std type, so it should probably go in the existing std module in the impls folder, but other than that I agree with @alice-i-cecile's recommendations.

@jcornaz
Copy link
Contributor

jcornaz commented Dec 13, 2021

I opened a PR that solves this issue. @shelbyd, I hope you don't mind that I didn't let you solve it yourself. I just stumbled into this problem too. And since the problem is pretty annoying, yet easy to fix, I just couldn't help but open a PR.

@bors bors bot closed this as completed in d07c8a8 Dec 29, 2021
mockersf pushed a commit to mockersf/bevy that referenced this issue Jan 1, 2022
# Objective

Resolves bevyengine#3277 

Currenty if we try to serialize a scene that contains a `Duration` (which is very common, since `Timer` contains one), we get an error saying:

> Type 'core::time::Duration' does not support ReflectValue serialization


## Solution

Let `Duration` implement `SerializeValue`.



Co-authored-by: Jonathan Cornaz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Reflection Runtime information about types C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants