Page MenuHome

BGE : KX_VertexProxy support for more than 2 UV channel.
AbandonedPublic

Authored by Porteries Tristan (panzergame) on Apr 15 2015, 7:15 PM.

Details

Summary

I have added an optional named "index" argument for methode get/setUV, I have also modified the and set to deprecated methodes setUV2 and getUV2 : the doc was wrong and the methode can't be called anyway because it declared as VARARG in the .h and convert directly the args value to a vector in the .cpp.

Diff Detail

Repository
rB Blender
Branch
ge_multi_uv_channel

Event Timeline

Porteries Tristan (panzergame) retitled this revision from to BGE : KX_VertexProxy support for more than 2 UV channel..Apr 15 2015, 7:15 PM
Porteries Tristan (panzergame) updated this object.
Porteries Tristan (panzergame) updated this revision to Diff 3982.
source/gameengine/Converter/BL_BlenderDataConversion.cpp
518 ↗(On Diff #3982)

Why you removed the UV check?
If you remove this check the other texture coordinates ( environment, reflection, object, normal, orco and tangent maps) will be also converted.
I don't know if this will cause problems.
Did you check all possible setting? Did you also checked the other settings it in combination with UV scroll and the video texture?

source/gameengine/Converter/BL_BlenderDataConversion.cpp
518 ↗(On Diff #3982)

Without this we can convert other UV layer, and methode getUV(index) with an index of an unused UV don't return always (0.0, 0.0, 0.0).

I think that is just an optimization.

As you mentioned get/setUV() will using the wrong UV map if a material using multiple textures and one of the textures are disabled. But this bug was also before. If you want to fix that you should do this in a separate patch.

SetUV2() was broken since blender 2.66a, In my opinion we can remove it, because we only can break compatibility to older files then blender 2.66. If the others are against removing the old API, you need to return a deprecated message and change the documentation too.

source/gameengine/Converter/BL_BlenderDataConversion.cpp
518 ↗(On Diff #3982)

No the cheek is necessary. If you disable a texture in between or set the texture to an other type as UV, then the previous UV map will be used.
I think that was a fix in Blender 2.66. See my UV-Scroll test file.

I have tested getUV(index) and setUV(uv, index) and it is working. So this change is not necessary to get it working.

Porteries Tristan (panzergame) updated this revision to Diff 4157.

Add deprecated messages.

inline comments

doc/python_api/rst/bge_types/bge.types.KX_VertexProxy.rst
129

would it be better getUV(index=0) as it seems that index = 0 by default?

138

same as before, would it be better setUV(uv, index=0) as it seems that index = 0 by default?

Porteries Tristan (panzergame) updated this revision to Diff 4163.

Fix keyarguments in the doc.

Thomas Szepe (hg1) accepted this revision.

It looks good for me.

This revision is now accepted and ready to land.May 6 2015, 6:54 PM

Inline comments

doc/python_api/rst/bge_types/bge.types.KX_VertexProxy.rst
30

The list of all -> A list of all the

source/gameengine/Ketsji/KX_VertexProxy.cpp
426

Why remove this, but still use it inside KX_VertexProxy::PySetUV?

Jorge Bernal (lordloki) accepted this revision.

Other than the little inline comments, it looks ok to me.

source/gameengine/Ketsji/KX_VertexProxy.cpp
615

it should be "getUV(1)"

626

Same as before, it should be "setUV(uv, 1)"

source/gameengine/Ketsji/KX_VertexProxy.cpp
426

It's a redundant call see line 435, and it will say that if the list is wrong a part of it is apply, i think that is a wrong behaviour.

source/gameengine/Ketsji/KX_VertexProxy.cpp
426

Looks good, didn't see other code block.

source/gameengine/Ketsji/KX_VertexProxy.cpp
426

it is repeated at line 435 (out of the for loop)

Porteries Tristan (panzergame) updated this revision to Diff 4166.

"The list of…" -> "A list of…"

This revision was automatically updated to reflect the committed changes.

Why were neither @Campbell Barton (campbellbarton) nor myself added as reviewers to this patch? KX_VertexProxy.get/setUV(2) were supposed to be deprecated. I had talked with @Porteries Tristan (panzergame) about this previously on IRC, and I would much rather see issues with KX_VertexProxy.uvs fixed (as opposed to bringing back deprecated features).

Why were neither @Campbell Barton (campbellbarton) nor myself added as reviewers to this patch?

It was our mistake. I will go over all BGE open differentials to add you/campbell where you may become necessary.

This revision is now accepted and ready to land.Jun 18 2015, 10:38 AM
  • Try to implement CListPython for member variable "uvs".
  • temporary fix for python double free
  • Support uv assignement for uvs list.
  • Add GetReplicate function to use python generator.
  • Create the pŷthon generator owned by python, not BGE.
  • Leave function GetUV1/2 but just set callable SetUV2.
  • Update doc.
  • Set dprecated message and warning.
This revision now requires review to proceed.Jun 19 2015, 11:03 AM

Why you don't use CListValue instead of adding a new CListPython class.

source/gameengine/Expressions/ListPython.h
47–48

virtual CValue *Calc
virtual CValue *CalcFinal

50

CValue *val

83–85

Space in front of = is missing.

Because CListValue use CValue type for elemtents and we will spend to much time to convert a MT_Point2 in a CValue type.