item as kwarg

[Licini]

In SceneObject init, changing the item to be a kwarg. This will be very helpful in handling complicated multi-inheritance situations, in places like Viewer, where this is becoming very tricky.

[Licini]

@tomvanmele @gonzalocasas @chenkasirer Hey guys, I expanded this PR a bit to make a rather big update to clean up how data items are assigned to SceneObjects.

Previously, in SceneObject class __init__, we accept the geometry/datastructrue item as the first positional argument, in form of:

class SceneObject:
    def __init__(self, item, **kwargs):
        self.item=item
        ...

Then its subclasses like MeshObject we add on top:

class MeshObject(SceneObject):
    def __init__(self, mesh, **kwargs):
        super().init(item=mesh, **kwargs)
        self.mesh=mesh
        ...

Meanwhile we also created general sceneobject class for different context like RhinoSceneObject and BlenderSceneObject where their __init__ does not take positional arguments, only keyword arguments:

class RhinoSceneObject(SceneObject):
    def __init__(self, **kwargs):
        super().init(**kwargs)
        ...

At last we need to make specific sceneobject for context such as MeshObject for Rhino, which looks like:

class MeshObject(RhinoSceneObject, BaseMeshObject):
    def __init__(self, mesh, **kwargs):
        super().init(mesh=mesh, **kwargs)
        ...

Especially for the last one which involves multi-inheritance, it becomes very difficult to know how to pass in data item to the super init function, whether as positional or kwargs. And it is very often we encounter error of conflicted args or kwargs while extending further these classes.

So the idea here is to remove all this complexity. We do not use positional arguments at all. And we only catch item as kwarg ONCE at the root SceneObject class. And if we need alias such as MeshObject.mesh, we make it a property that fetches self.item, which is the "single source of truth".

I didn't do it for all, just few of them as a demonstration how it can look like. Please let me know if you guys think this is the good direction, then I will apply the same to all rest of SceneObject classes.

[Licini]

@tomvanmele Ok this one is done and ready. Could you take a look again? I know it's a lot files but all following same pattern.

[tomvanmele]

the revert of using self.vertex_xyz is not correct in my opinion...

[Licini]

the revert of using self.vertex_xyz is not correct in my opinion...

This was because the update_object that is called later inside these function will apply the transformation again. So the object will be transformed twice:

from compas.scene import Scene
from compas.datastructures import Graph

from compas.geometry import Translation


graph = Graph.from_nodes_and_edges([(0, 0, 0), (0, -1.5, 0), (-1, 1, 0), (1, 1, 0)], [(0, 1), (0, 2), (0, 3)])



scene = Scene()

obj = scene.add(graph, nodesize=0.2)
obj.transformation = Translation.from_vector([10, 0, 0])
scene.draw()

See in picture it is moved 20 instead of 10