Skip to content

Relaxed runner type from Fn to FnOnce#8961

Merged
alice-i-cecile merged 1 commit intobevyengine:mainfrom
B-head:runner-type-relaxation
Jun 29, 2023
Merged

Relaxed runner type from Fn to FnOnce#8961
alice-i-cecile merged 1 commit intobevyengine:mainfrom
B-head:runner-type-relaxation

Conversation

@B-head
Copy link
Contributor

@B-head B-head commented Jun 26, 2023

Objective

Relax unnecessary type restrictions on App.runner function.

Solution

Changed the type of App.runner from Fn(App) to FnOnce(App).

@github-actions
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 ✨

@alice-i-cecile alice-i-cecile added C-Usability A targeted quality-of-life change that makes Bevy easier to use A-App Bevy apps and plugins labels Jun 27, 2023
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

This makes sense, although I would expect that almost all use cases meet the tighter bounds.

What motivated this?

@alice-i-cecile alice-i-cecile 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 Jun 27, 2023
@alice-i-cecile
Copy link
Member

Aha, the linked PR provides a good motivation :D

@alice-i-cecile alice-i-cecile requested a review from hymm June 28, 2023 19:22
@alice-i-cecile
Copy link
Member

@nakedible: does this PR look right to you as well?

Copy link
Contributor

@nakedible nakedible left a comment

Choose a reason for hiding this comment

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

This is correct and adheres exactly to documentation of set_runner which says:

The runner function run_fn is called only once by [App::run].

Also, the code that actually calls this:

        let runner = std::mem::replace(&mut app.runner, Box::new(run_once));
        (runner)(app);

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jun 29, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 29, 2023
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jun 29, 2023
Merged via the queue into bevyengine:main with commit 418d327 Jun 29, 2023
@B-head B-head deleted the runner-type-relaxation branch June 30, 2023 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-App Bevy apps and plugins 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.

4 participants