Is this initialized somewhere else (such as by a calloc)?

Probably better to stick with NodeUpdateGS to follow the Law of Demeter: http://en.wikipedia.org/wiki/Law_of_Demeter

Perhaps we should leave the old code as a guide? I suppose version control can allow someone to dig up the old code.

Why get rid of the comment?

Can we just set Bullet to be 1, and not bother with this comment?

Why change this logic?

Was not setting parentUpdated and the worldScale causing any problems or bugs?

I don't like seeing behavior defined in a header file. I know it can be tedious to define the behavior of a one line method in the cpp file, but it keeps things cleaner. I don't know if the Blender style guide has anything to say about this.

Why was the case with a parent removed?

Looks like part of this comment is now missing. Was that intentional?

addInitFromFrame was removed

UpdateWorldData needs time elapsed=0, and parentUpdated=true (second parameter) that will go all the way to
https://developer.blender.org/diffusion/B/browse/soc-2014-bge/source/gameengine/Ketsji/KX_SG_NodeRelationships.cpp$50
as true, meaning that 'the parent of the root node is updated' -> meaning a force calculate world coordinates / the world has changed. Makes sense for the initial computation.

I think it is clear for the true to be passed explicitly in this case. The default is false.
Maybe it shouldn't be?

I can put the old cold back in with #if 0

I didn't think it was adding anything

I think it must be 5 for backwards compatibility

I just reordered it in a clear way, consistent for all updateCoordinates implementations

I don't know what you are talking about here

In this case, it is just getter/setters

RelativeTranslate and Rotate, according to the documentation, should transform either in local coordinates, or in world coordinates. Never the parent's.
I emailed about this to bf-committers

yes, parent was removed from the function signature.
That part of the comment is relative to the parent

Ok.

I wonder then if NodeUpdateGS should take an optional parameter for parentUpdated that defaults to false. That should avoid breaking anything, and allow you to still have it set to true for this case.

Is this a function that is even used anymore? If we can safely remove the contents of the function, can we not just remove the function entirely? If the function is still useful, then perhaps you should rewrite the functionality before merging to master.

Whatever we decide to do with this function, we should be consistent with KX_BlenderSceneConverter::ResetPhysicsObjectsAnimationIpo, as that function appears to suffer from the same problem.

Ok.

If something breaks by changing the numbers in an enum, we either need to fix that, or add a comment to warn others about changing the enum.

Ok.

It looks like the old code does not set worldScale or parentUpdated. Your new code does. Does this change any current behavior, such as fixing any bugs?

I understand that they are just getters/setters, I still don't really like behavior being defined in the header file. I don't mind it staying if some others say it is ok, since I don't think this is really covered in the style guide.

Sorry, I must have missed that email. I'll go look it up and read through it.

Ok.

I can do that

Yes, both functions have the same problem, I talked about this with campbell at the time. The functions are supposed to be used, but both are doing nothing since a very long time.
He said that they need to be written again (not removed), and that the code was with #if 0 to serve as a basis to the next person, so I didn't touch it, but had already removed the contents of the second function on a previous commit.
I removed it when I removed addInitFromFrame, maybe it doesn't even serve as basis?
At the moment I don't know enough about IPO's to rewrite this, but I can take a look

Not reported ones.
They are correct changes, a vertex child inherits only the translation, but it still should apply it's local scale and orientation to itself. I didn't bother testing, but if the scale of a child was changed after the game conversion (that set's the world coordinates with normal parents before changing the type of parenting), the child would never change size.
And the children of a vertex child would likely not be updated in some cases (because updateParent wasn't being set to true, it is keeping the previous value), in some other cases the children would be recomputed with no need.

I see this kind of thing everywhere, I didn't put it there, just reformatted. I can, off course, move it to the cpp.
Do you want to ask someone or I just move it?

here it is: http://lists.blender.org/pipermail/bf-committers/2014-July/043923.html
I have summed up the remaining questions in the scenegraph documentation page http://wiki.blender.org/index.php/Dev:Source/GameEngine/SceneGraph

Checking this again:
The UpdateWorldData function is from SG_Node
https://developer.blender.org/diffusion/B/browse/soc-2014-bge/source/gameengine/SceneGraph/SG_Node.cpp;3f79ceb0eab3c9610e2fc688152291f0b443af5b$171
and used in several other places such as KX_Scene and the videotexture module.
The NodeUpdateGS is from KX_GameObject.
I don't think it hurts to call this function from the DataConversion as it knows about SG_Nodes.. it makes them.
However, this is a minor thing, I totally don't mind changing this to NodeUpdateGS and changing that to accept an optional parameter defaulting to false

I took another look at this, I still don't know enough to rewrite it, and there is lot of other similar functions.
I'm adding back what I removed as a comment for whomever does it

The world coordinates are initialized for all the scenegraph in
BL_ConvertBlenderObjects()
just bellow the comment that says:
/* Find all 'root' parents (objects that have no parents in SceneGraph) and init the world transforms */

So, there is no need to do it here.
I added a comment, 10 lines above here, saying:
/* Note: world coordinates are calculated for all nodes when the scene graph

• is complete, after processing vec_parent_child */

Why UpdateWorldData instead of NodeUpdateGS ?

@Inês Almeida (brita_) : Did you test this part, to avoid bugs ?

Why remove this comment ?