GH-868 Bump godot-engine/godot-cpp to 4.4.beta2

[Naros]

Fixes #868

• Fix documentation, does not appear that node docs are being incorporated
• Remove cmake/godot-docs-generator (now included in CMakeLists.txt)
• Removed commented lines in the CMake configuration

For now given that the godot-cpp docs generator hook isn't working, I've retained our implementation for now.

[Naros]

Platform build mappings need to be fixed:

• The MacOS builds as orchestrator.macos.32.release.dylib but expected orchestrator.darwin.64.release.dylib.
• The Windows builds as orchestrator.windows.64.release.dll, which is OK.
• The arm64 builds as orchestrator.android.32.release.so, but expected liborchestrator.android.64.release.so.
• The arm32 builds as orchestrator.android.32.release.so, but expected liborchestrator.android.32.release.do.
• The Linux builds as orchestrator.linux.64.release.so, which is OK.

In addition, there are a few build warnings to fix:

• Arm32/Arm64/Linux

  • Warnings method_utils.cpp lines 162/176 - integers of different signs: 'int' and 'size_type'

• Arm32/Arm64

  • graph_edit.cpp:1510 lambda capture 'this' is not used

[enetheru]

These filenames I assume were taken from my PR, but you have specifically set the name to use ORCHESTRATOR_ARCH so this will break.

[Naros]

Hi @enetheru agreed, I have highlighted this in the above comment. I don't have a way to test the builds locally, so this was my first pass to see how things would land on multi-architecture builds. Will be fixed in the next rebase.

[enetheru]

The godot-cpp defines the msvc runtime flags as public, so will propogate them to consumers, there shouldnt be a need to set them manually like this.

infact, it currently has all flags as public which is something I want to change, but the msvc runtime ones will remain public.

[Naros]

Great, I see the logic for this in windows.cmake, thanks for the reference.

[Naros]

@enetheru are there any plans to backport these cmake changes in godot-cpp to prior branches or this only applies to 4.4+?

[enetheru]

@Naros I would love that to happen, there isnt much else other than cmake code thats changed. Perhaps the python profile stuff.
@dsnopek would be the person to talk to regarding this.

I think it could easily be backported to all 4.x versions.

[Naros]

Hi @enetheru it seems that if the CMAKE_SYSTEM_PREPROCESSOR is set to armv7-a which is meant to designate the build of Android for arm32, the GODOT_ARCH variable is set to unknown. This resolve to ORCHESTRATOR_ARCH being set to 32, but my question is does this properly compile godot-cpp ?

[Naros]

Hi @enetheru another issue I notice is that the ${Python3_EXECUTABLE} variable isn't being set. This is why the documentation is not being generated with this PR. Should godot-cpp fail or write some warning if that executable can't be found by the find_package call?

UPDATE: I added find_package(Python3 3.4 REQUIRED) to my cmakelist.txt and its being resolved. But I still don't see the doc_data.cpp file being generated, although the function is being called. I'll keep investigating.

[enetheru]

Hi @enetheru it seems that if the CMAKE_SYSTEM_PREPROCESSOR is set to armv7-a which is meant to designate the build of Android for arm32, the GODOT_ARCH variable is set to unknown. This resolve to ORCHESTRATOR_ARCH being set to 32, but my question is does this properly compile godot-cpp ?

Ignore this reply :

I did suspect this will happen when I translated the code from the SCons system for this, which also is missing a valid alias.

Check the cmake/godotcpp.cmake:47 for the function( godot_arch_map ALIAS PROC )
In there you will see which aliases there are. And it shoud match tools/godotcpp.py:180 architecture_aliases = {
The selection logic in the scons is also at tools/godotcpp.py:415

I'm going to add armv7-a to the alias list and add a WARNING to raise a bug if the architecture is unknown but valid so we > can add it to both CMake and SCons

Edit: I'll just have quick look at the android.cpp code first, there is some mention of architecture in there.
Edit2: Looks like I have some work to do today.

[enetheru]

UPDATE: I added find_package(Python3 3.4 REQUIRED) to my cmakelist.txt and its being resolved. But I still don't see the doc_data.cpp file being generated, although the function is being called. I'll keep investigating.

Interesting, it appears that find_package does not make the package available globally, and that in cmake 3.24 a couple more variables were added to enable such a feature( find_package( ... GLOBAL ... and CMAKE_FIND_PACKAGE_TARGETS_GLOBAL)

So consumers are required to find python if they want to use those functions, we need to add a guard to the functions so that we can warn about such things. Maybe designate a module to include for consumers.. what approach would you like most?

[enetheru]

what approach would you like most?

Another approach is to add godot-cpp/cmake to CMAKE_MODULES_DIR like you were doing before. I would rename python_callouts.cmake into GodotCPPModule.cmake or something to match the cmake naming scheme and put the find_package( Python ... in that file behind a guard so that it gets included in the main project.

Perhaps like this: https://github.com/enetheru/godot-cpp/tree/cmake_module

[enetheru]

Hi @enetheru it seems that if the CMAKE_SYSTEM_PREPROCESSOR is set to armv7-a which is meant to designate the build of Android for arm32, the GODOT_ARCH variable is set to unknown. This resolve to ORCHESTRATOR_ARCH being set to 32, but my question is does this properly compile godot-cpp ?

OK so after re-going over all the things again because it's been a while since I wrote that part.

Short answer is yes. The toolchain or ANDROID_ variables select the compiler and architecture, and the godot-cpp code doesn't change any of that (except for OSX which I will explain shortly)

I needed to match the SCons output name, so I create the godot_arch_map function to transform CMAKE_SYSTEM_PROCESSOR into the arch name used by SCons, but it doesnt include all aliases, as you have discovered. I saw in the android.py there is a shortlist so I will make sure they are included, and drop the unknown in favour of CMAKE_SYSTEM_PROCESSOR

The perhaps exception to the case is when I needed to match the 'universal' arch for darwin which sets the OSX_ARCHITECTURES property of the targets. to make both "x86_64;arm64" binaries, but there has been discussion lately of scrapping the universal builds in favour of an extra build step after the fact that merges the two together.. I am waiting on results of that discussion.

Thank for the questions. :D

[enetheru]

The perhaps exception to the case is when I needed to match the 'universal' arch for darwin which sets the OSX_ARCHITECTURES property

After the MSVC_RUNTIME stuff, I can see this as being problem too now, so I think I will remove GODOT_ARCH entirely, and leave it upto the consumer to set CMAKE_OSX_ARCHITECTURES.

if I can map whatever architectire to one of the known names from godot then I will, otherwise I'll default to CMAKE_SYSTEM_PROCESSOR what do you think?

Edit: Here is a branch with the changes: https://github.com/enetheru/godot-cpp/tree/arch_confusion

[enetheru]

Figured out that this is the reason the doc_source_generator isnt working, this GLOB pattern doent find the file since it hasnt been generated yet.

The custom command to generate the sources relies on the result being added to a target, and since only the results from the GLOB are added(missing the doc_source.cpp) the dependency chain to generate the file doesnt exist.

Moving the "${DOC_DATA_CPP_FILE}" down to the add_library as an explicit file dependency will solve it.

[enetheru]

how about this trick?

math( EXPR ORCHESTRATOR_ARCH "${CMAKE_SIZEOF_VOID_P} * 8" ) # CMAKE_SIZEOF_VOID_P refers to target architecture.