Register missing types in bevy_window
Objective
- Fixes https://github.com/bevyengine/bevy/issues/7990.
Solution
- Register needed types, verified pasted code in issue works.
Do I need to register more Option<T> types?
IMO we should not be serializing window state in scenes. This feels like a massive footgun waiting to happen.
I'm basically in agreement with that. I think we should instead exclude these components and entities from the default serialization.
I'm basically in agreement with that. I think we should instead exclude these components and entities from the default serialization.
Should be possible once we merge #6793
IMO we should not be serializing window state in scenes. This feels like a massive footgun waiting to happen.
As in, scenes should not be able to spawn windows? If so I strongly disagree. "Windows in scenes" seems like a useful feature and was one of the reasons used to justify windows as entities. Our Window type is designed to be a "high level user facing component" and I'm not seeing anything that would be particularly troublesome here.
I think that serializing windows-as-entities is a valid use case, but not something that's going to be desired most of the time.
I think we should merge this, but not for 0.10.1, and not until #6793 is shipped to give users better control over what exactly is getting stored in the scene.
Dynamic scenes being broken really sucks, but in the current state the linked code will try to save the game, grabbing window state. When that scene is later spawned we'll have two windows, which will be very surprising (especially for a minor version bump), and will often panic user games which call query.single on windows.
I guess thats reasonable. Hard to say which behavior is more broken: serialization being broken by default, or spawning serialized default worlds duplicating the window created by DefaultPlugins.
Definitely worth discussing more / resolving these interactions. I think I'm still in favor of fixing this now because:
- This is a crash in a common use case
- Supporting this scenario was intended
- The duplicate window problem is resolved by disabling default window creation.
But if you still feel like this should wait thats enough controversy to hold off until later.
This is a crash in a common use case
I guess "common" is a pretty big stretch of the word
You've convinced me: weak preference to merge now, as the created buggy behavior is easier for end users to work around :p