Don't update app if we're already exiting

[Brezak]

Objective

As was notice on discord. Running an app currently output ERROR bevy_winit: Failed to send a app exit notification! This is a bug. Reason: sending on a full channel. This happens because after triggering a event loop exit we still receive queued events. This triggers the event loop exit code again causing the warning. If one of the events we receive is AboutToWait we also update the app which goes against the promise made in the AppExit documentation.

Solution

• Add a exit_happened flag that gets set when we trigger a app exit.
• After a app exit has been triggered. Only handle the LoopExiting event.
• While handling the LoopExiting event send the AppExit that triggered the exit to the winit_runner.

Testing

• Run the scene 3d example.

cargo run --example 3d_scene

• Close the window.
• You should only see one No windows are open, exiting message and no errors.

If somebody could test this code on wasm I'd really appreciate it. Every time I try to do it I get a GPU lacks support: TextureFormat::R16Float does not support TextureUsages::STORAGE_BINDING. error, so I can't test it myself.

[JMS55]

If somebody could test this code on wasm I'd really appreciate it. Every time I try to do it I get a GPU lacks support: TextureFormat::R16Float does not support TextureUsages::STORAGE_BINDING. error, so I can't test it myself.

It's a warning that ScreenSpaceAmbientOcclusionPlugin will not work on wasm. It won't affect anything unless you're using SSAO, which if off by default.

[Brezak]

If somebody could test this code on wasm I'd really appreciate it. Every time I try to do it I get a GPU lacks support: TextureFormat::R16Float does not support TextureUsages::STORAGE_BINDING. error, so I can't test it myself.

It's a warning that ScreenSpaceAmbientOcclusionPlugin will not work on wasm. It won't affect anything unless you're using SSAO, which if off by default.

It dies after emitting that. I just assumed the warning was connected. I'll still need somebody else to tests it. Both to check if the error is only on my side and because I just can't test test the code if it dies.

[kristoff3r]

wasm is kinda broken on main until #13210 lands. When I rebase this on top of that PR it seems to work on wasm though.

[BD103]

Looks good! I have a few small suggestions, but they're non-blocking. The one thing I was not able to test was closing a window to exit the app. For some reason it is freezing on my laptop. (It's on main too, though, not just you.)

Edit: It's freezing because of #13208, so don't worry about it. If someone else to test closing a window to exit an app, that would be great! :)

[tychedelia]

My PR here #13236 chose to just use event_loop.exiting() which I think is simpler. I agree with the outstanding question about whether it makes sense to process events after exit has been signaled or not. If it is indeed true that we guarantee nothing happens after exit has been called, it makes sense to swallow additional incoming events. However, I don't understand all the ceremony around exit_happened and processing the LoopExiting event. Personally, i'd just wrap the entire handler in if !event_loop.exiting().

[tychedelia]

I think this is unreachable. We only receive this event if we call event_loop.exit(), which is guaranteed to have set *exit_happened = Some(app_exit).

[Brezak]

The event drop code specifically let's through the LoopExiting event.

bevy/crates/bevy_winit/src/lib.rs

Lines 364 to 367 in 851b21a

	if exit_happened.is_some() && event != Event::LoopExiting {
	debug!("App is exiting. Dropping event: {event:?}");
	return;
	}

I didn't put much thought into my solution as I know this code will get replaced soon^TM. I'd modify your solution to drop events after we start exiting the event loop and merge it instead of this one. That's not my call to make though.

[tychedelia]

I'm just saying we'll only receive LoopExiting if we've already called event_loop.exit() which in the control flow here guarantees exit_happened is Some. Agree that this code will likely change a lot post non-send changes.

[alice-i-cecile]

Closed in favor of #13236.