Update 2.0-dev to Godot 4.2 + mingw support #151
Conversation
vilhalmer
force-pushed
the
2.0-dev-godot4.2
branch
from
a8608d4 to
af07c5f
Compare
This is always an annoying part of working with GDExtension. Godot internally can be compiled with double float support. Generally speaking, if it works for the downloadable version of Godot, I'm happy :)
I'm happy for that change, I do have plans to change XR tools in a way that it could be submoduled, but it has some downsides to maintaining it so it's unlikely that will happen soon.
Honestly, don't know either, just copied from the base implementation myself.
I'm surprised they re-added it as there is nothing that will be using it.. That said, OpenXR has introduced Mac support so maybe they are following suit? |
|
Note that CI needs to be updated with newer versions so the build checks run through. |
|
I've updated CI, though I can't guarantee it will work on the first try :) Edit: realized I can test this out on my fork, so I'll work out any issues on a different branch there. |
vilhalmer
force-pushed
the
2.0-dev-godot4.2
branch
from
71dc283 to
80e9e45
Compare
|
There we go: https://github.com/vilhalmer/godot_openvr/actions/runs/8810090117 Apparently the action versions I updated to also throw deprecation warnings, but at least they're not... as deprecated? I'll plan on a second pass to get them to the latest versions soon. |
|
What is blocking this from merge? |
|
I haven't been pushing on it because I'm taking my time with the followups to make sure I haven't made any terrible API decisions, but I think this one is ready. I think Bastiaan just has a lot of higher priority projects :) I'll take a look and see if there are any stray commits I should tack on here in the next day or two. |
|
Having browsed through the changes I don't see anything that stands out to me as wrong. If other can test this and verify it is working as expected, I'm more than happy to merge this in. |
|
Once we do I think we also need to spend some time rejigging the branches. The master branch should probably be renamed to 3.x, while the 2.0-dev branch becomes our master branch. |
If we can avoid the microsoft redistributable it means that game player don't need to install https://learn.microsoft.com/en-us/cpp/windows/latest-supported-vc-redist?view=msvc-170 |
|
How did it go? |
|
I didn't find anything additional that should go in here, it should be good for people to test. |
|
In Godot Engine main we tend rebase and squash all commits into one, but not sure about GodotVR/godot_openvr. I would like squashing. |
|
I've just left it for review purposes since Github offers the option at merge time, but if that's not enabled on this repo I'll definitely squash. Might keep the build work in a separate commit. |
|
I had to pull the branch onto my fork to be able to build since all the builds expired. https://github.com/V-Sekai/godot_openvr/tree/2.0-dev-overlays |
|
Test plan:
|
|
The github actions only provide |
|
I had some trouble building for Godot Engine 4.3 so I rewrote the scons script rushed. Did not upgrade the demo project. Did not restore linux. Did not restore mingw. https://github.com/V-Sekai/godot_openvr/tree/2.0-dev-overlays-4.3-stable
Note: took pieces from https://github.com/godotengine/godot-cpp-template |
|
I'll take a look at your changes tomorrow and see if I can figure out what's wrong in my version. I admittedly have never compiled it with a full Visual Studio environment, because I was specifically avoiding setting one up. I had it building with the bare tools installed via winget, but I haven't tried it again since I got it working. I want to do a lot of scons cleanup, but i was trying to avoid doing any more than I already did here. Maybe it would be better to just drop my build changes here and introduce a new PR for that once I have it reconciled. |
|
Sorry for the delay, Friday ended up being a very different day than I had planned.
Can you share the errors and some info about your build environment? Is it Visual Studio? |
|
I won’t be able to check for the next week but my goal is mingw, msvc and linux gcc and linux clang. |
|
Ok, some errors had crept in with my local code that aren't present in this branch, but when I check out this branch I'm able to run scons without any arguments with a local openvr checkout and get a build with the MSVC toolchain (warnings trimmed out): I'm using the standalone build tools from here.
Were you building with |
|
That sounds great. I will be traveling soon so I might not able to test.
Please do. |
|
I'm back, what do we need to do? |
|
If you could provide as much detail about your build environment as possible, that would be great. I want to be able to reproduce the issue locally and make sure I have automated builds that check everything works in the future. |
|
It's fully compiled for me, but I don't have a test project that operates steamvr. |
|
That doesn't help me though since you compiled it by throwing out the entire build system 😅 |
|
Perhaps you can get it to build on github actions? |
|
I'm asking for details about your environment because it has a known failure with my code that I am trying to merge in this PR. This seems like a basic debugging step to ensure that others will actually be able to build. |
|
I'm more than happy to follow your lead on this. I don't have the time to give this a proper test myself but if at least two people can confirm the OpenVR plugin is working for them in Godot 4.x I'm happy to merge this if the CI clears. I also suggest that after that, I do a bit of git-fu and change the current master branch to a 3.x branch for legacy, and change the 2.0-dev branch to become the new master. |
|
Also @fire thanks for the ping! I wouldn't have caught the updates otherwise. |
|
My criteria for merging would be for two people to:
|
|
I have an minimal openxr demo from godotengine/godot#97907. https://github.com/user-attachments/files/17271320/demo-bug.zip we may also modify this if it's easier than modifying xr tools |
|
I also have at least four more PRs worth of work once this first bit is done, so if we want to wait a little longer before converting 2.0-dev to master we can get some feedback on some breaking API changes before calling 2.0 "done". |
|
We can do the whole alpha, beta, rc thing if you want. It's not really necessary but we can do it as an aid. |
|
Nah, I don't think we need to go that far. I'll get everything else I have outstanding posted in parallel (as much as possible) after this is in and we can figure out our strategy then. Your merge criteria seem reasonable to me, now we just need to find one more person :D |
|
You'll always need to tweak a demo a little because OpenXRs action map system is fairly different to OpenVRs one implementation wise. BUt other than that agree with Fires list of tickboxes. And indeed, I don't think we need to go through a whole alpha/beta/whatever release thing. The audience isn't big enough to make that worth the effort. After things are merged and we change over the branches, we can look into doing a release and if more issues are found, they will just become patch releases with fixes. |
|
@BastiaanOlij do you want me to manually squash this, or does this repo have the automatic squash on merge enabled? |
|
The general godot policy is the pull request maker squashes. |
|
Can you squash? |
|
I could, but we still want another person to test it and there will be at least one more change to fix the issue you pointed out with github actions. Every change to this branch is a bunch of work on my end to keep downstream branches updated, is it really needed to do it right this second? I promise I'm not going to forget to do it before we merge. (Sorry for the delay, last week was chaotic.) |
|
sorry! haven't been able to test. is there a ready to use example to send to my friends for testing |
|
I'll try to get actions packaging up the extension correctly, then the demo project in this repo should be runnable by replacing (But if your friends don't mind compiling code, it should Just Work™ after a build because the results go into the demo project by default.) |
|
I couldn't package the extension correctly, and the demo wasn't set up. Sorry! |
|
Ok, I think CI is producing the whole addon correctly now. Here's a run where I hacked it to produce the final zip file for non-master: https://github.com/vilhalmer/godot_openvr/actions/runs/11785210944 And the results of the code that's actually committed, skipping the release artifact correctly but still producing the intermediate platform-specific artifacts: https://github.com/vilhalmer/godot_openvr/actions/runs/11785458499
As noted in the readme I removed the xr tools from the repo so they need to be installed from the asset library. I'll flesh out that section a bit more to explain where to download the built addon. |
|
I'm a bit delayed, but I'll check as soon as I can. |
|
Can you package xr tools by downloading an exact version https://github.com/GodotVR/godot-xr-tools by commit zip?
|
|
I guess we could do that, I removed it because it's not actually needed for anything but the demo and most people will care about the demo once at most. @BastiaanOlij do you have an opinion about this? I'd almost be tempted to rework the demo to just not use it, simpler seems better so people don't get the components mixed up when they're new to all this. |
I'd say the simpler we keep it, the better. We can always refer to XR tools in the documentation as a suggestion. |
|
It was work, but I could assemble the package and test it.
https://github.com/V-Sekai/godot_openvr/releases/download/v2.0-dev-godot4.2/demo.zip
|
|
I think all those things can be addressed separately. I'm happy to merge this. |
- Update godot-cpp to track 4.2 - Update openvr to 2.0.10 - Remove bundled godot-xr-tools - Remove support for MacOS, never tested and long broken - Update gdextension interface for changes in godot 4.1 - Support finding system-installed libopenvr on platforms with pkg-config - Support for mingw in build using tunabrain-patched openvr header - Add autoload singleton to register XR interface with server - This works around godotengine/godot#64975 the same way as the reference XR and TiltFive extensions. - Default to static libc++ to match godot-cpp - Remove CRT debug flag to match godot and godot-cpp - Update CI to publish gdextension artifacts correctly
vilhalmer
force-pushed
the
2.0-dev-godot4.2
branch
from
05dfabf to
2d603cc
Compare
|
Squashed! |
|
@BastiaanOlij let's do it. |
|
Merged, and I renamed the branches so you'll have to do a full reset of your local master branch and do a forced push to your fork. |
|
Thank you! More to come very soon! |
With these changes, OpenVR scene applications work again in Godot 4.2. The demo is functional. Overlays do not yet work, I'll have a followup PR to fix them with some changes for how they're created and rendered as discussed in Discord.
While I was adjusting the build system to work with latest godot-cpp, I also added support for building using mingw. I can attempt to break that into a separate PR if it would be easier to review, though I did most of my testing with a mingw build because I am new to development on Windows in general and keeping my usual toolchain made progress a lot faster. There were no code changes to support this, just some new logic for scons.
I did test MSVC builds from within the Build Tools environment, but I have not tested inside the IDE. There were no significant changes to that part of the SConstruct, but testing from someone more familiar with this whole toolchain would be very welcome. The MSVC build spits out a lot of type conversion warnings which gcc does not, and I intend to continue investigating those before this is merged to make sure they're safe and/or clean them up.
I'm currently without an actual headset to do final testing after getting MSVC working again. I should have it back in a week or two, but I figured I might as well get some other eyeballs on this in the meantime.
Things I'm not sure about
Other notes
This mingw support is not the cross-compilation support from #99, this is only support for compiling for Windows within a mingw/msys2 environment on Windows. I'll work on getting cross-compilation up to date in a separate PR if everything looks good, as I'd like to be able to build my app for both platforms within a single pipeline.
I can also clean up the commit history a bit if that's desired, I was figuring things out as I went and thought it would be best to preserve my train of thought for review.
I left some TODOs in SConstruct for other things I'd like to fix up to make the build more robust, but didn't want to balloon the changes any further just yet.