2

I have this collection:

vector<unique_ptr<Light>>* lights;

I have many descendants of the Light class, like DirectionalLight, PointLight and so on.

I wish to store all descendants of Light within that lights vector like so:

template<typename T>
unique_ptr<T> CreateLight()
{
    static_assert(std::is_base_of<Light, T>::value, "Type must of descendant of type Light. ");
    unique_ptr<T> light(new T());
    lights->emplace_back(light);
    return light;
}

The reason for this method is that I store my light in a collection for my renderer, which will do its magic to make the lights affect the shaders.

EDIT

These collections are parts of a class named Scene. I need them all the time and I need to have all Light instances on the heap (together with all the other instances the Scene class has). Every frame the Renderer will go through the collection of lights to affect the scene objects' shaders with them. Accessing this vector any given time is of paramount importance.

I still need a reference to my light in the scene though so I can manipulate its properties.

The error message is this:

Severity    Code    Description Project File    Line    Suppression State
Error   C2664   'std::unique_ptr<Light,std::default_delete<_Ty>>::unique_ptr(const std::unique_ptr<_Ty,std::default_delete<_Ty>> &)': cannot convert argument 1 from 'std::unique_ptr<DirectionalLight,std::default_delete<_Ty>>' to 'std::nullptr_t'

This fails during build, not runtime. I, of course, took a look at answers like this one but to no avail.

I require assistance to get this sorted out.

JeJo
  • 30,635
  • 6
  • 49
  • 88
agiro
  • 2,018
  • 2
  • 30
  • 62
  • 3
    Unrelated to your problem, but there are almost no use-cases for having a pointer to a container like `std::vector`. – Some programmer dude Aug 19 '18 at 13:04
  • So if I leave that one out, it won't copy the whole contents to the method when I pass the container? – agiro Aug 19 '18 at 13:05
  • Not with either copy-elision or move-construction. – Some programmer dude Aug 19 '18 at 13:07
  • Okay, I go ahead and look them up. – agiro Aug 19 '18 at 13:07
  • Use `std::shared_ptr` for sharing of memory resources – acade Aug 19 '18 at 13:12
  • As for your problem, you should not really see e.g. `std::unique_ptr` as a self-deleting pointer only, it's almost always better to look at it from an *ownership* perspective. Will some "resource" have only a single owner (`std::unique_ptr`) or multiple owners (`std::shared_ptr`)? – Some programmer dude Aug 19 '18 at 13:13
  • True that one. In my case it's rater `shared` as I plan to pass it to the renderer and the game logic as well. – agiro Aug 19 '18 at 13:15
  • By the way if I pass it to the other method like `void Render(vector& gameObjects, vector>& lights, Camera& camera)` that won't create a copy even if I store the container without the pointer operator, right? I know it's off topic, but you made a useful comment. – agiro Aug 19 '18 at 13:23
  • 1
    Avoid raw pointers if possible. Passing by reference creates no copies. A compiler won't let you make a copy of `vector` anyway. – Evg Aug 19 '18 at 13:41

3 Answers3

3
lights->emplace_back(light);

change to:

lights->emplace_back(new T());
isnullxbh
  • 807
  • 13
  • 20
  • Thanks but then how do I get the freshly created `light` instance so I can return it? – agiro Aug 19 '18 at 13:06
  • @agiro, You can not do this, because unique_ptr is owner for T(), and you can't have two instance that are owners of distinct instance. – isnullxbh Aug 19 '18 at 13:11
  • @agiro, I agree with JeJo, function 'CreateLight' look like a fabric method, you shouldn't burden it with side-operations like 'add light to array'. But it is a tip, I don't insist on that. – isnullxbh Aug 19 '18 at 13:50
  • I take every advice I get as I'm primarily a c#/java developer and I understand this code smells c# too. These issues I'm facing now are kind of obvious in c# :( – agiro Aug 19 '18 at 14:20
3

I have many descendants of the Light class, like DirectionalLight, PointLight and so on. I wish to store all descendants of Light within that lights vector like so.

Then you simply need, a vector of unique_ptr<Light> rather than the pointer to the vector of unique_ptr<Light>; which is kind of over-engineered way of doing things.

std::vector< std::unique_ptr<Light> > lights;

Then you can handle everything easily.

template<typename T>
void CreateLight()
{
   static_assert(std::is_base_of<Light, T>::value, "Type must of descendant of type Light. ");
   // store to vector directly
   lights.emplace_back( std::make_unique<T>(/*args*/));
}

how do I get the freshly created light instance so I can return it?

As you have the freshly created instance at the last of lights vector, you can directly use it wherever you want. See here

//Either directly
auto iter = lights.rbegin();
if(*iter) std::cout << "Done\n";

 // or
 auto lastElement = std::move(lights.back());
 if(lastElement) std::cout << "Done\n";
JeJo
  • 30,635
  • 6
  • 49
  • 88
  • When you `std::move` from `light.back()`, the `unqie_ptr` is transferred from `lights` to `lastElement`. When `lastElement` goes out of scope the object managed by it will be destroyed. – Evg Aug 19 '18 at 14:16
  • @Evgeny Yes. You shouldn't be accessing the last element of the vector again after moving, as there won't be anything. Just shown the two possible use cases, since I don't know what exactly OP wants to do with it. – JeJo Aug 19 '18 at 14:21
  • It's a good answer, I try it out right away. I elaborate the use case of the collections a bit in my question – agiro Aug 19 '18 at 14:25
  • @JeJo I added the use case of this vector to the question. If you can spare the time, please take a look at it. – agiro Aug 19 '18 at 14:32
  • @agiro, to obtain a reference to the `i`-th element use `Light& light = *lights[i];`. – Evg Aug 19 '18 at 14:49
  • 1
    I've use that pattern in Java factories where the object gets stored somewhere but a separate reference is returned so now one has a reference on his own to manipulate the object and some other entity in the system has a reference too so that can manipulate the object as well. – agiro Aug 19 '18 at 15:00
  • By the way I tried this method you shown and it fails to build in `xmemory0` for some reason in the `construct` method, line 920: cannot convert argument1. I've given up on returning the created object though, I have a `void` now. – agiro Aug 19 '18 at 15:11
  • @JeJo This is the exact error: `Severity Code Description Project File Line Suppression State Error C2664 'std::unique_ptr>::unique_ptr(const std::unique_ptr<_Ty,std::default_delete<_Ty>> &)': cannot convert argument 1 from 'std::unique_ptr>' to 'std::nullptr_t'` – agiro Aug 19 '18 at 15:34
  • @agiro First of all check whether you have any of the situations given in [this link](https://learn.microsoft.com/en-us/cpp/error-messages/compiler-errors-2/compiler-error-c2664). Then you will find also the way out. – JeJo Aug 20 '18 at 05:52
  • 1
    @agiro secondly, *in Java factories where the object gets stored somewhere but a separate reference is returned* The way I proposed is, almost similar: you create new object on the heap, which can be accessed through the stored pointers. However, like in Java, you cannot directly save the new objects to any container, instead, you need pointers to access them. If you need to have multiple references you need to use [`std::shared_ptr`](https://en.cppreference.com/w/cpp/memory/shared_ptr) – JeJo Aug 20 '18 at 05:58
  • @JeJo thanks for the help. The `shared_ptr` is also worth checking out. This C++ sure does give more control than Java but boi at what price xd – agiro Aug 20 '18 at 08:47
0

You can simply return a reference to a newly created object:

std::vector<std::unique_ptr<Light>>* lights;

template<typename T, typename... Args>
T& CreateLight(Args&&... args)
{
    ...
    auto light = std::make_unique<T>(std::forward<Args>(args)...);
    lights->push_back(std::move(light));
    return *static_cast<T*>(lights->back().get());
}

Edit. After reading JeJo's answer, modified CreateLight so that it accepts any number (including zero) of arguments to construct T.

Evg
  • 25,259
  • 5
  • 41
  • 83
  • It throws a bunch of errors, but how does the container definition look like in your scenario? `vector lights`? – agiro Aug 19 '18 at 13:40
  • The same as yours. Sorry, `lights.back` should be `lights->back`. Edited my answer. – Evg Aug 19 '18 at 13:44
  • Thanks but it still "cannot convert from Light to DirectionalLight & " – agiro Aug 19 '18 at 13:48
  • @agiro, corrected once again. Don't forget to make the destructor of `Light` virtual. – Evg Aug 19 '18 at 14:00