[Bf-codereview] Pepper to trunk merge (issue 4934047)

mogurijin at gmail.com mogurijin at gmail.com
Fri Aug 26 21:49:56 CEST 2011


Here are my responses to the game engine comments so far. Also, would
there be any objections to me adding Benoit as a "reviewer" for the BGE
animations part of the patch?


http://codereview.appspot.com/4934047/diff/6043/source/gameengine/Ketsji/BL_ActionManager.cpp
File source/gameengine/Ketsji/BL_ActionManager.cpp (right):

http://codereview.appspot.com/4934047/diff/6043/source/gameengine/Ketsji/BL_ActionManager.cpp#newcode46
source/gameengine/Ketsji/BL_ActionManager.cpp:46: if (m_layers[layer])
Initially not every layer was filled like it is now. The NULL checks
came from then and weren't removed when the code changed. They can be
removed.

On 2011/08/24 14:28:30, brechtvl wrote:
> The "if (m_layers[layer])" checks seem a bit inconsistent, they are
done in some
> functions and not in others, but it seems that these pointers are
never null.

http://codereview.appspot.com/4934047/diff/6043/source/gameengine/Ketsji/CMakeLists.txt
File source/gameengine/Ketsji/CMakeLists.txt (right):

http://codereview.appspot.com/4934047/diff/6043/source/gameengine/Ketsji/CMakeLists.txt#newcode78
source/gameengine/Ketsji/CMakeLists.txt:78: ../../../source/blender/gpu
If you do an SVN blame, it looks like the result of a messed up merge. I
think the paths were cleaned up a bit in trunk, and the "new" paths
shown are the old versions.

On 2011/08/26 13:00:20, brechtvl wrote:
> Why were all these include paths added here, they mostly duplicate
existing
> ones?

http://codereview.appspot.com/4934047/diff/6043/source/gameengine/Ketsji/KX_KetsjiEngine.cpp
File source/gameengine/Ketsji/KX_KetsjiEngine.cpp (right):

http://codereview.appspot.com/4934047/diff/6043/source/gameengine/Ketsji/KX_KetsjiEngine.cpp#newcode1011
source/gameengine/Ketsji/KX_KetsjiEngine.cpp:1011: // nothing to do
here, everything relative now...
This is a change from neXyon, but I'm going to guess it can be removed
now. Should DoSound() also be removed from KX_KetsjiEngine then?

On 2011/08/26 13:00:20, brechtvl wrote:
> I guess this code can just be removed then?

http://codereview.appspot.com/4934047/


More information about the Bf-codereview mailing list