Remi123 / MotionMatching

Motion Matching for Godot 4.
MIT License
38 stars 3 forks source link

Optimization #12

Closed fire closed 1 year ago

fire commented 1 year ago

Want to make a build for testing since it fails to compile on my mac.

fire commented 1 year ago

Getting some weird errors.

register_types.cpp
src\MMAnimationPlayer.hpp(151): error C2059: syntax error: '{'
src\MMAnimationPlayer.hpp(151): error C2334: unexpected token(s) preceding '{'; skipping apparent function body
src\MMAnimationPlayer.hpp(111): error C2653: 'AnimationMixer': is not a class or namespace name
src\MMAnimationPlayer.hpp(118): error C3861: 'get_root_motion_track': identifier not found
src\MMAnimationPlayer.hpp(124): error C3861: 'get_root_motion_track': identifier not found
src\MMAnimationPlayer.hpp(215): error C3861: 'get_root_motion_track': identifier not found
src\MMAnimationPlayer.hpp(266): error C3861: 'get_root_motion_track': identifier not found
src\MMAnimationPlayer.hpp(342): error C3861: 'get_root_motion_track': identifier not found
scons: *** [src\register_types.windows.template_debug.x86_64.obj] Error 2
scons: building terminated because of errors.
fire commented 1 year ago

tl;dr

  1. Can't use pragma once in cpp files.
  2. 'logf' is not a member of 'std'; did you mean 'logbf'

src/register_types.cpp:1:9: warning: #pragma once in main file
    1 | #pragma once
      |         ^~~~
In file included from boost/boost/include/boost/accumulators/statistics/extended_p_square.hpp:28,
                 from boost/boost/include/boost/accumulators/statistics.hpp:17,
                 from src/MotionMatcher.hpp:36,
                 from src/register_types.cpp:12:
boost/boost/include/boost/accumulators/statistics/times2_iterator.hpp:29:14: warning: 'template<class _Operation> class std::binder1st' is deprecated: use 'std::bind' instead [-Wdeprecated-declarations]
   29 |         std::binder1st<std::multiplies<std::size_t> >
      |              ^~~~~~~~~
In file included from /usr/include/c++/11/bits/stl_function.h:1421,
                 from /usr/include/c++/11/string:48,
                 from godot-cpp/include/godot_cpp/core/method_bind.hpp:43,
                 from godot-cpp/include/godot_cpp/core/class_db.hpp:38,
                 from src/register_types.h:3,
                 from src/register_types.cpp:3:
/usr/include/c++/11/backward/binders.h:108:11: note: declared here
  108 |     class binder1st
      |           ^~~~~~~~~
In file included from src/MMAnimationPlayer.hpp:31,
                 from src/MotionFeatures/BonePositionVelocityMotionFeature.hpp:31,
                 from src/register_types.cpp:15:
src/KForm.hpp: In static member function 'static godot::Vector3 kform::_log(godot::Vector3)':
src/KForm.hpp:159:29: error: 'logf' is not a member of 'std'; did you mean 'logbf'?
  159 |         return Vector3(std::logf(v.x),std::logf(v.y),std::logf(v.z));
      |                             ^~~~
      |                             logbf
src/KForm.hpp:159:44: error: 'logf' is not a member of 'std'; did you mean 'logbf'?
  159 |         return Vector3(std::logf(v.x),std::logf(v.y),std::logf(v.z));
      |                                            ^~~~
      |                                            logbf
src/KForm.hpp:159:59: error: 'logf' is not a member of 'std'; did you mean 'logbf'?
  159 |         return Vector3(std::logf(v.x),std::logf(v.y),std::logf(v.z));
      |                                                           ^~~~
      |                                                           logbf```
Remi123 commented 1 year ago

About the error : The Optimization branch is now on godot 4.2 beta, due to the change to animation player with tokage's big refactor. AnimationMixer is now the parent class of animation player, which remove half my properties to obtain the root bone so for me it's a bonus.

However this create trouble with CICD. Godot_Cpp doesn't include a 4.2 branch as of yet, so we can't build it in CICD because it needs to generate functions from extension_api.json and gdextension_interface.h in version 4.2.

With Godot4.2beta you can generate them with godot --dump-extension-api --dump-gdextension-interface, and place the resulting files in C:\YourPathTo\MotionMatching\godot-cpp\gdextension. Then you build and it should works as expected.

Moving forward, the minimum version supported will be 4.2, since it expose get_root_motion_track.

fire commented 1 year ago

As far as I know godot-cpp tracks master, so it should be 4.2. That's odd.

Remi123 commented 1 year ago

Ok leaving a comment before going to be : Mac has some trouble with size_t to Variant in a error message. Send a fix but don't know if it's the only error yet.

Linux is compiled on ubuntu 20.04 which has Gcc-9.0, but std::reduce is only implemented in gcc-9.3.

Will fix soon

Update : Reduce in ubuntu's g++ doesn't like multitype. So I'm using accumulate which is what I'm doing in the end

fire commented 1 year ago

What's your thoughts on squash and force push? I do this for godot engine commits.

fire commented 1 year ago

Let me know when it's ok to merge?

fire commented 1 year ago

See https://github.com/Remi123/MotionMatching/commit/c3494ed9089cf3b12635877a330c61d89432f3da with removing exceptions in kdtree

Note that I missed a clause:

  if (k < 1) { 
  if (point.size() != dimension) // Missed this clause
    return;
    throw std::invalid_argument(
    // throw std::invalid_argument(
        "kdtree::k_nearest_neighbors(): point must be of same dimension as "
    //     "kdtree::k_nearest_neighbors(): point must be of same dimension as "
        "kdtree");
GeorgeS2019 commented 1 year ago

Thx for pushing hard .the commit titles say them all...thx.both of you