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.

Sign up for GitHub

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

Jump to bottom

Change Server initialization order #61436

Merged
merged 1 commit into from
Jun 1, 2022
Merged

Change Server initialization order #61436

merged 1 commit into from
Jun 1, 2022

Conversation

reduz
Copy link
Member

@reduz reduz commented May 26, 2022

  • Registration of server classes happened after Display initialization.
  • This made no sense in practice and avoided the registration of custom server drivers (like custom XR server, custom Rendering server, custom XR server).
  • Initialization moved to before Display.

@reduz reduz requested a review from a team as a code owner May 26, 2022 12:53
@Chaosus Chaosus added this to the 4.0 milestone May 26, 2022
BastiaanOlij
BastiaanOlij approved these changes May 26, 2022
View reviewed changes
Copy link
Contributor

@BastiaanOlij BastiaanOlij left a comment

Choose a reason for hiding this comment

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

I'm getting a weird crash later on in the process when testing OpenXR but I don't think this is due to this PR. This PR seems to do exactly what it should and allows me to get through the OpenXR initialisation

@akien-mga
Copy link
Member

There seems to be issues with running --dump-extension-api and --generate-mono-glue.

@reduz
Copy link
Member Author

reduz commented May 26, 2022

@akien-mga I tried to replicate it locally and I can't. I am not entirely sure what may be causing them.

@BastiaanOlij
Copy link
Contributor

BastiaanOlij commented May 28, 2022 •
edited
Loading

Fix is here: #61585

Turns out it is because when we initialise RenderingServer, it calls back into itself which is not allowed in a constructor.

This was referenced Jun 1, 2022
Merged
Closed
@akien-mga akien-mga force-pushed the change-server-initialization-order branch from 8c419bc to c8925c7 Compare June 1, 2022 14:47
@reduz
Change Server initialization order
54542ef 
* Registration of server classes happened after Display initialization.
* This made no sense in practice and avoided the registration of custom server drivers (like custom XR server, custom Rendering server, custom XR server).
* Initialization moved to _before_ Display.
@akien-mga akien-mga force-pushed the change-server-initialization-order branch from c8925c7 to 54542ef Compare June 1, 2022 14:48
@akien-mga akien-mga merged commit 79aa13a into godotengine:master Jun 1, 2022
@akien-mga
Copy link
Member

Thanks!

@ModularNucleus ModularNucleus mentioned this pull request Jun 4, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BastiaanOlij BastiaanOlij BastiaanOlij approved these changes

@akien-mga akien-mga akien-mga approved these changes

Assignees
No one assigned
Labels
enhancement topic:core
Projects
None yet
Milestone
4.0
Development

Successfully merging this pull request may close these issues.
4 participants
@reduz @akien-mga @BastiaanOlij @Chaosus