Should the API use std::unique_ptr ?

[markaren]

This is a question that ought to be asked.

Probably the most correct thing for the API is to use std::unique_ptr to handle ownership in the scenegraph. Thus, ::create should return a std::unique_ptr for objects derived from Object3D. However, as the library is first and foremost targetet at beginners, std::shared_ptr is easier to work with. std::unique_ptr, while more correct, makes the usage of the API more complicated.

Any outside views?

And, yes, I have a PR standby.

[wol101]

std::unique_ptr makes knowing the lifetime of an object much easier and you always know where it is actually owned. The only convincing arguments I've seen for std::shared_ptr use involves multiple threads and termination indeterminancy. My vote would go for std::unique_ptr and I imagine that some of the getter functions would need to return pointers instead. I can see that might make maintenance a bit harder but I'm not sure it actually makes it any/much harder to use.

[markaren]

I can see that might make maintenance a bit harder but I'm not sure it actually makes it any/much harder to use.

The change has the largest impact on user code. Material and BufferGeometry will remain shared, as that is how they are supposed to work. I.e. shared ownership. So it is mainly Object3D::add and Object::create et.al that is affected.

The usability issue is that:

auto mesh = Mesh::create(...);
scene.add(std::move(mesh)); //move is needed

// later in the code

canvas.animate([&]{
    mesh.position.x += dt; // <-- Illegal. Object has been moved from. 
});

so, the code above becomes something akin to:

auto mesh = Mesh::create(...);
auto meshPtr = mesh.get();  // or give mesh a name that is searchable in the scenegraph
scene.add(std::move(mesh)); //move is needed

// later in the code

canvas.animate([&]{
    meshPtr->position.x += dt; 
});

which is much less plesant..

[markaren]

Of course, another way to think about this is to not let the scenegraph take ownership. I.e. the user must ensure the validity of the objects. This is already possible today, as I support both non-owning references and shared pointers to be added.

[wol101]

I don't have particularly strong opinions about this. but the feeling I get is that many C++ people feel that too much reliance on shared pointers can lead to subtle bugs and ownership loops and that unique pointers enforce an ownership discipline. In your examples, if scene owns the object then I would keep a pointer (or get a pointer from scene when I needed it). However I think the most important thing here is that I imagine it is much easier to port from javascript with shared pointers and I'd rather it worked reliably than anything else. I can't imagine there are any important performance issues. I certainly agree that for objects that you might want to share between many scenes a shared pointer seems like a good fit. And if I want a unique_ptr then I can easily create it myself.