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

mogurijin at gmail.com mogurijin at gmail.com
Sun Aug 28 10:39:37 CEST 2011


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

http://codereview.appspot.com/4934047/diff/6043/source/gameengine/Converter/BL_ActionActuator.cpp#newcode203
source/gameengine/Converter/BL_ActionActuator.cpp:203: if (m_playtype ==
ACT_ACTION_PLAY)
This isn't set every frame, just every time a new action is played.

On 2011/08/27 09:19:57, dfelinto wrote:
> I don't see why you need to set this everyframe.
> an option is to have it set in the python mode function (and in the
flipper
> after it's converted to a play actuator) -- let's hope no one is
checking the
> action type from python during this time ;)

http://codereview.appspot.com/4934047/diff/6043/source/gameengine/Converter/BL_ActionActuator.h
File source/gameengine/Converter/BL_ActionActuator.h (right):

http://codereview.appspot.com/4934047/diff/6043/source/gameengine/Converter/BL_ActionActuator.h#newcode152
source/gameengine/Converter/BL_ActionActuator.h:152: ACT_FLAG_PLAY_END	=
1<<5
I thought I had tested this and C++ didn't allow it. However, after
running another test, it seems fine with it (at least msvc). I know I
leave commas like you suggest all the time in Python.

On 2011/08/27 09:19:57, dfelinto wrote:
> I'm not sure C++ allows to, but if you can put a comma in the end of
this line
> you make it easier for new elements to be added later (a patch can
come in
> without changing the previous line, making the svn blame more useful
;)

> -ACT_FLAG_PLAY_END»      = 1<<5
> +ACT_FLAG_PLAY_END»      = 1<<5,

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

http://codereview.appspot.com/4934047/diff/6043/source/gameengine/Converter/BL_ArmatureObject.cpp#newcode147
source/gameengine/Converter/BL_ArmatureObject.cpp:147: short mode =
ACTSTRIPMODE_BLEND;
If you look, the mode variable was already there, it was just moved. The
changes to this function shouldn't have gotten committed as they were
bits of code I was playing with earlier but decided not to use.

On 2011/08/27 09:19:57, dfelinto wrote:
> ? seriously, why bother setting up a "mode" variable if the old code
is removed
> anyways?
> If you think it's relevant to keep the old switch structure for
further
> reference,implementations why don't you comment it out completely, add
some
> notes, and add the actual code (dstweight = 1.0F - srcweight;) outside
the
> switch?

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


More information about the Bf-codereview mailing list