Skip to content
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

Adding Motion Matching Library #29892

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

Aa20475
Copy link

@Aa20475 Aa20475 commented Jun 19, 2019

Basic Structure of Node and tree made

TODO: Build tree

#include "core/io/resource_loader.h"

bool AnimationNodeMotionMatchEditor::can_edit(
const Ref<AnimationNode> &p_node) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a note, in the cpp files, we use <tab> instead of <space> for indentation.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually set up the pre-commit-hook too. Will recheck it..

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice.

@fire
Copy link
Member

fire commented Jul 4, 2019

Remember that the red crosses are the travis and appveyor ci checks failing. These are test compiles of the project.

@Aa20475
Copy link
Author

Aa20475 commented Jul 10, 2019

Remember that the red crosses are the travis and appveyor ci checks failing. These are test compiles of the project.

Ohh! I really am not sure why they fail... :(
Help me with it please.....

@fire
Copy link
Member

fire commented Jul 10, 2019

  1. Clang format.

  2. Here's an error log.

modules/motionmatch/animation_motion_match_editor.cpp: In member function 'void AnimationNodeMotionMatchEditor::_update_tracks()':
modules/motionmatch/animation_motion_match_editor.cpp:267:44: error: passing NULL to non-pointer argument 1 of 'Variant::Variant(int64_t)' [-Werror=conversion-null]
  267 |                           .get("location", NULL));
      |                                            ^~~~
In file included from ./core/method_ptrcall.h:36,
                 from ./core/method_bind.h:35,
                 from ./core/class_db.h:34,
                 from ./scene/main/node.h:34,
                 from ./editor/audio_stream_preview.h:35,
                 from ./editor/editor_node.h:35,
                 from ./editor/plugins/animation_tree_editor_plugin.h:34,
                 from modules/motionmatch/animation_motion_match_editor.h:6,
                 from modules/motionmatch/animation_motion_match_editor.cpp:1:
./core/variant.h:251:18: note:   declared here
  251 |  Variant(int64_t p_int); // real one
      |          ~~~~~~~~^~~~~
g++-9 -o modules/motionmatch/animation_node_motion_match.x11.tools.64.o -c -Wctor-dtor-privacy -Wnon-virtual-dtor -Wnoexcept -Wplacement-new=1 -g3 -pipe -fpie -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wshadow-local -Walloc-zero -Wduplicated-branches -Wduplicated-cond -Wstringop-overflow=4 -Wlogical-op -Wattribute-alias=2 -Werror -DDEBUG_ENABLED -DDEBUG_MEMORY_ENABLED -DTOUCH_ENABLED -DALSA_ENABLED -DALSAMIDI_ENABLED -DJOYDEV_ENABLED -DX11_ENABLED -DUNIX_ENABLED -DOPENGL_ENABLED -DGLES_ENABLED -DZSTD_STATIC_LINKING_ONLY -DGLAD_ENABLED -DGLES_OVER_GL -DMODULE_ASSIMP_ENABLED -DMODULE_BMP_ENABLED -DMODULE_BULLET_ENABLED -DMODULE_CSG_ENABLED -DMODULE_CVTT_ENABLED -DMODULE_DDS_ENABLED -DMODULE_ENET_ENABLED -DMODULE_ETC_ENABLED -DMODULE_FREETYPE_ENABLED -DMODULE_GDNATIVE_ENABLED -DMODULE_GDSCRIPT_ENABLED -DMODULE_GRIDMAP_ENABLED -DMODULE_HDR_ENABLED -DMODULE_JPG_ENABLED -DMODULE_MBEDTLS_ENABLED -DMODULE_MOBILE_VR_ENABLED -DMODULE_MONO_ENABLED -DMODULE_MOTIONMATCH_ENABLED -DDEBUG_MEMORY_ALLOC -DDISABLE_FORCED_INLINE -DPTRCALL_ENABLED -DTOOLS_ENABLED -DGDSCRIPT_ENABLED -DMINIZIP_ENABLED -Ithirdparty/libpng -Ithirdparty/glad -Ithirdparty/zstd -Ithirdparty/zlib -Iplatform/x11 -I. -Ieditor modules/motionmatch/animation_node_motion_match.cpp
cc1plus: all warnings being treated as errors
scons: *** [modules/motionmatch/animation_motion_match_editor.x11.tools.64.o] Error 1
modules/motionmatch/animation_node_motion_match.cpp: In member function 'void AnimationNodeMotionMatch::add_coordinates(PoolRealArray)':
modules/motionmatch/animation_node_motion_match.cpp:112:20: error: comparison of integer expressions of different signedness: 'int' and 'uint32_t' {aka 'unsigned int'} [-Werror=sign-compare]
  112 |   if (point.size() != dim_len) {
      |       ~~~~~~~~~~~~~^~~~~~~~~~
modules/motionmatch/animation_node_motion_match.cpp: In member function 'void AnimationNodeMotionMatch::load_coordinates(PoolRealArray)':
modules/motionmatch/animation_node_motion_match.cpp:130:23: error: comparison of integer expressions of different signedness: 'int' and 'uint32_t' {aka 'unsigned int'} [-Werror=sign-compare]
  130 |     for (int i = 0; i < points.size() / dim_len; i++) {
      |                     ~~^~~~~~~~~~~~~~~~~~~~~~~~~
modules/motionmatch/animation_node_motion_match.cpp:132:25: error: comparison of integer expressions of different signedness: 'int' and 'uint32_t' {aka 'unsigned int'} [-Werror=sign-compare]
  132 |       for (int j = 0; j < dim_len; j++) {
      |                       ~~^~~~~~~~~
modules/motionmatch/animation_node_motion_match.cpp: In member function 'void AnimationNodeMotionMatch::clear_root()':
modules/motionmatch/animation_node_motion_match.cpp:162:16: error: ambiguous overload for 'operator=' (operand types are 'Ref<AnimationNodeMotionMatch::KDNode>' and 'std::nullptr_t')
  162 |   root->left = nullptr;
      |                ^~~~~~~
In file included from ./core/io/stream_peer.h:34,
                 from ./core/io/packet_peer.h:34,
                 from ./core/io/networked_multiplayer_peer.h:34,
                 from ./core/io/multiplayer_api.h:34,
                 from ./core/script_language.h:34,
                 from ./scene/main/node.h:39,
                 from ./scene/3d/spatial.h:34,
                 from ./scene/3d/collision_object.h:34,
                 from ./scene/3d/physics_body.h:35,
                 from modules/motionmatch/animation_node_motion_match.h:4,
                 from modules/motionmatch/animation_node_motion_match.cpp:1:
./core/reference.h:152:7: note: candidate: 'void Ref<T>::operator=(const Ref<T>&) [with T = AnimationNodeMotionMatch::KDNode]'
  152 |  void operator=(const Ref &p_from) {
      |       ^~~~~~~~
./core/reference.h:185:7: note: candidate: 'void Ref<T>::operator=(const Variant&) [with T = AnimationNodeMotionMatch::KDNode]'
  185 |  void operator=(const Variant &p_variant) {
      |       ^~~~~~~~
modules/motionmatch/animation_node_motion_match.cpp:163:17: error: ambiguous overload for 'operator=' (operand types are 'Ref<AnimationNodeMotionMatch::KDNode>' and 'std::nullptr_t')
  163 |   root->right = nullptr;
      |                 ^~~~~~~
In file included from ./core/io/stream_peer.h:34,
                 from ./core/io/packet_peer.h:34,
                 from ./core/io/networked_multiplayer_peer.h:34,
                 from ./core/io/multiplayer_api.h:34,
                 from ./core/script_language.h:34,
                 from ./scene/main/node.h:39,
                 from ./scene/3d/spatial.h:34,
                 from ./scene/3d/collision_object.h:34,
                 from ./scene/3d/physics_body.h:35,
                 from modules/motionmatch/animation_node_motion_match.h:4,
                 from modules/motionmatch/animation_node_motion_match.cpp:1:
./core/reference.h:152:7: note: candidate: 'void Ref<T>::operator=(const Ref<T>&) [with T = AnimationNodeMotionMatch::KDNode]'
  152 |  void operator=(const Ref &p_from) {
      |       ^~~~~~~~
./core/reference.h:185:7: note: candidate: 'void Ref<T>::operator=(const Variant&) [with T = AnimationNodeMotionMatch::KDNode]'
  185 |  void operator=(const Variant &p_variant) {
      |       ^~~~~~~~
modules/motionmatch/animation_node_motion_match.cpp: In member function 'void AnimationNodeMotionMatch::build_tree()':
modules/motionmatch/animation_node_motion_match.cpp:194:18: error: suggest parentheses around assignment used as truth value [-Werror=parentheses]
  194 |     else if (err = K_ERROR)
      |              ~~~~^~~~~~~~~
modules/motionmatch/animation_node_motion_match.cpp: In member function 'PoolRealArray AnimationNodeMotionMatch::KNNSearch(PoolRealArray, uint32_t)':
modules/motionmatch/animation_node_motion_match.cpp:237:18: error: suggest parentheses around assignment used as truth value [-Werror=parentheses]
  237 |     else if (err = K_ERROR)
      |              ~~~~^~~~~~~~~
modules/motionmatch/animation_node_motion_match.cpp:247:39: error: comparison of integer expressions of different signedness: 'int' and 'uint32_t' {aka 'unsigned int'} [-Werror=sign-compare]
  247 |     while (node->get_indices().size() > min_leaves) {
      |            ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~
modules/motionmatch/animation_node_motion_match.cpp:255:23: error: comparison of integer expressions of different signedness: 'int' and 'uint32_t' {aka 'unsigned int'} [-Werror=sign-compare]
  255 |     for (int j = 0; j < k; j++) {
      |                     ~~^~~
modules/motionmatch/animation_node_motion_match.cpp: In member function 'bool AnimationNodeMotionMatch::KDNode::are_all_points_same(PoolRealArray, uint32_t)':
modules/motionmatch/animation_node_motion_match.cpp:324:21: error: comparison of integer expressions of different signedness: 'int' and 'uint32_t' {aka 'unsigned int'} [-Werror=sign-compare]
  324 |   for (int i = 0; i < dim_len; i++) {
      |                   ~~^~~~~~~~~
modules/motionmatch/animation_node_motion_match.cpp: In member function 'void AnimationNodeMotionMatch::KDNode::leaf_split(PoolRealArray, uint32_t, uint32_t, uint32_t)':
modules/motionmatch/animation_node_motion_match.cpp:383:36: error: comparison of integer expressions of different signedness: 'int' and 'uint32_t' {aka 'unsigned int'} [-Werror=sign-compare]
  383 |     if (left->point_indices.size() > min_leaves &&
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~
modules/motionmatch/animation_node_motion_match.cpp:387:37: error: comparison of integer expressions of different signedness: 'int' and 'uint32_t' {aka 'unsigned int'} [-Werror=sign-compare]
  387 |     if (right->point_indices.size() > min_leaves &&
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~
cc1plus: all warnings being treated as errors
scons: *** [modules/motionmatch/animation_node_motion_match.x11.tools.64.o] Error 1
scons: building terminated because of errors.
The command "if [ "$STATIC_CHECKS" = "yes" ]; then sh ./misc/travis/clang-format.sh; else scons -j2 CC=$CC CXX=$CXX platform=$PLATFORM tools=$TOOLS target=$TARGET $OPTIONS $EXTRA_ARGS; fi" exited with 2.```

@fire
Copy link
Member

fire commented Sep 28, 2019

You can use clang-format -i *.cpp and clang-format -i *.h to format your source files to the Godot style standard.

Rebasing in git allows you to put the branch on top of the current git 3.2 master.

It also allows you to squash multiple commits together for clarity.

@Aa20475
Copy link
Author

Aa20475 commented Sep 28, 2019

Okay thanks!

@akien-mga akien-mga modified the milestones: 3.2, 4.0 Nov 7, 2019
@aaronfranke
Copy link
Member

@Aa20475 Is this still desired? If so, it needs to be rebased on the latest master branch, and squashed, as per this article. Also, since this PR is a feature proposal, you should consider opening a proposal which explains example use cases and how this approach will solve the problem.

Otherwise, abandoned pull requests will be closed in the future as announced here.

@Aa20475
Copy link
Author

Aa20475 commented May 26, 2020

Okay will do it asap. I'd still love to work on this PR. ✌️

@fire
Copy link
Member

fire commented May 26, 2020

#32408 is my rebased pr, feel free to take it.

@Aa20475
Copy link
Author

Aa20475 commented May 26, 2020

That's cool! Can you give me a short desc of the changes you made , so that i can catch up?

@fire
Copy link
Member

fire commented May 26, 2020

  1. Fixed compiler errors.
  2. Was not able to correct the cost and trajectory predictions.
  3. Changed some code from whatever to int. I believe int64 is the default for size now. Not sure.
  4. CICD errors are fixed then, but I don't have any test cases
  5. CICD errors on master

Note: Edited

@Aa20475
Copy link
Author

Aa20475 commented May 27, 2020

Ohh okay! That's cool!

I've been exploring about it too. We still need to speed up the kdtrees knnsearch in some way. I thought we can implement it to run of GPU.

There is a more complex and sophisticated algorithm which adds reinforcement learning to this which is the actual algorithm in the paper that proposed motion fields.

But I thought speeding up the KNNsearch takes the first priority.

Also, any luck with running it during game preview?

@fire
Copy link
Member

fire commented May 27, 2020

Can you create a test project?

@Aa20475
Copy link
Author

Aa20475 commented Sep 10, 2020

I thought I had it somewhere on my Hard disk. I'll find it or make a new one asap!

@Aa20475
Copy link
Author

Aa20475 commented Sep 10, 2020

Thanks a lot, @fire for doing the changes. I will make a test project quick and let you know.

@fire
Copy link
Member

fire commented Nov 14, 2020

Can you post the design document for the GSOC, so I can make a proposal for this?

@aaronfranke
Copy link
Member

@Aa20475 This PR doesn't build, please fix it and squash the commits. See this article for more information.

@fire
Copy link
Member

fire commented Feb 12, 2024

This is an incomplete motion matching implementation. A more recent version is from https://github.com/Remi123/MotionMatching. I would like to archive this pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants