[Bf-codereview] BGE resize callback (issue4639043)

[mogurijin at gmail.com]

On 2011/06/17 17:24:40, dfelinto wrote:
> Hi Mitchell,

> I never implemented callbacks, but it seems like you did the right way
(assuming
> GHOST resize event is really not reliable).

> I added some code syntax comments along the patch.
> (adding bf-codereview as CC)


https://codereview.appspot.com/4639043/diff/1/source/gameengine/Ketsji/KX_KetsjiEngine.cpp
> File source/gameengine/Ketsji/KX_KetsjiEngine.cpp (right):


https://codereview.appspot.com/4639043/diff/1/source/gameengine/Ketsji/KX_KetsjiEngine.cpp#newcode111
> source/gameengine/Ketsji/KX_KetsjiEngine.cpp:111: * ResizeCallback
functions
> use a tab between instead of space here (doxygen style along the rest
of the
> file)


https://codereview.appspot.com/4639043/diff/1/source/gameengine/Ketsji/KX_KetsjiEngine.cpp#newcode209
> source/gameengine/Ketsji/KX_KetsjiEngine.cpp:209: m_prevWidth(0),
> I would not include "prev" in the variable name (i.e. call it m_width
and
> m_height or m_viewWidth, m_windowsWidth, m_canvasWidth. ...)


https://codereview.appspot.com/4639043/diff/1/source/gameengine/Ketsji/KX_KetsjiEngine.cpp#newcode239
> source/gameengine/Ketsji/KX_KetsjiEngine.cpp:239:
> don't you also need to clear m_pyfunc?


https://codereview.appspot.com/4639043/diff/1/source/gameengine/Ketsji/KX_Scene.cpp
> File source/gameengine/Ketsji/KX_Scene.cpp (right):


https://codereview.appspot.com/4639043/diff/1/source/gameengine/Ketsji/KX_Scene.cpp#newcode129
> source/gameengine/Ketsji/KX_Scene.cpp:129: // This callback is used to
update
> the 2D filter manager
> I don't think we need this comment here.

The m_pyfunc reference is dealt with in ResizeCallback's destructor, so
it doesn't need to be handled in KX_KetsjiEngine's destructor. As for
the comment in KX_Scene, I added it because it was a 2d filter callback
placed in KX_Scene. Since this seemed a touch out of place, I decided to
add a comment. I'll create a new patch addressing some of the other
comments.

https://codereview.appspot.com/4639043/