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

benoit.bolsee at online.be benoit.bolsee at online.be
Tue Jul 26 23:07:50 CEST 2011


more comments. I haven't checked the do_version() function yet.


http://codereview.appspot.com/4289041/diff/50001/source/blender/blenkernel/intern/material.c
File source/blender/blenkernel/intern/material.c (right):

http://codereview.appspot.com/4289041/diff/50001/source/blender/blenkernel/intern/material.c#newcode560
source/blender/blenkernel/intern/material.c:560: memmove((*matar),
(*matar) + 1, sizeof(void *) * ((*totcol) - (index + 1)));
Although this code is not part of the patch, it looks enormously like a
bug. Correct code should be
memmove((*matar)+index, (*matar)+(index+1), sizeof(void
*)*((*totcol)-(index+1)));

http://codereview.appspot.com/4289041/diff/50001/source/blender/blenkernel/intern/material.c#newcode1542
source/blender/blenkernel/intern/material.c:1542: flag &= ~TF_ALPHA <<
15;
The correct expression is ~(TF_ALPHA << 15)
Always put paranthesis around shift operator as the precedence order is
often misleading.

http://codereview.appspot.com/4289041/diff/50001/source/blender/blenkernel/intern/material.c#newcode1686
source/blender/blenkernel/intern/material.c:1686:
an optimization is possible here: if the MTFace mode don't change the
material settings, no need to create a new one.

http://codereview.appspot.com/4289041/diff/50001/source/blender/blenkernel/intern/material.c#newcode1716
source/blender/blenkernel/intern/material.c:1716: //			if(me->mat[a] ==
ma) material_pop_id(&me->id, a);
Two problem:
1. You must break after material_pop_id (a material can only be present
once afaik)
2. Doesn't this screw up the material number for the mface?

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


More information about the Bf-codereview mailing list