[Bf-codereview] BGE: Material replacement for texface options (issue4289041)

dfelinto at gmail.com dfelinto at gmail.com
Fri Mar 11 20:58:26 CET 2011


Some of my own comments in the code.
(ps !! this tool is really nice)


http://codereview.appspot.com/4289041/diff/1/source/blender/makesrna/intern/rna_actuator.c
File source/blender/makesrna/intern/rna_actuator.c (right):

http://codereview.appspot.com/4289041/diff/1/source/blender/makesrna/intern/rna_actuator.c#newcode1176
source/blender/makesrna/intern/rna_actuator.c:1176:
RNA_def_property_ui_text(prop, "Distance", "Keep this distance to
target");
Hm the changes on this file have absolute nothing to do with the patch.
I was working on that in parallel and commit it.

http://codereview.appspot.com/4289041/diff/1/source/gameengine/Converter/BL_BlenderDataConversion.cpp
File source/gameengine/Converter/BL_BlenderDataConversion.cpp (right):

http://codereview.appspot.com/4289041/diff/1/source/gameengine/Converter/BL_BlenderDataConversion.cpp#newcode93
source/gameengine/Converter/BL_BlenderDataConversion.cpp:93: #include
"BKE_image.h"
I have no idea when to include "includes" here or in the extern "C"
below.

http://codereview.appspot.com/4289041/diff/1/source/gameengine/Converter/BL_BlenderDataConversion.cpp#newcode659
source/gameengine/Converter/BL_BlenderDataConversion.cpp:659:
material->ras_mode |= (mat && (mat->gameopt.alpha_blend &
GEMAT_ALPHA_SORT))? ZSORT: 0;
unlike in the original code I'm only allowing ZSort for Alpha Blend.

http://codereview.appspot.com/4289041/diff/1/source/gameengine/Converter/BL_BlenderDataConversion.cpp#newcode734
source/gameengine/Converter/BL_BlenderDataConversion.cpp:734:
material->tface		= tface; //XXXDF
we still need to pass on the tface because of the image. I forgot to
remove my comment here. sorry

http://codereview.appspot.com/4289041/diff/1/source/gameengine/Converter/BL_BlenderDataConversion.cpp#newcode905
source/gameengine/Converter/BL_BlenderDataConversion.cpp:905: /**
If I uncomment this line you need to check "use texture face" even in
Texture Face mode. I think it would be good to require that for
consistency sake. However users may find more handy to not have to?
maybe gray'ing out this option when in TextureFace would be enough to
communicate this.

http://codereview.appspot.com/4289041/diff/1/source/gameengine/Rasterizer/RAS_IPolygonMaterial.cpp
File source/gameengine/Rasterizer/RAS_IPolygonMaterial.cpp (right):

http://codereview.appspot.com/4289041/diff/1/source/gameengine/Rasterizer/RAS_IPolygonMaterial.cpp#newcode120
source/gameengine/Rasterizer/RAS_IPolygonMaterial.cpp:120:
ok that was something I forgot to revert. There is no reason to not use
the handy C++ m_prop(prop) syntax.

http://codereview.appspot.com/4289041/


More information about the Bf-codereview mailing list