@Ivorforce What is the harm in adding the API before removing the copy constructor? It would be nice to have both existing in the codebase so that third-party modules can support multiple Godot versions. I'm not saying we should delay removing the copy constructor for that use case, I'm just saying I don't think that should hold everything up. Plus, this way, it is easier to cherry-pick the addition of the functions to older branches if needed.

As for confusion on which to use, if the plan is to remove one, then... it doesn't matter, we'll migrate them all one way or another, since that migration will be forced to happen when removing the copy constructors.

We could definitely add the function to other container types, yeah. Which ones did you have in mind?

This PR's deprecation of the implicit duplication has caught a bug in the Wayland code: #101774 (comment)

I suppose that's another argument in favor of merging this in sooner rather than later :)

@aaronfranke I think for now, we should simply make the existing constructor explicit.

While .duplicate is a design pattern that has some merits, it's a bit of a paradigm shift and would need discussion and effort to make our APIs consistent in that regard. If you want to pursue this, I'd recommend starting by writing a proposal, so we can start discussing.

On the other hand, making the existing constructor explicit has similar benefits, but it would be a simple improvement we could merge right now.

@Ivorforce Attempting to do that has revealed a new problem. In functions like GDScript::get_all_dependencies(), GLTFDocument::get_supported_gltf_extensions_hashset(), and a dozen others, it is constructing the object being returned, so ideally we expect it to just be trivially moved to its destination. However, the presence of our explicit copy constructor means that instead a non-trivial copy is being performed in cases like these. If we were to remove the copy constructor entirely, that would allow us to use the implicit "move constructor", which I just now learned existed. But if we define a copy constructor, the implicit move constructor is deleted, and we would have to re-add it. A simpler option is to just delete the copy constructor and instead add a duplicate() function.

Another thing: Intellisense can find all uses of a function, but doing the same with constructors is much harder.

@Ivorforce Attempting to do that has revealed a new problem. In functions like GDScript::get_all_dependencies(), GLTFDocument::get_supported_gltf_extensions_hashset(), and a dozen others, it is constructing the object being returned, so ideally we expect it to just be trivially moved to its destination.

Yea... this is a problem we'll have to deal with eventually anyway. HashSet needs a move constructor, and whenever something is returned, it should be moved rather than copied.
I'm happy to do this, just let me know.

However, the presence of our explicit copy constructor means that instead a non-trivial copy is being performed in cases like these. If we were to remove the copy constructor entirely, that would allow us to use the implicit "move constructor", which I just now learned existed. But if we define a copy constructor, the implicit move constructor is deleted, and we would have to re-add it. A simpler option is to just delete the copy constructor and instead add a duplicate() function.

I don't think C++ can generate a correct trivial copy constructor for HashSet. If it were to generate one (which I don't think it would, as long as we have either any constructor or any value set to defaults), it would copy the pointers. However, HashSet owns its arrays, so it would need a copy constructor that copies the underlying arrays.

Another thing: Intellisense can find all uses of a function, but doing the same with constructors is much harder.

That may be true, and it's a good argument for the .duplicate pattern. But again, our number one priority is consistency. Currently, we use constructors quite extensively, and it would be surprising if some constructors were constructors and some were factory methods. It would probably lead to users using the wrong one, leading to inefficiencies. If we want to switch, we'll need to make a coordinated switch and change our paradigm throughout the codebase (or at the very least, consistently for this class and similar classes). Changing just one instance will lead to problems.