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

Refactor the project structure #202

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

m4gr3d
Copy link
Collaborator

@m4gr3d m4gr3d commented Sep 9, 2024

Refactor the project structure to reduce duplication of code, logic and build configs across the various vendors we support.

  • Consolidate all the vendors modules into a single godotopenxr module with multiple vendors product flavors to allow for vendor-specific customizations
  • Consolidate both gradle and cmake build configs
  • Add GodotOpenXR.kt to consolidate common java/kotlin logic for the OpenXR vendors plugin implementations
  • Move native code from the common directory to the godotopenxr/src/main/cpp directory

Note the changes in this PR are primarily due to file migration and updates of the logic and build configs as needed to match the new layout. No capabilities, functionality were added / removed / lost as part of the refactor.

@m4gr3d m4gr3d added the enhancement New feature or request label Sep 9, 2024
@m4gr3d m4gr3d added this to the 3.1.0 milestone Sep 9, 2024
@m4gr3d m4gr3d force-pushed the consolidate_project_structure branch from 71e0a65 to c8c935a Compare September 9, 2024 03:17
@dsnopek
Copy link
Collaborator

dsnopek commented Sep 9, 2024

Overall this looks good to me! I tested it locally, and everything seemed to work fine.

However, I wonder if we should call the directory godotopenxrvendors rather than just godotopenxr? I know this internally within the project, but I don't want to give the impression that this is adding OpenXR support to Godot in general (as opposed to just implementing vendor extensions).

@m4gr3d
Copy link
Collaborator Author

m4gr3d commented Sep 9, 2024

Overall this looks good to me! I tested it locally, and everything seemed to work fine.

However, I wonder if we should call the directory godotopenxrvendors rather than just godotopenxr? I know this internally within the project, but I don't want to give the impression that this is adding OpenXR support to Godot in general (as opposed to just implementing vendor extensions).

Sounds good to me. I went with godotopenxr to reflect the current structure; i.e: we're going from:

  • godotopenxrkhronos
  • godotopenxrlynx
  • godotopenxrmeta
  • godotopenxrpico

To:

  • godotopenxr
    • khronos
    • lynx
    • meta
    • pico

We could also set the name to reflect its function (i.e: plugin) especially given the project will already be checked out within a godot_openxr_vendors root directory.

@dsnopek
Copy link
Collaborator

dsnopek commented Sep 9, 2024

We could also set the name to reflect its function (i.e: plugin) especially given the project will already be checked out within a godot_openxr_vendors root directory.

Yeah, that makes sense! And plugin is shorter. :-) I think I personally like this option best so far

@m4gr3d m4gr3d force-pushed the consolidate_project_structure branch from c8c935a to 73af0da Compare September 11, 2024 21:22
@m4gr3d
Copy link
Collaborator Author

m4gr3d commented Sep 11, 2024

We could also set the name to reflect its function (i.e: plugin) especially given the project will already be checked out within a godot_openxr_vendors root directory.

Yeah, that makes sense! And plugin is shorter. :-) I think I personally like this option best so far

Done!

Copy link
Collaborator

@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.

Looks great!

I tested rebuilding and running the composition layers sample on Quest 3, and everything seems to work.

- Consolidate all the vendors modules into a single `plugin` module with multiple vendors product flavors to allow for vendor-specific customizations
- Add `GodotOpenXR.kt` to consolidate common java/kotlin logic for the OpenXR vendors plugin implementations
- Move native code from the `common` directory to the `plugin/src/main/cpp` directory
@m4gr3d m4gr3d force-pushed the consolidate_project_structure branch from 73af0da to 03e19c5 Compare September 13, 2024 14:34
@m4gr3d m4gr3d merged commit b854a74 into GodotVR:master Sep 13, 2024
10 checks passed
@m4gr3d m4gr3d deleted the consolidate_project_structure branch September 13, 2024 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants