0

Been stuck on this code for the past hour, still trying to get my head around smart pointers and implementing them but this issue has had my stumped for quite a bit.

void GameState::addEntity(std::unique_ptr<Entity> gameObject)
{
    if(gameObject->isCollidable()){
        _actors.Add(gameObject);
    } else {
        _props.Add(gameObject);
    }
}

// This is the method the above function is trying to call.
void GameObjectManager::Add(std::unique_ptr<Entity> gameObject)
{
    _gameObjects.insert(std::make_pair(ID, std::move(gameObject)));
    ID++;
}

The error message I'm reciving is;

'std::unique_ptr<_Ty>::unique_ptr' : cannot access private member declared in class 'std::unique_ptr<_Ty>'  
Joseph Mansfield
  • 108,238
  • 20
  • 242
  • 324
user1725794
  • 247
  • 1
  • 5
  • 14

3 Answers3

3

You need to pass ownership from addEntity to Add by using std::move:

if(gameObject->isCollidable()){
    _actors.Add(std::move(gameObject));
} else {
    _props.Add(std::move(gameObject));
}

You cannot pass a unique_ptr without explicitly allowing it to be moved from. This is what makes it a unique pointer. If you could copy it, both the original and the copy would be pointing to the same object. When you move from it, the original pointer gives up ownership to the new one.

Joseph Mansfield
  • 108,238
  • 20
  • 242
  • 324
1

You are trying to pass a unique_ptr lvalue to a function which takes a unique_ptr by value, which would result in creating a copy of that pointer: this is intentionally forbidden. Unique pointers are meant to model unique ownership: this means that the entity that holds a unique pointer has the ownership of the pointed object.

Copying a unique pointer around would mean having several owning pointers spread all over the system: this would of course defeat the purpose itself of unique ownership, which explains why you can't copy unique pointers.

What you can do is to transfer this ownership, so if you call a function that accepts a unique pointer, you could move the unique_ptr when you pass it as an argument to that function:

_actors.Add(std::move(gameObject));
//          ^^^^^^^^^

Notice, that transferring ownership by moving away from a unique pointer means that the caller function is left with a "zombie" unique pointer object which is no longer owning the object it previously pointed to: therefore, the caller function should not try to dereference it anymore.

Andy Prowl
  • 124,023
  • 23
  • 387
  • 451
1

You cannot pass a unique_ptr by value. Following are trying to create copies of a unique_ptr, which is forbidden:

if(gameObject->isCollidable()){
    _actors.Add(gameObject);     // THIS LINE
} else {
    _props.Add(gameObject);      // and THIS LINE
}

A solution is passing with std::move():

if(gameObject->isCollidable()){
    _actors.Add(std::move(gameObject));
//              ^^^^^^^^^
} else {
    _props.Add(std::move(gameObject));
//              ^^^^^^^^^
}

This will result in two ownership transfers. One to the std::unique_ptr<Entity> which is the input parameter of GameObjectManager::Add and one to the std::pair in _gameObjects.

Another solution is modifying the signature of GameObjectManager::Add to get a reference:

void GameObjectManager::Add(std::unique_ptr<Entity> & gameObject)
//                                                 ^^^

Now, you can call the method as you are currently doing, and this will result in only a single ownership transfer (to the pair in _gameObjects).

But, as pointed out by @Xeo in the comments the second option violates the best practice rule-of-thumb which is: Ownership transfer should be explicit at all points.

meyumer
  • 5,063
  • 1
  • 17
  • 21
  • How do you mean Xeo, would using the reference method be bad progrmaming? – user1725794 Apr 14 '13 at 16:44
  • @user1725794: It would be subtle. Ownership transfer of a unique pointer should always be explicit. This way you end up with two routines sharing a unique pointer, which kind of defeats the purpose of `unique_ptr` (i.e. that the object should be owned by *one* entity at a time). Btw, if you want to notify some user, you have to use the `@` character before the username. – Andy Prowl Apr 14 '13 at 16:48