gameflow: add bonus level type

[walkawayy]

Resolves #645.

Checklist

• I have read the coding conventions
• I have added a changelog entry about what my pull request accomplishes, or it is an internal change

Description

Added the bonus level type for custom levels that unlocks if all main game secrets are found.

[rr-]

I'd wait with this feature till 2.17.

[walkawayy]

Removed the exit to bonus level from the gameflow @lahm86. This won't be in the next release so obvi no rush, but I know you mentioned wanting to play around with this. So just FYI.

I think a current limitation may be that you can't have an FMV in between multiple levels.

[lahm86]

I see what you mean about the FMVs - I've attached an updated test gameflow.

Tomb1Main_gameflow.zip

If you don't collect all secrets, after the total stats screen, the FMVs will play whereas they shouldn't really as they're part of the bonus setup.

Would it be an idea to move the logic into the GFS_EXIT_TO_LEVEL case instead? That way the bonus levels are skipped over entirely.

Otherwise, it looks great 👍

[lahm86]

Works great now, thanks 👍

[lahm86]

It's all looking great to me - just a few minor comments.

[lahm86]

LGTM, just one extra debug message since yesterday.

[rr-]

The modifications made in Stats_ShowTotal require the caller to take on additional responsibilities, such as selecting the start and end level numbers, as well as providing a heading. In order to simplify this, it would be better to replace the introduced parameters with just GAMEFLOW_LEVEL_TYPE level_type.

The changes overall appear to be inspired by the first_level_num and last_level_num idiom used in our code. However, it is important to note that these variables should only be used to retrieve the first and last level numbers, and not for direct comparison with arbitrary level numbers. This is because the order of levels should not be determined solely by their appearance in the JSON, but rather by sequences like exit_to_level. Simple >= and <= comparisons do not account for this. While there are instances in the code where we compare level_num >= first_level_num or level_num <= last_level_num, this is not something we should continue doing in the new code, as it reinforces a reliance on the JSON order.

Upon further examination, the checks for first_bonus_num and last_bonus_num are seemingly only meant to determine if a given level_num belongs to the bonus levels. It would be more efficient to check this using g_GameFlow.levels[level_num].level_type == GFL_BONUS. This allows us to eliminate the need for the first_bonus_num, last_bonus_num and even has_bonus variables entirely.

Considering these points, it would be more appropriate for Savegame_PlayAvailableStory to now disregard the bonus levels, rather than halting at the first encounter of a bonus level. In theory, this revision would allow the builder to place bonus levels at the beginning of the gameflow file, followed by the normal levels, without causing any issues.

Here is a proposed patch that has been implemented to address the feedback mentioned, but please note that I haven't really tested it.

diff --git a/src/game/gameflow.c b/src/game/gameflow.c
index f95e9330..8ca4762c 100644
--- a/src/game/gameflow.c
+++ b/src/game/gameflow.c
@@ -678,9 +678,6 @@ static bool GameFlow_LoadScriptLevels(struct json_object_s *obj)
     g_GameFlow.gym_level_num = -1;
     g_GameFlow.first_level_num = -1;
     g_GameFlow.last_level_num = -1;
-    g_GameFlow.has_bonus = false;
-    g_GameFlow.first_bonus_num = -1;
-    g_GameFlow.last_bonus_num = -1;
     g_GameFlow.title_level_num = -1;
     g_GameFlow.level_count = jlvl_arr->length;
 
@@ -746,11 +743,6 @@ static bool GameFlow_LoadScriptLevels(struct json_object_s *obj)
             g_GameFlow.last_level_num = level_num;
         } else if (!strcmp(tmp_s, "bonus")) {
             cur->level_type = GFL_BONUS;
-            if (g_GameFlow.first_bonus_num == -1) {
-                g_GameFlow.has_bonus = true;
-                g_GameFlow.first_bonus_num = level_num;
-            }
-            g_GameFlow.last_bonus_num = level_num;
         } else if (!strcmp(tmp_s, "cutscene")) {
             cur->level_type = GFL_CUTSCENE;
         } else if (!strcmp(tmp_s, "current")) {
@@ -1199,8 +1191,7 @@ GameFlow_InterpretSequence(int32_t level_num, GAMEFLOW_LEVEL_TYPE level_type)
                 return ret;
             }
             if (level_type == GFL_SAVED) {
-                if (g_GameFlow.has_bonus
-                    && level_num >= g_GameFlow.first_bonus_num) {
+                if (g_GameFlow.levels[level_num].level_type == GFL_BONUS) {
                     level_type = GFL_BONUS;
                 } else {
                     level_type = GFL_NORMAL;
@@ -1240,15 +1231,7 @@ GameFlow_InterpretSequence(int32_t level_num, GAMEFLOW_LEVEL_TYPE level_type)
             if (g_Config.enable_total_stats && level_type != GFL_SAVED) {
                 GAMEFLOW_DISPLAY_PICTURE_DATA *data = seq->data;
                 LOG_DEBUG("level_type: %d", level_type);
-                if (level_type == GFL_BONUS) {
-                    Stats_ShowTotal(
-                        data->path, g_GameFlow.first_bonus_num,
-                        g_GameFlow.last_bonus_num, GS_STATS_BONUS_STATISTICS);
-                } else {
-                    Stats_ShowTotal(
-                        data->path, g_GameFlow.first_level_num,
-                        g_GameFlow.last_level_num, GS_STATS_FINAL_STATISTICS);
-                }
+                Stats_ShowTotal(data->path, level_type == GFL_BONUS ? GFL_BONUS : GFL_NORMAL);
             }
             break;
 
@@ -1300,7 +1283,7 @@ GameFlow_InterpretSequence(int32_t level_num, GAMEFLOW_LEVEL_TYPE level_type)
         case GFS_EXIT_TO_LEVEL: {
             int32_t next_level =
                 ((int32_t)(intptr_t)seq->data & ((1 << 6) - 1));
-            if (g_GameFlow.has_bonus && next_level >= g_GameFlow.first_bonus_num
+            if (g_GameFlow.levels[next_level].level_type == GFL_BONUS
                 && !g_GameInfo.bonus_level_unlock) {
                 return GF_EXIT_TO_TITLE;
             }
diff --git a/src/game/gameflow.h b/src/game/gameflow.h
index 65a3d9ad..2797008b 100644
--- a/src/game/gameflow.h
+++ b/src/game/gameflow.h
@@ -60,9 +60,6 @@ typedef struct GAMEFLOW {
     int32_t gym_level_num;
     int32_t first_level_num;
     int32_t last_level_num;
-    bool has_bonus;
-    int32_t first_bonus_num;
-    int32_t last_bonus_num;
     int32_t title_level_num;
     int32_t level_count;
     char *savegame_fmt_legacy;
diff --git a/src/game/savegame/savegame.c b/src/game/savegame/savegame.c
index 912f1d70..946e6427 100644
--- a/src/game/savegame/savegame.c
+++ b/src/game/savegame/savegame.c
@@ -644,10 +644,12 @@ GAMEFLOW_OPTION Savegame_PlayAvailableStory(int32_t slot_num)
 
         gf_option = GameFlow_StorySoFar(gf_param, savegame_info->level_num);
 
-        if ((gf_param >= savegame_info->level_num
-             && gf_param <= g_GameFlow.last_level_num)
-            || (gf_param >= savegame_info->level_num
-                && gf_param <= g_GameFlow.last_bonus_num)) {
+        if (g_GameFlow.levels[gf_param].level_type == GFL_BONUS) {
+            continue;
+        }
+
+        if (gf_param >= savegame_info->level_num
+                && gf_param <= g_GameFlow.last_level_num) {
             break;
         }
 
diff --git a/src/game/shell.c b/src/game/shell.c
index db2d3514..490bbe81 100644
--- a/src/game/shell.c
+++ b/src/game/shell.c
@@ -187,8 +187,7 @@ void Shell_Main(void)
 
         case GF_START_GAME: {
             GAMEFLOW_LEVEL_TYPE level_type = GFL_NORMAL;
-            if (g_GameFlow.has_bonus
-                && gf_param >= g_GameFlow.first_bonus_num) {
+            if (g_GameFlow.levels[gf_param].level_type == GFL_BONUS) {
                 level_type = GFL_BONUS;
             }
             gf_option = GameFlow_InterpretSequence(gf_param, level_type);
diff --git a/src/game/stats.c b/src/game/stats.c
index 1d08bc37..6f3b10c6 100644
--- a/src/game/stats.c
+++ b/src/game/stats.c
@@ -383,8 +383,7 @@ void Stats_Show(int32_t level_num)
 }
 
 void Stats_ShowTotal(
-    const char *filename, int first_level, int last_level,
-    GAME_STRING_ID heading)
+    const char *filename, GAMEFLOW_LEVEL_TYPE level_type)
 {
     char buf[100];
     char time_str[100];
@@ -401,7 +400,10 @@ void Stats_ShowTotal(
 
     int16_t secret_flags = 0;
 
-    for (int i = first_level; i <= last_level; i++) {
+    for (int i = 0; i < g_GameFlow.level_count; i++) {
+        if (g_GameFlow.levels[i].level_type != level_type) {
+            continue;
+        }
         const GAME_STATS *stats = &g_GameInfo.current[i].stats;
 
         total_timer += stats->timer;
@@ -505,7 +507,7 @@ void Stats_ShowTotal(
     Text_AddOutline(txt, true, TS_BACKGROUND);
 
     // heading
-    sprintf(buf, "%s", g_GameFlow.strings[heading]);
+    sprintf(buf, "%s", g_GameFlow.strings[level_type == GFL_BONUS ? GS_STATS_BONUS_STATISTICS : GS_STATS_FINAL_STATISTICS]);
     txt = Text_Create(0, top_y + 2, buf);
     Text_CentreH(txt, 1);
     Text_CentreV(txt, 1);
diff --git a/src/game/stats.h b/src/game/stats.h
index 7183061a..651ac3c4 100644
--- a/src/game/stats.h
+++ b/src/game/stats.h
@@ -11,6 +11,4 @@ int32_t Stats_GetPickups(void);
 int32_t Stats_GetKillables(void);
 int32_t Stats_GetSecrets(void);
 void Stats_Show(int32_t level_num);
-void Stats_ShowTotal(
-    const char *filename, int first_level, int last_level,
-    GAME_STRING_ID heading);
+void Stats_ShowTotal(const char *filename, GAMEFLOW_LEVEL_TYPE level_type);

[walkawayy]

@lahm86 would you mind posting your test gameflow if you have one off hand? I accidentally deleted mine. Everything looks good from the normal TR1 playthrough.

[lahm86]

@lahm86 would you mind posting your test gameflow if you have one off hand? I accidentally deleted mine. Everything looks good from the normal TR1 playthrough.

This one should hopefully do it.
https://github.com/LostArtefacts/Tomb1Main/files/12589324/Tomb1Main_gameflow.zip

[walkawayy]

I think it looks good except for story so far with a bonus level gets stuck in an infinite loop of the Gym FMV lol.

[walkawayy]

I don't think this is needed in Savegame_PlayAvailableStory?

        if (gf_direction == GF_EXIT_GAME) {
            break;
        }

[lahm86]

LGTM. I've been testing it out today and it all seems good; tried setting the gameflow up in different orders (from what I'd call standard to obscure) and everything carries over and flows perfectly. Story-so-far checks out, and save state seems good. Really nice feature 👍

[rr-]

LGTM ❤️