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

Improve ergonomics of platform_support's Instant #17577

Merged
merged 1 commit into from
Feb 2, 2025

Conversation

Mathspy
Copy link
Contributor

@Mathspy Mathspy commented Jan 28, 2025

Objective

  • Make working with bevy_time more ergonomic in no_std environments.

Currently bevy_time expects the getter in environments where time can't be obtained automatically via the instruction set or the standard library to be of type *mut fn() -> Duration. fn() is already a function pointer, so *mut fn() is a pointer to a function pointer. This is harder to use and error prone since creating a pointer out of something like &mut fn() -> Duration when the lifetime of the reference isn't static will lead to an undefined behavior once the reference is freed

Solution

  • Accept a fn() -> Duration instead

Testing

  • I made a whole game on the Playdate that relies on bevy_time heavily, see: bevydate_time for usage of the Instant's getter.

Showcase

Click to view showcase
test2.mp4

Migration Guide

This is a breaking change but it's not for people coming from Bevy v0.15

Small thank you note

Thanks to my friend https://github.com/repnop for helping me understand how to deal with function pointers in unsafe environments

Thanks to my friend https://github.com/repnop for helping me understand
how to deal with function pointers in `unsafe` environments

Co-authored-by: Wesley Norris <[email protected]>
Copy link
Contributor

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

@BenjaminBrienen BenjaminBrienen added D-Trivial Nice and easy! A great choice to get started with Bevy C-Usability A targeted quality-of-life change that makes Bevy easier to use A-Time Involves time keeping and reporting D-Unsafe Touches with unsafe code in some way S-Needs-Review Needs reviewer attention (from anyone!) to move forward O-Embedded Weird hardware and no_std platforms labels Jan 28, 2025
Copy link
Contributor

@bushrat011899 bushrat011899 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that is better, thanks for submitting this! And thanks for confirming that the Playdate works too!

@Mathspy
Copy link
Contributor Author

Mathspy commented Jan 28, 2025

Yeah that is better, thanks for submitting this! And thanks for confirming that the Playdate works too!

Thank YOU for all the hard work championing Bevy's no_std support in such a short time ❤️ I was really excited when I saw #15460 a few months ago when I ordered my Playdate and couldn't wait to run Bevy on it

@BenjaminBrienen BenjaminBrienen added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jan 28, 2025
@BD103
Copy link
Member

BD103 commented Jan 29, 2025

The changes look good, and the demo video was super cool! (Now I want a Playdate ^^)

@mockersf mockersf added this pull request to the merge queue Feb 2, 2025
Merged via the queue into bevyengine:main with commit 469b218 Feb 2, 2025
36 checks passed
@Mathspy Mathspy deleted the no-std-time-ergonomics branch February 3, 2025 01:10
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 D-Trivial Nice and easy! A great choice to get started with Bevy D-Unsafe Touches with unsafe code in some way O-Embedded Weird hardware and no_std platforms 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.

5 participants