[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