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

Change application/version setting source to remove redundancy #67

Merged
merged 2 commits into from
Dec 18, 2023

Conversation

patrickdown
Copy link
Collaborator

This change remaps some settings
It removes xr/tilt_five/application_id and uses application/config/name instead.
It removes xr/tilt_five/application_version and uses application/config/version instead.

This change is in alignment with the intended use of these fields for identifying the application in the T5 service logging.

Additionally if xr/tilt_five/default_display_name does not have a supplied value application/config/name is used.

@Malcolmnixon
Copy link
Collaborator

Is your intent to do the equivalent C# changes in a different PR?

This change remaps some settings
It removes `xr/tilt_five/application_id` and uses  `application/config/name` instead.
It removes `xr/tilt_five/application_version` and uses `application/config/version` instead.

This change is in alignment with the intended use of these fields for identifying the application in the T5 service logging.

Additionally if `xr/tilt_five/default_display_name` does not have a supplied value `application/config/name` is used.

Changed for gdscript and C# also
@patrickdown
Copy link
Collaborator Author

Is your intent to do the equivalent C# changes in a different PR?

Wow, completely missed that. I amended it with C# changes.

Copy link
Collaborator

@Malcolmnixon Malcolmnixon left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@patrickdown patrickdown merged commit a0207ea into GodotVR:main Dec 18, 2023
1 check passed
@patrickdown patrickdown deleted the move_app_info branch December 18, 2023 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants