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

[Merged by Bors] - Add default implementation of Serialize and Deserialize to Timer and Stopwatch #6248

Closed
wants to merge 6 commits into from

Conversation

SleepySwords
Copy link
Contributor

@SleepySwords SleepySwords commented Oct 13, 2022

Objective

Fixes #6244

Solution

Uses derive to implement Serialize and Deserialize for Timer and Stopwatch

Things to consider

  • Should fields such as finished and times_finished_this_tick in Timer be serialized?
  • Does Countdown and PrintOnCompletionTimer need to be serialized and deserialized?

Changelog

Added Serialize and Deserialize implementations to Timer and Stopwatch, Countdown.

@alice-i-cecile alice-i-cecile added C-Usability A targeted quality-of-life change that makes Bevy easier to use A-Time Involves time keeping and reporting labels Oct 13, 2022
@alice-i-cecile
Copy link
Member

We should move serde behind a feature flag as it's a heavy dependency :) See #6023 by @targrub for an example of how to do so.

@alice-i-cecile
Copy link
Member

Should fields such as finished and times_finished_this_tick in Timer be serialized?

Yes, this is important state that will need to be e.g. transferred across the network.

Does Countdown and PrintOnCompletionTimer need to be serialized and deserialized?

No, this isn't used in that example, so should not be done to avoid distracting readers.

@alice-i-cecile
Copy link
Member

Ping @bzm3r for a review.

@mockersf mockersf added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Oct 13, 2022
@SleepySwords
Copy link
Contributor Author

It's probably worth mentioning that when #6247 is merged, implementations for Serialize and Deserialize would need to be added to TimerMode.

@alice-i-cecile
Copy link
Member

#6247 is getting merged, so I'm holding off on this for a sec. @SleepySwords can you add that impl to this PR when you get a second? Once that's done I'll merge this PR ASAP.

@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Oct 17, 2022
…Stopwatch (#6248)

# Objective

Fixes #6244

## Solution

Uses derive to implement `Serialize` and `Deserialize` for `Timer` and `Stopwatch`

### Things to consider
- Should fields such as `finished` and `times_finished_this_tick` in `Timer` be serialized?
- Does `Countdown` and `PrintOnCompletionTimer` need to be serialized and deserialized?

## Changelog

Added `Serialize` and `Deserialize` implementations to `Timer` and `Stopwatch`, `Countdown`.
@bors
Copy link
Contributor

bors bot commented Oct 17, 2022

@bors bors bot changed the title Add default implementation of Serialize and Deserialize to Timer and Stopwatch [Merged by Bors] - Add default implementation of Serialize and Deserialize to Timer and Stopwatch Oct 17, 2022
@bors bors bot closed this Oct 17, 2022
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 19, 2022
…Stopwatch (bevyengine#6248)

# Objective

Fixes bevyengine#6244

## Solution

Uses derive to implement `Serialize` and `Deserialize` for `Timer` and `Stopwatch`

### Things to consider
- Should fields such as `finished` and `times_finished_this_tick` in `Timer` be serialized?
- Does `Countdown` and `PrintOnCompletionTimer` need to be serialized and deserialized?

## Changelog

Added `Serialize` and `Deserialize` implementations to `Timer` and `Stopwatch`, `Countdown`.
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
…Stopwatch (bevyengine#6248)

# Objective

Fixes bevyengine#6244

## Solution

Uses derive to implement `Serialize` and `Deserialize` for `Timer` and `Stopwatch`

### Things to consider
- Should fields such as `finished` and `times_finished_this_tick` in `Timer` be serialized?
- Does `Countdown` and `PrintOnCompletionTimer` need to be serialized and deserialized?

## Changelog

Added `Serialize` and `Deserialize` implementations to `Timer` and `Stopwatch`, `Countdown`.
Pietrek14 pushed a commit to Pietrek14/bevy that referenced this pull request Dec 17, 2022
…Stopwatch (bevyengine#6248)

# Objective

Fixes bevyengine#6244

## Solution

Uses derive to implement `Serialize` and `Deserialize` for `Timer` and `Stopwatch`

### Things to consider
- Should fields such as `finished` and `times_finished_this_tick` in `Timer` be serialized?
- Does `Countdown` and `PrintOnCompletionTimer` need to be serialized and deserialized?

## Changelog

Added `Serialize` and `Deserialize` implementations to `Timer` and `Stopwatch`, `Countdown`.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
…Stopwatch (bevyengine#6248)

# Objective

Fixes bevyengine#6244

## Solution

Uses derive to implement `Serialize` and `Deserialize` for `Timer` and `Stopwatch`

### Things to consider
- Should fields such as `finished` and `times_finished_this_tick` in `Timer` be serialized?
- Does `Countdown` and `PrintOnCompletionTimer` need to be serialized and deserialized?

## Changelog

Added `Serialize` and `Deserialize` implementations to `Timer` and `Stopwatch`, `Countdown`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Time Involves time keeping and reporting C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Serialize/Deserialize for Timer and Stopwatch
4 participants