Skip to content

Conversation

@ChristophHaag
Copy link
Contributor

No description provided.

@ChristophHaag ChristophHaag requested a review from a team as a code owner October 8, 2025 23:34
@ChristophHaag ChristophHaag force-pushed the initialize_XrRenderModelPropertiesEXT_type branch from 316d93a to fdb785a Compare October 9, 2025 06:03
@AThousandShips AThousandShips changed the title initialize XrRenderModelPropertiesEXT::type Ensure XrRenderModelPropertiesEXT::type is initialized Oct 9, 2025
@AThousandShips AThousandShips added this to the 4.6 milestone Oct 9, 2025
Copy link
Contributor

@devloglogan devloglogan left a comment

Choose a reason for hiding this comment

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

Please update this so it follows the braced initialization format like the rest of this file, see the properties_info initialization immediately above.

So something like

XrRenderModelPropertiesEXT properties = {
	XR_TYPE_RENDER_MODEL_PROPERTIES_EXT, // type
	nullptr, // next
};

@ChristophHaag
Copy link
Contributor Author

I had that first, but mac ci complained that the whole struct needs to be initialized with this syntax so it would be

	XrRenderModelPropertiesEXT properties = {
		XR_TYPE_RENDER_MODEL_PROPERTIES_EXT,
		nullptr,
		{},
		false,
	};

even though only typed and next need to actually be initialized, the rest is (over)written by the runtime. Should I change it to that?

@devloglogan
Copy link
Contributor

Huh, I didn't realize CI had that restriction! I personally would still change it to that though since that's the standard, including the variable names in the comments. And I'd put a 0 for animatableNodeCount rather than false.

@ChristophHaag ChristophHaag force-pushed the initialize_XrRenderModelPropertiesEXT_type branch from fdb785a to e33e069 Compare October 9, 2025 16:45
Copy link
Contributor

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good to me :-)

@Repiteo Repiteo merged commit 1274fac into godotengine:master Oct 13, 2025
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Oct 13, 2025

Thanks! Congratulations on your first merged contribution! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants