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

Revert App::run() behavior/Remove winit specific code from bevy_app #10389

Merged
merged 1 commit into from
Nov 16, 2023

Conversation

tim-blackbird
Copy link
Contributor

Objective

The way bevy_app works was changed unnecessarily in #9826 whose changes should have been specific to bevy_winit.
I'm somewhat disappointed that happened and we can see in #10195 that it made things more complicated.

Even worse, in #10385 it's clear that this breaks the clean abstraction over another engine someone built with Bevy!

Fixes #10385.

Solution

While this code is breaking relative to 0.12.0, it reverts the behavior of bevy_app back to how it was in 0.11.
Due to the nature of the breakage relative to 0.11 I hope this will be considered for 0.12.1.

@alice-i-cecile alice-i-cecile added this to the 0.12.1 milestone Nov 5, 2023
@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-Windowing Platform-agnostic interface layer to run your app in P-High This is particularly urgent, and deserves immediate attention A-App Bevy apps and plugins labels Nov 5, 2023
@alice-i-cecile alice-i-cecile requested a review from hymm November 5, 2023 14:44
@hymm
Copy link
Contributor

hymm commented Nov 5, 2023

I'm down for this. Can someone test if this still removes the black frames on iOS.

Copy link
Member

@cart cart left a comment

Choose a reason for hiding this comment

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

Sadly we haven't been able to recruit an iOS tester to check the black frame issue. I think these are the right changes for 0.12.1, even if we regress the black frame on iOS. However based on my understanding, this should still fix it.

@cart cart added this pull request to the merge queue Nov 16, 2023
Merged via the queue into bevyengine:main with commit bc9e159 Nov 16, 2023
26 checks passed
ickk added a commit to ickk/bevy that referenced this pull request Nov 17, 2023
ickk added a commit to ickk/bevy that referenced this pull request Nov 17, 2023
github-merge-queue bot pushed a commit that referenced this pull request Nov 18, 2023
Bevy introduced unintentional breaking behaviour along with the v0.12.0
release regarding the `App::set_runner` API. See: #10385, #10389 for
details. We weren't able to catch this before release because this API
is only used internally in one or two places (the very places which
motivated the break).

This commit adds a regression test to help guarantee some expected
behaviour for custom runners, namely that `app::update` won't be called
before the runner has a chance to initialise state.
github-merge-queue bot pushed a commit that referenced this pull request Nov 26, 2023
# Objective

- Window size, scale and position are not correct on the first execution
of the systems
- Fixes #10407,  fixes #10642

## Solution

- Don't run `update` before we get a chance to create the window in
winit
- Finish reverting #9826 after #10389
cart pushed a commit that referenced this pull request Nov 30, 2023
…app` (#10389)

# Objective
The way `bevy_app` works was changed unnecessarily in #9826 whose
changes should have been specific to `bevy_winit`.
I'm somewhat disappointed that happened and we can see in
#10195 that it made things more
complicated.

Even worse, in #10385 it's clear that this breaks the clean abstraction
over another engine someone built with Bevy!

Fixes #10385.

## Solution

- Move the changes made to `bevy_app` in #9826 to `bevy_winit`
- Revert the changes to `ScheduleRunnerPlugin` and the `run_once` runner
in #10195 as they're no longer necessary.

While this code is breaking relative to `0.12.0`, it reverts the
behavior of `bevy_app` back to how it was in `0.11`.
Due to the nature of the breakage relative to `0.11` I hope this will be
considered for `0.12.1`.
cart pushed a commit that referenced this pull request Nov 30, 2023
# Objective

- Window size, scale and position are not correct on the first execution
of the systems
- Fixes #10407,  fixes #10642

## Solution

- Don't run `update` before we get a chance to create the window in
winit
- Finish reverting #9826 after #10389
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
…app` (bevyengine#10389)

# Objective
The way `bevy_app` works was changed unnecessarily in bevyengine#9826 whose
changes should have been specific to `bevy_winit`.
I'm somewhat disappointed that happened and we can see in
bevyengine#10195 that it made things more
complicated.

Even worse, in bevyengine#10385 it's clear that this breaks the clean abstraction
over another engine someone built with Bevy!

Fixes bevyengine#10385.

## Solution

- Move the changes made to `bevy_app` in bevyengine#9826 to `bevy_winit`
- Revert the changes to `ScheduleRunnerPlugin` and the `run_once` runner
in bevyengine#10195 as they're no longer necessary.

While this code is breaking relative to `0.12.0`, it reverts the
behavior of `bevy_app` back to how it was in `0.11`.
Due to the nature of the breakage relative to `0.11` I hope this will be
considered for `0.12.1`.
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
…#10609)

Bevy introduced unintentional breaking behaviour along with the v0.12.0
release regarding the `App::set_runner` API. See: bevyengine#10385, bevyengine#10389 for
details. We weren't able to catch this before release because this API
is only used internally in one or two places (the very places which
motivated the break).

This commit adds a regression test to help guarantee some expected
behaviour for custom runners, namely that `app::update` won't be called
before the runner has a chance to initialise state.
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 A-Windowing Platform-agnostic interface layer to run your app in C-Bug An unexpected or incorrect behavior P-High This is particularly urgent, and deserves immediate attention
Projects
None yet
Development

Successfully merging this pull request may close these issues.

set_runner doesn't work like it did in 0.11 after updating to 0.12
4 participants