-
-
Notifications
You must be signed in to change notification settings - Fork 4k
Move Window creation from plugin build time to PreStartup #20019
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
base: main
Are you sure you want to change the base?
Conversation
entity_commands.insert(primary_cursor_options.clone()); | ||
} | ||
let primary_window = primary_window.clone(); | ||
let primary_cursor_options = self.primary_cursor_options.clone().unwrap_or_default(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the same behavior as before because CursorOptions
is a required component of Window
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm definitely a bit worried about this maybe breaking stuff, but the code looks good and it's really small so it will be easy to bisect.
Blocking until we figure out why this even works even though |
This reverts commit f6c8db4.
Blocked by #20021 |
Objective
Window
component for more granular change detection #19644Window
by observer instead of by plugin fields #20018Our
PrimaryWindow
and its associatedWindow
are currently spawned on plugin build time, hence making it very hard to write an observer for them likeOn<Add, PrimaryWindow>
Solution
PreStartup
. Using that schedule instead ofStartup
because the latter is more user-facing, and the user should not have to know about the details of when exactly the window is created. It should just exist.Testing
Additional Info
If all goes well, this change should make no difference to the user whatsoever.