Solving game state with polymorphism

I've been working on a small game as a hobby, and I've been having trouble finding a good way to control the state of the game that could scale easily. I couldn't find a solution that I felt was correct, so I wrote my own.

I created an abstract base class State that has an abstract function update that takes and returns a pointer to the State base class. All children of State's update function should check if conditions are met to change the State if so it should delete the current state, and return another child of State. If it shouldn't transition states it update its data members and return the current state for the next pass in the loop.

I'm not a very experienced C++ programmer, and I feel I'm in over my head. I came to this solution by myself, and even though it work correctly and appears like it will scale fine in the future I'm worried if I went about this wrong or if there are safer or more efficient ways to deal with this problem. Looking back at my code I'm surprised I can delete a pointer of a class inside one of its own functions. Also the update method by design is completely unsafe which could be trouble.

I cut the state machine out of my project, and made a simple model of how it could be used, with two example states.

main.cpp

#include "State.h"
#include "State1.h"
int main() {
    State* state = new State1();
    //Main loop, how state updates itself.
    while (state->isRunning()) {
        state = state->update(state);
    }
    return 0;
}

State.h

class State {
public:
    State();
    virtual ~State();

    //Very unsafe method to update data in the main loop and change state.
    //Up to programmer to not mess things up.
    virtual State* update(State* currentState) = 0;
    //getter for running
    bool isRunning() const;
protected:
    static bool running;
};

State has a static variable running to keep track of when it should quit the main loop, like when the user closes the window. I felt a static variable was fine

I see some things that may help you improve your program.

Fix your includes

Case matters to a C++ compiler, so when you write #include it is not the same as the standard #include even if your compiler and/or operating system happens to accept it.

Prefer references to pointers

The argument to update is a State * but I think it should instead be a State &. You don't really ever want a nullptr there, and the current code doesn't check for it.

Don't delete yourself via pointer

Within the update routine we have these two lines:

State *State1::update(State *currentState) {
    // some other code
    delete currentState;
    return new State2(dataMember);
}

However, the usage in main is this:

state = state->update(state);

The problem with this is that the delete above invalidates all member data, including dataMember which is then erroneously used on the following line. Instead you could use something like this:

State *retval = new State2(dataMember);
delete currentState;
return retval;

I don't really like that very much either, but at least it does not reference invalid memory.

Consider using some other abstraction

The use of a single global variable for State::running effectively limits the design to only having a single useful instance of State at one time. If that's acceptable (and it seems to be in this use) then it suggests to me that a class hierarchy may not really the most appropriate abstraction mechanism. Instead, it's likely that an array (or vector or list or whatever) of functions may work better. Within each function, any memory allocation or freeing could happen without the user of the state machine having to worry about it.

StackExchange Code Review Q#106182, answer score: 5