Hello forum!

I was wondering if this bad design:

I have a class that handles components of my game. One of these components is the state component.

It holds a list of smart pointers to states. These states derive from their base class. The handler of this component iterates over the list and calls all the active states update methods. If a state is paused, it will be skipped.

Now, how would I handle the situation of a state being no longer required in order to free the RAM?

Should I just delete the state? That is what I feel like is the only possibility.

Is that okay? Is this bad design?

What other ways would be better?

There are many kinds of "smart" pointer, all of which do very different things and have very different semantics and implications regarding ownership (which seems to be the crux of your issue here). So what kind of smart pointer are you thinking about here?

A std::unique_ptr would probably be a reasonable choice, as it sounds like the component is the authoritative owner of the states and controls their lifetime. In that case you just reset() the pointer to release the underlying object.

Yes, forgot to mention that I'm using the std::unique_ptr!

So it is completely legit to have this authoritative owner which controls lifetime?

So it is completely legit to have this authoritative owner which controls lifetime?

Yes, and it's usually good. Having clear rules and conventions about who owns what leads to fewer bugs, generally speaking, and easier-to-reason-about interfaces.

So it would be bad if the state wants to delete itself?

For example, the state has its own key-events. Now, a key has been pressed that will create a new state. The old state is now pointless.

However, the state does not know its smart pointer in the list. How would this work out?
Or should it be avoided?

So it would be bad if the state wants to delete itself?

Depends. If you're talking about "delete this;" than that is almost certainly a bad idea. If you're talking about knowing that the state has completed its work and should become inactive permanently, that's another thing altogether. You already have a system by which a state can make itself active or not; this sets a flag which the owner of the state queries when doing updates. If you want to support "permanent deactivation," you can simply create a similar flag and have the owner collect all states that marked themselves as permanently deactivated during an update and delete them itself.

Personally it seems like a weird thing for a state to do, but maybe it makes more sense in the context of your specific implementations.

For example, the state has its own key-events. Now, a key has been pressed that will create a new state. The old state is now pointless.

Ah! I thought that flags are a rather ugly way of handling, that was just my feeling though, as I had this idea of deleting them directly.

This makes a little more sense; this is just transitioning to a new state, which you can also communicate via some flags or calls to the owner. You may not actually want to destroy the original state here (what if the user wants to come back to it and see everything in the same spot?) though.

That is why I have my pause state, but some states really have no further purpose and just serve as brief GUI interface and so on. Therefore, I want to get rid of them.

For example a pause-screen.

if(!component->IsAlive()) remove from list.

This might be for another thread, but I thought getter/setter should be avoided by using methods that just do things instead of requesting information? This might be more of a religious thing - there were some people talking about it. It feels hard to avoid getter/setter sometimes, though.

If you can't query other objects, it makes it very awkward to construct any sort of useful program. You'd have to have a function inside the state that calls out and tells its owner to delete it, which couldn't do that immediately because the state is still alive at that point, so it would have to make a note and do that later. Ugly.

Making an object full of getters and setters for each field is usually a bad idea because that leaves you with no real encapsulation. But there's nothing wrong with an object being able to report some aspect of its state.