GodotVR / godot_openxr_vendors

Godot 4 wrapper for OpenXR vendors loaders and extensions
MIT License
90 stars 19 forks source link

Migrate the export scripts from gdscript to C++ via gdextension #61

Closed m4gr3d closed 8 months ago

m4gr3d commented 9 months ago

This removes the need to enable the plugin before it's usable in the export preset settings.

BastiaanOlij commented 8 months ago

I wish we fixed the enabling of plugin logic, it feels like a dirty workaround :)

That said, I think this logic works much better as a GDExtension anyway. My only "concern" is that it's less accessible, it'll be harder to find contributors who want to work on this. Though that's more an issue for plugins like XR tools than this.

BastiaanOlij commented 8 months ago

Approving this seeing it makes sense to move forward with this, but I'm still on the fence if we should have an extension per platform.

It makes far more sense to me to have a single extension so that logic is properly shared and not registered multiple times with the IDE especially as vendors adopt each others extensions. We don't need separate plugins to have separate export classes.

m4gr3d commented 8 months ago

Approving this seeing it makes sense to move forward with this, but I'm still on the fence if we should have an extension per platform.

It makes far more sense to me to have a single extension so that logic is properly shared and not registered multiple times with the IDE especially as vendors adopt each others extensions. We don't need separate plugins to have separate export classes.

Unfortunately, so long as each vendor has its own openxr loader, it's not possible to have a single gdextension for all vendors since the loader shared libraries will conflict with each other.

BastiaanOlij commented 8 months ago

Unfortunately, so long as each vendor has its own openxr loader, it's not possible to have a single gdextension for all vendors since the loader shared libraries will conflict with each other.

The GDExtension just contains the editor and extension classes, it doesn't limit having separate folders with the loaders and picking the right loader to install right? Or is there something that prevents us from having multiple editor plugin classes in a single extension library?

I don't see anything in the current source that would prevent us from combining all into a single extension, we still have the separate loader folders for the AARs off course. Or am I missing something?

m4gr3d commented 8 months ago

Unfortunately, so long as each vendor has its own openxr loader, it's not possible to have a single gdextension for all vendors since the loader shared libraries will conflict with each other.

The GDExtension just contains the editor and extension classes, it doesn't limit having separate folders with the loaders and picking the right loader to install right? Or is there something that prevents us from having multiple editor plugin classes in a single extension library?

I don't see anything in the current source that would prevent us from combining all into a single extension, we still have the separate loader folders for the AARs off course. Or am I missing something?

The extension classes have vendor specific header dependencies (example) which prevents them from being easily combined.

The editor plugin classes could be in a single extension library since they don't have dependencies on the loaders, this would require them to be compiled separately from the extension classes however which would defeat the point of having a single extension library.