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

Create winit windows before app.initialize() #916

Merged
merged 1 commit into from
Nov 26, 2020
Merged

Create winit windows before app.initialize() #916

merged 1 commit into from
Nov 26, 2020

Conversation

smokku
Copy link
Member

@smokku smokku commented Nov 22, 2020

This is required so startup systems have access
to Windows and WinitWindows resources.

@smokku
Copy link
Member Author

smokku commented Nov 22, 2020

This commit restores code that was removed in #879

@cart
Copy link
Member

cart commented Nov 22, 2020

Haha this one is my fault. @bjorn3 mentioned that the line wasn't necessary and I totally forgot that we added it for a reason. Can you add a comment above that line to indicate why its there to protect future contributors from future me?

This is required so startup systems have access
to Windows and WinitWindows resources.
@smokku
Copy link
Member Author

smokku commented Nov 22, 2020

Sure. Done.

@Moxinilian Moxinilian added C-Bug An unexpected or incorrect behavior A-Windowing Platform-agnostic interface layer to run your app in labels Nov 23, 2020
@Moxinilian
Copy link
Member

I cannot see the comment being added, are you sure you pushed it properly?

@smokku
Copy link
Member Author

smokku commented Nov 23, 2020

I did, then I force-pushed it gone. I force-pushed it back again now.

I really should not be doing PRs from master.
Unfortunately GH does not allow me to change the source branch for PR.

@@ -147,6 +147,14 @@ pub fn winit_runner(mut app: App) {

app.resources.insert_thread_local(event_loop.create_proxy());

// Create Windows and WinitWindows resources, so startup systems
// in below app.initialize() have access to them.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd take out "in below app.initialize()" but that's not a big deal.

// Create Windows and WinitWindows resources so
// startup systems have access to them.

Copy link
Member Author

Choose a reason for hiding this comment

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

The goal of this mention is so one do not move the call below app.initialize() as this would render it useless.

@cart cart merged commit fd6b787 into bevyengine:master Nov 26, 2020
@smokku smokku mentioned this pull request Dec 12, 2020
@fopsdev fopsdev mentioned this pull request Jan 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Windowing Platform-agnostic interface layer to run your app in C-Bug An unexpected or incorrect behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants