-
-
Notifications
You must be signed in to change notification settings - Fork 23.7k
GitHub Actions: Test project exporting on CI #109146
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
base: master
Are you sure you want to change the base?
Conversation
976845a to
cc2c229
Compare
779f6ad to
9b4c6c6
Compare
|
This is fantastic for testing purposes, and already coming along really nicely; great job! Though, the test project itself shouldn't be in the repo. It's not something to worry about during this testing phase, but we should do something similar to the regression test project where it's downloaded from a remote location |
00c8440 to
72a215a
Compare
e6cd321 to
4da0e30
Compare
|
Anything that can be done to help move this one forward? I'd love to see this merged in some form for 4.6, since it would've helped catch one or two of the regressions that I ran into during the last beta. |
4da0e30 to
ae3cd86
Compare
I'll take a look at it again next week. Once I get CI to pass, we can look into creating a new repository in the @godotengine organization then update this PR to remove the test project from it (and make it clone the other repository instead). There's already https://github.com/godotengine/godot-tests, but we don't seem to be using the tests from that repository anywhere currently. The alternative is to modify https://github.com/godotengine/regression-test-project to include two projects side-by-side (the existing project + the new one). What do you think? |
I guess using that repository makes sense. Eventually we might want to run more of this kind of integration tests which could be hosted in that repo. The existing tests may or may not be obsolete but you can likely replace the |
ae3cd86 to
24c1a8a
Compare
|
I'm getting a CI failure due to sanitizers on the regression test project (not the project added by this PR). How come it doesn't occur on |
7f8f543 to
1503d21
Compare
|
I've removed C# for now as it was too difficult to get working. We can look into it again in a future PR, especially since the way C# is integrated in Godot may change in the near future. I've also switched localization to gettext, as CSV constantly caused errors on export (even on my own machine in non-headless export). This is presumably due to its one-to-many nature, where importing a single CSV file creates multiple Also, the GDExtension build takes a very long time (10+ minutes). Is there a way to speed it up? Maybe we should commit a precompiled Linux library in the test project when I open a PR to godot-tests. |
I sped up the build times of godot-cpp in Godot Jolt a lot by using CMake's built-in "unity builds", i.e. what Godot refers to as SCU builds. I feel like it would be fairly straight-forward to add support for that to godot-cpp's SCons setup. You really only need it for the generated I'd be curious to hear what you think of that, @dsnopek. EDIT: Maybe caching the build artifacts in GitHub Actions is an option as well? |
5344dec to
8f3c0a8
Compare
|
I've removed VoxelGI and OBJ mesh/scenes from the test project for now, since they caused (benign) errors on headless export I couldn't fix. I have a fix in the works for VoxelGI, which will also make PCKs smaller by introducing a PlaceholderVoxelGI resource, but it'll be in a separate PR.
I'm still running into issues with GDExtension though. Running the project directly works, but running the exported PCK results in the GDExtension not being found. |
a851839 to
da704c3
Compare
7fa346c to
6bd65f7
Compare
527f6ff to
4a012e6
Compare
|
CI is passing now, so I've opened a PR on godot-tests with the test project: Once the above PR is merged, I'll change this PR to download the test project from godot-tests instead of embedding it here. |
4a012e6 to
93c23a1
Compare
|
@Calinou Any plans on resuming the GDExtension part of this if/when this is merged, or did you find yourself stuck on the dynamic library loading issue you were having? |
Yes, I'll look into it once this is merged. I've tried to use I've reverted the PR to use |
Just a reminder that this still needs to be done before merging. |
This allows finding issues in headless project export early on, including when exporting for a dedicated server. We also use this opportunity to check whether the audiovisual output between the project being run from its files and the exported PCK matches (it should always be a perfect match, assuming the same GPU is used for both runs). This can be used to catch audiovisual discrepancies, which could indicate a bug in the export process.
93c23a1 to
6d8d124
Compare
This allows finding issues in headless project export early on, including when exporting for a dedicated server.
We also use this opportunity to check whether the audiovisual output between the project being run from its files and the exported PCK matches (it should always be a perfect match, assuming the same GPU is used for both runs). This can be used to catch audiovisual discrepancies, which could indicate a bug in the export process. This also tests Movie Maker mode functionality while we're at it.
The test project is currently part of the main repository, so I've attempted to minimize the file size while making use of as many Godot features as possible. We could move this test project to a separate repository, but this would increase the complexity of the setup and make it harder for contributors to test this project locally (as well as further improve it).
This export test is performed on the Linux Editor w/ Mono job, which will allow for testing C# (not implemented in this PR yet).
The development of this PR has already uncovered several bugs so far:
--headlessmode #109177Sounds that were
Preview
The project features:
C#andGDExtension C++ using godot-cpp.OBJ scene/mesh importVoxelGI exportAudio playbackTODO
For a future PR (due to the complexity):