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

Adding a runtime selection dropdown into the editor #35

Merged
merged 1 commit into from
Mar 7, 2021

Conversation

BastiaanOlij
Copy link
Member

@BastiaanOlij BastiaanOlij commented Feb 18, 2021

Adding a dropdown (windows only) to the editor that allows us to select from different (known) runtimes:
image

The known runtimes are stored in runtimes.json in our addons folder so we can easily add more entries. On reading the JSON we will check if the paths to the runtime files exist and only offer those we find. We'll probably need to make it smarter to check the same path on different drives in case the user has decided to install Oculus/Steam on a D: or E: drive or something but that is for an enhancement PR.

If the current runtime is not found in the list, it is added (in editor only, the runtimes.json is not altered).

@BastiaanOlij
Copy link
Member Author

This sort of works but we don't seem to have enough rights to update the registry key. Looking into that.

@BastiaanOlij BastiaanOlij self-assigned this Feb 18, 2021
@BastiaanOlij BastiaanOlij added the enhancement New feature or request label Feb 18, 2021
@BastiaanOlij BastiaanOlij force-pushed the runtime_select branch 2 times, most recently from e6b382a to f77208a Compare February 22, 2021 14:29
@BastiaanOlij
Copy link
Member Author

Ok, so I couldn't get changing the registry entry through our plugin to work because of rights issues.

But with a pretty dirty hack I was able to get it to work with powershell.
The biggest issue is that I had to base64 encode the instructions. Powershell does not allow running scripts because somehow that is more unsafe, but it does allow us to base64 encode the script and just pass it through the command line. Go figure.
Problem is that powershell want the source data to be UTF16LE which Godot currently doesn't support as input to the base64 code. I cheated here, it'll work for our usecase. Burvzg added support to Godot 4, we'll need to look into backporting.

@BastiaanOlij BastiaanOlij marked this pull request as ready for review February 22, 2021 14:36
@BastiaanOlij
Copy link
Member Author

@ChristophHaag I've completely changed this to be based on the environment variable.
It needs: godotengine/godot#46413
Should in theory work on Linux but as I can't test I don't know, it may have a problem with the ~ not being parsed to the current user folder.

Copy link
Collaborator

@ChristophHaag ChristophHaag left a comment

Choose a reason for hiding this comment

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

With the home directory fix it works nicely on linux.

Short wishlist (maybe for the future):

  • Show the full path somewhere in the ui (tooltip on the dropdownbox?)
  • Persistently store the selected runtime
  • Allow adding runtimes from the UI

demo/addons/godot-openxr/runtimes.json Show resolved Hide resolved
demo/addons/godot-openxr/runtimes.json Show resolved Hide resolved
src/godot_openxr.cpp Outdated Show resolved Hide resolved
demo/addons/godot-openxr/plugin.gd Outdated Show resolved Hide resolved
demo/addons/godot-openxr/plugin.cfg Outdated Show resolved Hide resolved
@BastiaanOlij
Copy link
Member Author

Short wishlist (maybe for the future):

I've added the tooltip but the other two I think we should do in new PRs.
One thing to discuss here is where we store the updated JSON. I consider the addons folder in a project to be read only because you need to be able to replace it with an updated version when we release a new version of the plugin.

@BastiaanOlij
Copy link
Member Author

@ChristophHaag if you can have another quick look, I think this is ready to be merged. It works really well imho

@ChristophHaag
Copy link
Collaborator

looks good to me

@BastiaanOlij BastiaanOlij merged commit 6420b60 into GodotVR:master Mar 7, 2021
@BastiaanOlij BastiaanOlij deleted the runtime_select branch June 29, 2021 03:24
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