Skip to content

Conversation

Faless
Copy link
Contributor

@Faless Faless commented Aug 24, 2022

Including hashfuncs (or any hash_* template) was broken and causing build error due to missing functions.

In file included from /media/SSD/Code/git/godot-cpp-dev/include/godot_cpp/templates/hash_map.hpp:36,
                 from src/tests.h:35,
                 from src/register_types.cpp:40:
/media/SSD/Code/git/godot-cpp-dev/include/godot_cpp/templates/hashfuncs.hpp: In static member function 'static uint32_t godot::HashMapHasherDefault::hash(const godot::Vector4i&)':
/media/SSD/Code/git/godot-cpp-dev/include/godot_cpp/templates/hashfuncs.hpp:211:16: error: 'hash_murmur3_one_32' was not declared in this scope; did you mean 'hash_djb2_one_32'?
  211 |   uint32_t h = hash_murmur3_one_32(p_vec.x);
      |                ^~~~~~~~~~~~~~~~~~~
      |                hash_djb2_one_32
/media/SSD/Code/git/godot-cpp-dev/include/godot_cpp/templates/hashfuncs.hpp:215:10: error: 'hash_fmix32' was not declared in this scope
  215 |   return hash_fmix32(h);
      |          ^~~~~~~~~~~
/media/SSD/Code/git/godot-cpp-dev/include/godot_cpp/templates/hashfuncs.hpp: In static member function 'static uint32_t godot::HashMapHasherDefault::hash(const godot::Vector4&)':
/media/SSD/Code/git/godot-cpp-dev/include/godot_cpp/templates/hashfuncs.hpp:228:16: error: 'hash_murmur3_one_real' was not declared in this scope
  228 |   uint32_t h = hash_murmur3_one_real(p_vec.x);
      |                ^~~~~~~~~~~~~~~~~~~~~
/media/SSD/Code/git/godot-cpp-dev/include/godot_cpp/templates/hashfuncs.hpp:232:10: error: 'hash_fmix32' was not declared in this scope
  232 |   return hash_fmix32(h);
      |          ^~~~~~~~~~~
scons: *** [src/register_types.os] Error 1

This PR re-synchronize the hashfuncs with upstream godot, adds some missing math funcstions, and includes all godot_cpp/templates/* headers in the test project (so we ensure they can at least be built).

Note: We should probably re-synchronize the other templates too since I believe they are outdated but I'd rather do that in a separate PR.

@Faless Faless added bug This has been identified as a bug topic:gdextension This relates to the new Godot 4 extension implementation labels Aug 24, 2022
@Faless Faless added this to the 4.0 milestone Aug 24, 2022
@akien-mga akien-mga requested a review from a team August 24, 2022 13:22
@Faless
Copy link
Contributor Author

Faless commented Aug 24, 2022

I'm not sure why the CMake MSVC build is failing, the SCons one seems to pass. Any CMake guru that can help?

tefusion added a commit to tefusion/godot-cpp that referenced this pull request Sep 4, 2022
tmp fix so pipelines work until godotengine#819 is merged
@akien-mga
Copy link
Member

No idea how to fix the MSVC issue but given that it's failing with a syntax error in gen/include/godot_cpp/variant/utility_functions.hpp which seems to include most of the stuff found in math_funcs.hpp, I suspect it might be unhappy about the is_nan / is_inf additions. This kind of MSVC syntax error typically happens when a symbol has been overridden or isn't defined.

tefusion added a commit to tefusion/godot-cpp that referenced this pull request Sep 8, 2022
tmp fix so pipelines work until godotengine#819 is merged
@akien-mga
Copy link
Member

If someone is motivated, I made #834 to get verbose output from the CMake builds to be able to compare the build flags used by SCons and by MSVC, both using VS 2019.

CMake:

C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\bin\HostX64\x64\CL.exe /c /I"D:\a\godot-cpp\godot-cpp\include" /I"D:\a\godot-cpp\godot-cpp\gen\include" /Zi /nologo /W3 /WX /diagnostics:column /O2 /Ob0 /D _MBCS /D WIN32 /D _WINDOWS /D DEBUG_ENABLED /D DEBUG_METHODS_ENABLED /D TYPED_METHOD_BIND /D WIN32_LEAN_AND_MEAN /D _CRT_SECURE_NO_WARNINGS /D "CMAKE_INTDIR=\"Debug\"" /Gm- /EHsc /MD /GS /fp:precise /Zc:wchar_t /Zc:forScope /Zc:inline /GR /std:c++17 /Fo"godot-cpp.dir\Debug\\" /Fd"D:\a\godot-cpp\godot-cpp\bin\godot-cpp.windows.release.64.pdb" /external:W0 /Gd /TP /wd4244 /wd4305 /wd4101 /wd4018 /wd4267 /wd4099 /errorReport:queue  /external:I "D:/a/godot-cpp/godot-cpp/godot-headers" [ ... ]

SCons:

cl /Fogen\src\classes\sprite_base3d.windows.debug.x86_64.obj /c gen\src\classes\sprite_base3d.cpp /TP /std:c++17 /nologo /Z7 /Od /EHsc /D_DEBUG /MDd /DTYPED_METHOD_BIND /DDEBUG_ENABLED /DDEBUG_METHODS_ENABLED /Igodot-headers /Iinclude /Igen\include

Maybe one of those triggers the error.

Either way, to unblock this I would suggest either trying to remove the is_nan/is_inf definitions, or disable the CMake build on CI temporarily.

@Faless
Copy link
Contributor Author

Faless commented Sep 9, 2022

My bet is on /DWIN32_LEAN_AND_MEAN. EDIT: seems like I lost my bet :-p

We probably want to test more than just them being able to compile, but
this is a start.
Ensures user inclusion of windows.h do not define "min" and "max"
macros.
@Faless Faless merged commit 024b6d2 into godotengine:master Sep 12, 2022
@Faless Faless deleted the bump/hashfuncs branch September 12, 2022 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug This has been identified as a bug topic:gdextension This relates to the new Godot 4 extension implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants