Finite State Machine / Scene Manager in C

Question

I'm new to C programming but this is my attempt at a Scene/State machine. I haven't had a code review before so I'm guessing this is gonna hurt :D

§ Preliminary

SceneFSM manages Scenes, one Scene "active" at a time. A Scene is just a C file with method definitions;

Each scene may (or may not) contain three methods;

• Start; called once whenever the scene starts
• Update; called repeatedly until a state switch is required
• End; called once whenever the scene ends

These methods return one of the responses detailed in SceneFSM.h. The responses determine what happens next in the machine.

There are a few things I'm particularly interested in help with;

§ 1

Is the SceneFSM__Run method stack efficient?

My intention is that the function stack does not get overflowed.

FSM Control starts at SceneFSM__Start, into SceneFSM__Run, then into whatever Scene function is applicable.

Importantly, control is allowed to come back to the SceneFSM__Run method after every scene switch, eventually coming back to SceneFSM__Start and SceneFSM__End to program exit.

§ 2

In order to switch to another scene, the currently running method should return NEXT but before doing so must set the struct member SceneFSM->nextSceneName so that the machine actually knows where to go next.

The rest of the responses require no additional parameters to be understood, just the NEXT response does :|**

I'm sure this is janky but I've no idea how to better define this.

§ 3

Anything else that's going to bite me in the ass?

I realise my while loop needs timing and I'm not done with error handling but is there anything else flaringly-obviously bad?

§ NB

"App" is just a struct that contains whatever data I need it to for this particular app. It isn't really relevant.

I haven't posted any Scene implementations but you can see their definition in the Scene struct. I can post them if it will help.

I've added the 'Boot' scene file to show how scene is switched.

Thank You for your time and input!

Dave

SceneFSM.h

#ifndef SceneFSM_h
#define SceneFSM_h

#include "App.h"

/*
------------------------------------------
Public Properties.
*/

/*
CONTINUE    Continue to execute the next Scene method.
NEXT        Switch to the scene named by SceneFSM->nextSceneName (call End on the current scene first).
            SegFault will occur if NEXT is returned and SceneFSM->nextSceneName is NULL or invalid.
            NB. NEXT can be returned from Update without setting SceneFSM->nextSceneName first,
            providing that the Scene's End function returns EXIT or ERROR.
EXIT        Exit immediately with success.
ERROR       Exit immediately with error.
*/

typedef enum { CONTINUE, NEXT, EXIT, ERROR } SceneResponse;

// Forward declaration of SceneFSM; it is used by Scene which is defined below.
typedef struct SceneFSM SceneFSM;

/*
Scene.
Start and End are executed when the scene starts and ends.
Update is executed repeatedly until it returns something other than CONTINUE.
All three functions are optionally assigned; the FSM will ignore missing ones.
*/
typedef struct {

    char * name;

    SceneResponse (* Start) (SceneFSM * _SceneFSM, App * _app);
    SceneResponse (* Update) (SceneFSM * _SceneFSM, App * _app);
    SceneResponse (* End) (SceneFSM * _SceneFSM, App * _app);
} Scene;

/*
SceneFSM.
Holder of scenes.
*/
struct SceneFSM {

    Scene * currentScene;
    char * nextSceneName;

    Scene * scenes;
    int numScenes;
};

/*
------------------------------------------
Public Declarations.
*/

void SceneFSM__Start(Scene * _scenes, int _numScenes, void (* _onEnd) (void));

#endif

SceneFSM.c

#include "SceneFSM.h"

#include <stdlib.h>
#include <string.h>
#include <signal.h>

/*
------------------------------------------
Private Declarations.
*/

static void SceneFSM__End(int _result);
static int SceneFSM__Run(SceneFSM * _SceneFSM, App * _app);
static SceneResponse SceneFSM__SwitchScene(SceneFSM * _SceneFSM, App * _app, SceneResponse _response);
static int SceneFSM__SceneIndexByName(SceneFSM * _SceneFSM, char * _name);

/*
------------------------------------------
Public Definitions.
*/

/*
Start the SceneFSM session.
*/
void SceneFSM__Start(Scene * _scenes, int _numScenes, void (* _onEnd) (void)) {

    App app;

    atexit(_onEnd);
    signal(SIGABRT, SceneFSM__End);
    signal(SIGTERM, SceneFSM__End);
    signal(SIGINT, SceneFSM__End);

    SceneFSM SceneFSM = { .scenes = _scenes, .numScenes = _numScenes };
    SceneFSM__End(SceneFSM__Run(&SceneFSM, &app));
}

/*
------------------------------------------
Private Definitions.
*/

/*
End the SceneFSM FSM.
*/
static void SceneFSM__End(int _result) {

    exit(_result);
}

/*
Run the FSM.
Executes Start, Update, End methods in the current scene and handles switching of scenes.
Rather a mess, but does not load the stack up with functions (as if each state called the next).
*/
static int SceneFSM__Run(SceneFSM * _SceneFSM, App * _app) {

    // Fetch initial scene.

    _SceneFSM->currentScene = &_SceneFSM->scenes[0];
    int response = NEXT;

    // Iterate while we have another state to go to.

    while(response == NEXT) {

        // Execute Scene Start

        if(_SceneFSM->currentScene->Start) {

            response = _SceneFSM->currentScene->Start(_SceneFSM, _app);

            if(response == NEXT && _SceneFSM->currentScene->End) {

                response = SceneFSM__SwitchScene(_SceneFSM, _app, response);
                continue;
            }

            if(response == ERROR || response == EXIT) break;
        }

        // Iterate Scene Update.

        if(_SceneFSM->currentScene->Update) {

            while(response == CONTINUE)
                response = _SceneFSM->currentScene->Update(_SceneFSM, _app);

            if(response == NEXT && _SceneFSM->currentScene->End)
                response = SceneFSM__SwitchScene(_SceneFSM, _app, response);

            if(response == ERROR || response == EXIT) break;
        }
    }

    if(response == ERROR) return 1;
    return 0;
}

/*
Switch to the scene defined in SceneFSM->nextSceneName.
*/
static SceneResponse SceneFSM__SwitchScene(SceneFSM * _SceneFSM, App * _app, SceneResponse _response) {

    SceneResponse endResponse = _SceneFSM->currentScene->End(_SceneFSM, _app);
    if(endResponse == ERROR || endResponse == EXIT) return endResponse;

    _SceneFSM->currentScene = &_SceneFSM->scenes[SceneFSM__SceneIndexByName(_SceneFSM, _SceneFSM->nextSceneName)];
    _SceneFSM->nextSceneName = NULL;

    return _response;
}

/*
Find the index of a named scene.
*/
static int SceneFSM__SceneIndexByName(SceneFSM * _SceneFSM, char * _name) {

    int index = 0;
    while(index < _SceneFSM->numScenes) {

        if(!strcmp(_SceneFSM->scenes[index].name, _name)) return index;
        index++;
    }

    return -1;
}

main.c

#include "ANSI.h"

#include "SceneFSM.h"
#include "Boot.h"
#include "Title.h"
#include "Game.h"

void end(void);

int main(int argc, const char * argv[]) {

    Scene scenes[] = {

        {
            .name = "Boot",
            .Start = Boot__Start,
            .End = Boot__End
        },
        {
            .name = "Title",
            .Start = Title__Start,
            .End = Title__End
        },
        {
            .name = "Game",
            .Start = Game__Start,
            .Update = Game__Update,
            .End = Game__End
        }
    };

    ANSI__Start();
    SceneFSM__Start(scenes, 3, end);

    return 1;
}

void end() {

    ANSI__End();
    printf("Program complete.\n");
}

Boot.c - Yep, the scene is called 'Boot'.
Showing example of how scene may be switched by setting name and returning NEXT. Does it smell a bit?

#include "Boot.h"
#include "Image.h"
#include "Image__PBMP.h"
#include <stdlib.h>

SceneResponse Boot__Start(SceneFSM * _fsm, App * _app) {

    // Load sprite library.

    _app->spriteLibrary = malloc(sizeof(Image));
    if(Image__Open("./Data/sprites.pbm", _app->spriteLibrary, Image__PBMP__Parse)) {

        printf("Cannot open sprites\n!");
        return ERROR;
    }

    _fsm->nextSceneName = "Title";
    return NEXT;
}

SceneResponse Boot__End(SceneFSM * _fsm, App * _app) {

    printf("Boot End\n");
    return 0;
}

Answer 1

Stack efficiency

Is the SceneFSM__Run method stack efficient?

There is no recursion, so stack usage is bounded. The only way to make it more efficient is to have less on the stack in each function, but since you don't have much to begin with I would not worry about this.

How to switch scenes

In order to switch to another scene, the currently running method should return NEXT but before doing so must set the struct member SceneFSM->nextSceneName so that the machine actually knows where to go next.

Instead of having CONTINUE, NEXT and EXIT codes, you could consider to instead have the functions return a pointer to the (name of the) scene that should run next, and use NULL to indicate that it should exit instead. This avoids having to pass a pointer to the SceneFSM to the methods of each scene. If you also want to distinguish between normal exits and errors, then you could either define a struct that contains both a response code and the name of the scene to switch to, and return that from the member functions, or you could define a special error scene.

You can also consider to have the End method return something different from SceneResponse or the abovementioned pointer, since the only thing it should return is whether it ran succesfully or encountered an error, any other value doesn't make sense.

Another idea would be to not have Start, Update and End methods for each scene, but instead have only one method per scene, and then just create more scenes, for example:

typedef struct {
    const char *name;
    SceneResponse (*Run)(SceneFSM *_SceneFSM, App *_app);
} Scene;
...
Scene scenes[] = {
    {
        .name = "Boot__Start",
        .Run = Boot__Start,
    },
    {
        .name = "Boot__End",
        .Run == Boot__End,
    },
    ...
};

And then have Boot__Start set nextSceneName to Boot__End, and have Boot__End set nextSceneName to Title__Start, and so on. This simplifies the scene manager, but will actually make it more flexible.

Avoid manually calling atexit() and exit()

There sometimes might be reasons to install cleanup handlers with atexit(), but in this case it is completely avoidable. Instead of calling exit() inside SceneFSM__End(), why not have SceneFSM__Start() return the result value, and then main() can continue doing cleanup after the scene manager finished? Like so:

int SceneFSM__Start(Scene *_scenes, int _numScenes, void (*_onEnd)(void)) {
    ...
    return SceneFSM__End(SceneFSM__Run(&SceneFSM, &app));
}
...
int main(int argc, const char *argv[]) {
    ...
    ANSI__Start();
    int result = SceneFSM__Start(scenes, 3, end);
    ANSI__End();

    printf("Program completed %s.\n", result == 0 ? "sucessfully" : "with errors");
    return result;
}

Use const where appropriate

There are a few places where you should make variables const. First, once you have defined your scenes, you don't want to change their names. So make name const:

typedef struct {
    const char *name;
    ...
} Scene;

Similarly, in struct SceneFSM, you can make scenes and numScenes const as well.

The function SceneFSM__SceneIndexByName() doesn't modify the data pointed to by the _SceneFSM member variable, so than can be made const as well:

static int SceneFSM__SceneIndexByName(const SceneFSM *_SceneFSM, char *_name) {
    ...
}

Making things const prevents accidental writes, and allows the compiler to generate better optimized code.