Jump to content
  • Advertisement
Sign in to follow this  
jwezorek

unique_ptr trees as the node tree of a 2D game framework difficulties

This topic is 544 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I'm working on modernizing an old 2D game engine in C++ that I started in (i think) pre-C++11 / C++14 days. 
 
It uses a scene graph/ sprite node tree, whatever you want to call it, that had been a tree of boost::shared_ptrs, but I want to change it to use std::unique_ptrs. My reasoning is that unique ownership of nodes by parent nodes and unique ownweship of the top level node by the scene object is what actually makes sense. I think I had been using shared_ptrs because at the time I did everything with shared_ptrs.
 
Everything is fine however one thing that is definitely awkward is the codebase's concept of registering an object to recieve a callback. The way it works is if you want some sprite to receive an update on each iteration of the game loop you register an update function with the parent scene, likewise for keyboard events and other user input events. 
 
so there's member functions like
  void Scene::scheduleUpdate(const std::shared_ptr<Actor>&, const UpdateHandlerFunc& func);
  void Scene::registerKeyEventHandler(const std::shared_ptr<Actor>, const KeyEventHandlerFunc& func);
  ...
to do this, which will store the functions in a hash table in the scene using Actor*'s as keys.
 
Now the trouble is that a common usage pattern would be something like the following, when initializing a scene would be to have some code like the following (the this's her
 
auto spaceship = sprite_sheet_->getSprite("spaceship"); //this returns a shared_ptr
// code to intialize spaceship...
this->addActor( spaceship ); // "this" here is a scene
this->scheduleUpdate(spaceship , 
    [&](const std::shared_ptr<coy::Actor>& actor, float dt) {
         actor->updateShip(dt); //or whatever
    }
);
 
the trouble is if I make getSprite return a unique_ptr which it should then the call to addActor would be a addActor( std::move(spaceship) ) which means I'd no longer have a pointer to spaceship to scedule an update to. I could change the order of those calls but youd be scheduling an update to sprite that is not (yet) a child of the scene, which seems bad. 
 
I could make addActor be optionally passed an ID that can then be used for doing things like scheduling updates?
 
Or I could get rid of the whole notion of scheduling updates and registering receivers of keyboard events, but I definitely don't want to replace it with havinng updatees need to inherit and override an  virtual update(float dt) member function because I am trying to keep from forcing users of the framework from having to necessarily do inheritance as much as possible. 
 
But I'm not sure what to replace it with: UpdateHandlerFunc et. al. member variables in Actor? 
Edited by jwezorek

Share this post


Link to post
Share on other sites
Advertisement

the trouble is if I make getSprite return a unique_ptr which it should
 

It shouldn't, though.

 

unique_ptr carries sole ownership wherever it goes. If you're returning it from the function, then you've already transferred ownership away from whatever had it before (in this case the sprite sheet), which is probably not what you intended.

 

This came up in another topic recently. Basically, don't pollute your interfaces with unique_ptr/shared_ptr/etc. unless you're dealing with ownership semantics. Otherwise, keep it simple with raw pointers or IDs.

Share this post


Link to post
Share on other sites

Well you could always keep a raw pointer or reference to the sprite that you pass to scheduleUpdate.
 
Something like this:

auto spaceship = sprite_sheet_->getSprite("spaceship"); //this returns a unique_ptr
// code to intialize spaceship...
auto& spaceshipRef = *spaceship.get();
this->addActor( std::move(spaceship) ); // "this" here is a scene
this->scheduleUpdate(spaceshipRef , 
    [&](const Actor& actor, float dt) {
         actor.updateShip(dt); //or whatever
    }
);

It seems like a somewhat decent solution as long as you manage ownership correctly, although I can't say if it's the best solution or not.

 

It shouldn't, though.

unique_ptr carries sole ownership wherever it goes. If you're returning it from the function, then you've already transferred ownership away from whatever had it before (in this case the sprite sheet), which is probably not what you intended.

We don't know exactly what the getSprite function does though, but if it's not intended to transfer ownership, then you are completely right. Interfaces shouldn't be bloated with smart pointers if the call is not about ownership. But I got the impression that jwezorek wanted the ownership to transfer into the addActor function.

Edited by Ansou

Share this post


Link to post
Share on other sites

 

the trouble is if I make getSprite return a unique_ptr which it should
 

It shouldn't, though.

 

unique_ptr carries sole ownership wherever it goes. If you're returning it from the function, then you've already transferred ownership away from whatever had it before (in this case the sprite sheet), which is probably not what you intended.

 

This came up in another topic recently. Basically, don't pollute your interfaces with unique_ptr/shared_ptr/etc. unless you're dealing with ownership semantics. Otherwise, keep it simple with raw pointers or IDs.

 

 

Sprite sheets here do not own sprites. It's a wrapper around a texture packed with sprite frames and metdata needed to grab a given sprite frame i.e. its location in the texture. Maybe the name is bad, could be a SpriteAtlas and createSprite maybe. But in any case it is like a factory not a container. I think it should return unique_ptrs because the semantics of the whole thing are that sprites should always have one and only one owner.

Edited by jwezorek

Share this post


Link to post
Share on other sites

...
If A is the sole owner, and B's lifetime is known to be shorter than C's lifetime, then A should have a unique pointer and B should have a raw pointer.
...

so then in this case, the register functions should take raw pointers or references to the actor being registered, right?

Share this post


Link to post
Share on other sites

so then in this case, the register functions should take raw pointers or references to the actor being registered, right?
 

 

Assuming you can guarantee that once an actor is destroyed, the code that handles the registered updates for that actor (which has stored a raw pointer to it) isn't going to run, then yes.

 

I would rename SpriteSheet::getSprite to SpriteSheet::createSprite to better describe what it does.

Share this post


Link to post
Share on other sites

I keep seeing this topic come up and I sat and tried to think about why people run into this. I think that the main difference is that people are still using pointers in many cases where dealing with objects with value semantics would be easier and more idiomatic. If I write a getSprite/createSprite function I want it to return an object by value so that I can use it as I see fit. I'd be far more likely to do something like this:

class SpriteThingyOrWhatever {
public:
  void userFunctions();
  //...

private:
  friend class TheFactoryClassThatContainsSomeDependencyUsedToCreateMe;
  SpriteThingyOrWhatever(Argument arg, ThingFromTheFactory thing);

  Something thingThatINeedToWork;
  std::unique_ptr<Gadget> otherThingThatINeedToWork();
  //...
};

The factory object that spits out the interface by value may need to 'return std::move(spriteBeingMade)' but can probably just 'return SpriteThingyOrWhatever(arg, thing)', which could RVO (and you want that).

 

You may need to brush up on the 'rule of five' functions, or member initializer lists, but preferring to work with objects by value rather than using pointers with mandates about ownership and lifetime (or built-in refcounting or whatever) will make you a happier person in C++ land.

 

Just my 2 bits.

Share this post


Link to post
Share on other sites

I keep seeing this topic come up and I sat and tried to think about why people run into this. I think that the main difference is that people are still using pointers in many cases where dealing with objects with value semantics would be easier and more idiomatic. If I write a getSprite/createSprite function I want it to return an object by value so that I can use it as I see fit. I'd be far more likely to do something like this:

 

The trouble with value semantics is what to do if the thing that you want to pass around needs to use olden days C++ polymorphism. If you pass around an object by value you have to know what that object is; if all you have is an interface to it, you have to pass around some flavor of pointer. You could wrap that olden days C++ polymorphism in various ways, to have an object that will do polymorphic dispatch but that you can treat like a value. To do that "idiomatically" is easy, it's called a shared_ptr<T> where T is a base class, which is why code from 10 years ago had boost::shared_ptrs everywhere. Before people got fancy about talking about the semantics  of ownership people were treating shared_ptrs as a value semantics wrapper around banal C++ polymorphism.

 

You can have value semantics and do polymorphic behavior via composition rather than inheritance, but this opens its own can of worms to framework writers. In the same way in this thread people are saying "dude, just use raw pointers" you find yourself saying "dude, just use inheritance." It's a trade-off like everything else.

 

In this example, spritesheet->createSprite("name_of_a_sprite_in_the_sheet") is generating a concrete object of type Sprite, but another way you could get a sprite is by defining your own class SpaceshipSprite that inherits from Sprite and calls a constructor like Sprite::Sprite(spritesheet, "name_of_a_sprite_in_the_sheet") from its constructor but also inserts child sprites for the spaceship's thruster flame or whatever in the constructor. If you want to then pass SpaceshipSprite to all the places where you could pass a Sprite, by value not by any flavor of reference, then all those places will have to be parametrized on type which will turn your turn your framework into a header-only or header-mostly framework and mean that you can't shield users of the framework from its dependencies, the hardware abstraction layer it uses etc., by statically linking to those dependencies and using them intenally. You can't link to anything, you're not code, you're a template.

Edited by jwezorek

Share this post


Link to post
Share on other sites

The other problem with value semantics is that a lot of functionality relies on identity (measured by comparing memory addresses) rather than equality (measured by comparing member values). You can add unique IDs into the objects, but then it's unclear whether a copy operation - especially implicit copies - should create a new ID or not.

 

For the original poster's problem, I see no reason why the whole lot can't be left as shared_ptr. Just because we now have a usable unique_ptr, doesn't mean one has to use it. And if you do decide to use it in your factories or whatever, it's perfectly fine to transfer the pointer into shared_ptrs after that. External systems can hold weak_ptr references if you like.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!