Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

What is the expected behavior when passing a Data as argument in the object behavior ? #197

Open
damienmarchal opened this issue Sep 22, 2021 · 4 comments

Comments

@damienmarchal
Copy link
Contributor

What should be the behavior when passing other data field as argument during object creation.

Example:

def createScene(root):
    root.addObject("MeshObjLoader", filename="mesh/cube.obj", name="loader");
    a = root.addObject("PointSetTopologyContainer", position=root.loader.position, name="topo") 
    b = root.addObject("PointSetTopologyContainer", position=root.loader.position.value, name="topo") 

To me a should make a link, while b should copy the value. If you share this few then ... the current implementation need to be fixed

@jnbrunet
Copy link
Collaborator

I also share your view @damienmarchal. It feels natural like this. When you pass an object or data as a parameter, you pass a "reference" to it. When you pass a value, you make a copy. +1 for me

@damienmarchal
Copy link
Contributor Author

damienmarchal commented Sep 23, 2021

Thanks @jnbrunet for the feedback.

Yesterday I made several tests to see the look and feel and made

def createScene(root):
    root.addObject("MeshObjLoader", filename="mesh/cube.obj", name="loader");
    a = root.addObject("PointSetTopologyContainer", position=root.loader.position.linkpath, name="topo") 
    b = root.addObject("PointSetTopologyContainer", position=root.loader.position.value, name="topo") 

So there is an explicit version for the two cases.
And for the implicit one:

    a = root.addObject("PointSetTopologyContainer", position=root.loader.position, name="topo") 

We need to decide if we continue to copy values (what we are doing right now) or if we make a link (more breaking ?).
With link-by-default, the performances will be very good but users has to understand what a link and fallback to value copy-value if this cause issues in their scenes .
with copy-by-default approach the performances will be bad and user will have to op-in for the fastest (link-base) version.

@damienmarchal
Copy link
Contributor Author

damienmarchal commented Sep 23, 2021

Another alternative could be to first deprecate (with a warning message) the existing implicit version then replace it with an exception saying that this ambiugous and force to replace it with one of the explicit ones.

a = root.addObject("PointSetTopologyContainer", position=root.loader.position.linkpath, name="topo") 
b = root.addObject("PointSetTopologyContainer", position=root.loader.position.value, name="topo") 

@jnbrunet
Copy link
Collaborator

Hey @damienmarchal

I'm curious in which cases the user would prefer a copy against a (COW anyway?) link. I have a feeling that a link would be more appropriate for most needs, hence creating less frustrations for the user, but I may be wrong.

From experience, people will naturally set the attribute to the source data directly, i.e.:

node.addObject("MechanicalObject", position=node.loader.position)

And forget about it. It is also much simpler to read. But it does come with an implicit decision made for the user.

A warning here would, I think, generate a "What the f. is that? Which one should I use? I just want this component positions..." In the user's head.

I am usually in favor of explicit against implicit, but here I think it is a good case of simplicity outweighs the cost of implicity (the cost which I think is minimal anyway...) .

My 2 cents! Whatever you choose, it will be a great improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants