address code review; rework requester mallocs and flags

src/game/console_cmd.c

@@ -709,7 +709,7 @@ static COMMAND_RESULT Console_Cmd_LoadGame(const char *args)
         return CR_FAILURE;
     }
 
-    if ((g_SavegameRequester.item_flags[slot_idx] & RIF_BLOCKED)) {
+    if (g_SavegameRequester.item_blocked[slot_idx]) {
         Console_Log(GS(OSD_LOAD_GAME_FAIL_UNAVAILABLE_SLOT), slot_num);
         return CR_FAILURE;
     }

src/game/option/option_passport.c

@@ -58,8 +58,8 @@ static REQUEST_INFO m_NewGameRequester = {
     .line_old_offset = 0,
     .pix_width = 162,
     .line_height = TEXT_HEIGHT + 7,
-    .flags = 0,
-    .item_flags = NULL,
+    .blockable = false,
+    .item_blocked = NULL,
     .x = 0,
     .y = 0,
     .heading_text = NULL,
@@ -77,8 +77,8 @@ static REQUEST_INFO m_SelectLevelRequester = {
     .line_old_offset = 0,
     .pix_width = 292,
     .line_height = TEXT_HEIGHT + 7,
-    .flags = 0,
-    .item_flags = NULL,
+    .blockable = false,
+    .item_blocked = NULL,
     .x = 0,
     .y = -32,
     .heading_text = NULL,
@@ -96,8 +96,8 @@ REQUEST_INFO g_SavegameRequester = {
     .line_old_offset = 0,
     .pix_width = 292,
     .line_height = TEXT_HEIGHT + 7,
-    .flags = 0,
-    .item_flags = NULL,
+    .blockable = false,
+    .item_blocked = NULL,
     .x = 0,
     .y = -32,
     .heading_text = NULL,
@@ -305,7 +305,7 @@ static void Option_PassportInitSaveRequester(int16_t page_num)
 static void Option_PassportInitSelectLevelRequester(void)
 {
     REQUEST_INFO *req = &m_SelectLevelRequester;
-    req->flags |= RIF_BLOCKABLE;
+    req->blockable = true;
     Requester_ClearTextstrings(req);
     Requester_SetHeading(req, GS(PASSPORT_SELECT_LEVEL));
 
@@ -336,10 +336,10 @@ static void Option_PassportInitNewGameRequester(void)
     REQUEST_INFO *req = &m_NewGameRequester;
     Requester_ClearTextstrings(req);
     Requester_SetHeading(req, GS(PASSPORT_SELECT_MODE));
-    Requester_AddItem(req, GS(PASSPORT_MODE_NEW_GAME), 0);
-    Requester_AddItem(req, GS(PASSPORT_MODE_NEW_GAME_PLUS), 0);
-    Requester_AddItem(req, GS(PASSPORT_MODE_NEW_GAME_JP), 0);
-    Requester_AddItem(req, GS(PASSPORT_MODE_NEW_GAME_JP_PLUS), 0);
+    Requester_AddItem(req, 0, "%s", GS(PASSPORT_MODE_NEW_GAME));
+    Requester_AddItem(req, 0, "%s", GS(PASSPORT_MODE_NEW_GAME_PLUS));
+    Requester_AddItem(req, 0, "%s", GS(PASSPORT_MODE_NEW_GAME_JP));
+    Requester_AddItem(req, 0, "%s", GS(PASSPORT_MODE_NEW_GAME_JP_PLUS));
     req->vis_lines = MAX_GAME_MODES;
 
     req->line_offset = 0;
@@ -403,7 +403,7 @@ static void Option_PassportShowSelectLevel(void)
 static void Option_PassportLoadGame(void)
 {
     Text_ChangeText(m_Text[TEXT_PAGE_NAME], GS(PASSPORT_LOAD_GAME));
-    g_SavegameRequester.flags |= RIF_BLOCKABLE;
+    g_SavegameRequester.blockable = true;
 
     if (m_PassportStatus.mode == PASSPORT_MODE_BROWSE) {
         if (g_InputDB.menu_confirm) {
@@ -413,9 +413,8 @@ static void Option_PassportLoadGame(void)
             m_PassportStatus.mode = PASSPORT_MODE_LOAD_GAME;
         }
     } else if (m_PassportStatus.mode == PASSPORT_MODE_LOAD_GAME) {
-        if (!(g_SavegameRequester.item_flags[g_SavegameRequester.requested]
-              & RIF_BLOCKED)
-            || !(g_SavegameRequester.flags & RIF_BLOCKABLE)) {
+        if (!g_SavegameRequester.item_blocked[g_SavegameRequester.requested]
+            || !g_SavegameRequester.blockable) {
             if (g_InputDB.menu_right) {
                 g_GameInfo.current_save_slot = g_SavegameRequester.requested;
                 Text_Hide(m_Text[TEXT_LEVEL_ARROW_RIGHT], true);
@@ -445,9 +444,8 @@ static void Option_PassportLoadGame(void)
             Text_Hide(m_Text[TEXT_LEVEL_ARROW_RIGHT], true);
         }
 
-        if ((g_SavegameRequester.item_flags[g_SavegameRequester.requested]
-             & RIF_BLOCKED)
-            && (g_SavegameRequester.flags & RIF_BLOCKABLE)) {
+        if (g_SavegameRequester.item_blocked[g_SavegameRequester.requested]
+            && g_SavegameRequester.blockable) {
             Text_Hide(m_Text[TEXT_LEVEL_ARROW_RIGHT], true);
         }
     } else if (m_PassportStatus.mode == PASSPORT_MODE_SELECT_LEVEL) {
@@ -485,7 +483,7 @@ static void Option_PassportSelectLevel(void)
 static void Option_PassportSaveGame(void)
 {
     Text_ChangeText(m_Text[TEXT_PAGE_NAME], GS(PASSPORT_SAVE_GAME));
-    g_SavegameRequester.flags &= ~RIF_BLOCKABLE;
+    g_SavegameRequester.blockable = false;
 
     if (m_PassportStatus.mode == PASSPORT_MODE_BROWSE) {
         if (g_InputDB.menu_confirm) {

src/game/phase/phase_pause.c

@@ -45,8 +45,8 @@ static REQUEST_INFO m_PauseRequester = {
     .line_old_offset = 0,
     .pix_width = 160,
     .line_height = TEXT_HEIGHT + 7,
-    .flags = 0,
-    .item_flags = NULL,
+    .blockable = false,
+    .item_blocked = NULL,
     .x = 0,
     .y = 0,
     .heading_text = NULL,
@@ -89,14 +89,15 @@ static int32_t Phase_Pause_DisplayRequester(
         Requester_SetSize(&m_PauseRequester, 2, -48);
         m_PauseRequester.requested = requested;
         Requester_SetHeading(&m_PauseRequester, header);
-        Requester_AddItem(&m_PauseRequester, option1, 0);
-        Requester_AddItem(&m_PauseRequester, option2, 0);
+        Requester_AddItem(&m_PauseRequester, 0, "%s", option1);
+        Requester_AddItem(&m_PauseRequester, 0, "%s", option2);
         m_IsTextReady = true;
         g_InputDB = (INPUT_STATE) { 0 };
         g_Input = (INPUT_STATE) { 0 };
     }
 
-    // Don't allow back because it breaks the requestor items.
+    // Don't allow menu_back because it clears the requester text.
+    // The player must use the pause requester options to quit or continue.
     if (g_InputDB.menu_back) {
         g_InputDB = (INPUT_STATE) { 0 };
         g_Input = (INPUT_STATE) { 0 };

src/game/requester.c

@@ -8,23 +8,20 @@
 
 #include <libtrx/memory.h>
 
+#include <stdarg.h>
 #include <stdbool.h>
 #include <stdio.h>
 #include <string.h>
 
 #define BOX_BORDER 2
 #define BOX_PADDING 10
 
-void Requester_Init(REQUEST_INFO *req, uint16_t max_items)
+void Requester_Init(REQUEST_INFO *req, const uint16_t max_items)
 {
     req->max_items = max_items;
     req->heading_text = Memory_Alloc(sizeof(char) * req->item_text_len);
-    req->item_flags = Memory_Alloc(sizeof(uint32_t) * max_items);
+    req->item_blocked = Memory_Alloc(sizeof(bool) * max_items);
     req->item_texts = Memory_Alloc(sizeof(char *) * max_items);
-    for (int i = 0; i < max_items; i++) {
-        req->item_texts[i] = Memory_Alloc(sizeof(char) * req->item_text_len);
-        req->item_flags[i] = 0;
-    }
     Requester_ClearTextstrings(req);
 }
 
@@ -33,7 +30,7 @@ void Requester_Shutdown(REQUEST_INFO *req)
     Requester_ClearTextstrings(req);
 
     Memory_FreePointer(&req->heading_text);
-    Memory_FreePointer(&req->item_flags);
+    Memory_FreePointer(&req->item_blocked);
     for (int i = 0; i < req->max_items; i++) {
         Memory_FreePointer(&req->item_texts[i]);
     }
@@ -168,8 +165,7 @@ int32_t Requester_Display(REQUEST_INFO *req)
     }
 
     if (g_InputDB.menu_confirm) {
-        if ((req->item_flags[req->requested] & RIF_BLOCKED)
-            && (req->flags & RIF_BLOCKABLE)) {
+        if (req->item_blocked[req->requested] && req->blockable) {
             g_Input = (INPUT_STATE) { 0 };
             return 0;
         } else {
@@ -189,27 +185,60 @@ void Requester_SetHeading(REQUEST_INFO *req, const char *string)
     Text_Remove(req->heading);
     req->heading = NULL;
 
-    size_t out_size = snprintf(NULL, 0, "%s", string) + 1;
+    const size_t out_size = snprintf(NULL, 0, "%s", string) + 1;
     snprintf(req->heading_text, out_size, "%s", string);
 }
 
 void Requester_ChangeItem(
-    REQUEST_INFO *req, int32_t idx, const char *string, uint16_t flag)
+    REQUEST_INFO *req, int32_t idx, bool blocked, const char *fmt, ...)
 {
-    if (idx > 0 && idx < req->max_items && string) {
-        size_t out_size = snprintf(NULL, 0, "%s", string) + 1;
-        snprintf(req->item_texts[idx], out_size, "%s", string);
-        req->item_flags[idx] = flag;
+    if (idx < 0 || idx >= req->max_items || !fmt) {
+        return;
+    }
+
+    if (req->item_texts[idx]) {
+        Memory_FreePointer(&req->item_texts[idx]);
+    }
+
+    va_list va;
+    va_start(va, fmt);
+    const size_t out_size = vsnprintf(NULL, 0, fmt, va) + 1;
+    req->item_texts[idx] = Memory_Alloc(sizeof(char) * out_size);
+    va_end(va);
+    Memory_FreePointer(&req->item_texts[idx]);
+
+    if (req->item_texts[idx] != NULL) {
+        va_start(va, fmt);
+        vsnprintf(req->item_texts[idx], out_size, fmt, va);
+        req->item_blocked[idx] = blocked;
+        va_end(va);
     }
 }
 
-void Requester_AddItem(REQUEST_INFO *req, const char *string, uint16_t flag)
+void Requester_AddItem(REQUEST_INFO *req, bool blocked, const char *fmt, ...)
 {
-    if (req->items_used < req->max_items && string) {
-        size_t out_size = snprintf(NULL, 0, "%s", string) + 1;
-        snprintf(req->item_texts[req->items_used], out_size, "%s", string);
-        req->item_flags[req->items_used] = flag;
+    if (req->items_used >= req->max_items || !fmt) {
+        return;
+    }
+
+    if (req->item_texts[req->items_used]) {
+        Memory_FreePointer(&req->item_texts[req->items_used]);
+    }
+
+    va_list va;
+    va_start(va, fmt);
+    const size_t out_size = vsnprintf(NULL, 0, fmt, va) + 1;
+    req->item_texts[req->items_used] = Memory_Alloc(sizeof(char) * out_size);
+    va_end(va);
+
+    if (req->item_texts[req->items_used] != NULL) {
+        va_start(va, fmt);
+        vsnprintf(req->item_texts[req->items_used], out_size, fmt, va);
+        req->item_blocked[req->items_used] = blocked;
         req->items_used++;
+        va_end(va);
+    } else {
+        Memory_FreePointer(&req->item_texts[req->items_used]);
     }
 }
 

src/game/requester.h

@@ -4,12 +4,12 @@
 
 #include <stdint.h>
 
-void Requester_Init(REQUEST_INFO *req, uint16_t num_items);
+void Requester_Init(REQUEST_INFO *req, const uint16_t num_items);
 void Requester_Shutdown(REQUEST_INFO *req);
 void Requester_ClearTextstrings(REQUEST_INFO *req);
 int32_t Requester_Display(REQUEST_INFO *req);
 void Requester_SetHeading(REQUEST_INFO *req, const char *string);
 void Requester_ChangeItem(
-    REQUEST_INFO *req, int32_t idx, const char *string, uint16_t flag);
-void Requester_AddItem(REQUEST_INFO *req, const char *string, uint16_t flag);
+    REQUEST_INFO *req, int32_t idx, bool blocked, const char *fmt, ...);
+void Requester_AddItem(REQUEST_INFO *req, bool blocked, const char *fmt, ...);
 void Requester_SetSize(REQUEST_INFO *req, int32_t max_lines, int16_t y);

src/game/savegame/savegame.c

@@ -432,14 +432,8 @@ bool Savegame_Save(int32_t slot_num, GAME_INFO *game_info)
 
     if (ret) {
         REQUEST_INFO *req = &g_SavegameRequester;
-        req->item_flags[slot_num] &= ~RIF_BLOCKED;
-        size_t out_size =
-            snprintf(
-                NULL, 0, "%s %d", g_GameFlow.levels[g_CurrentLevel].level_title,
-                g_SaveCounter)
-            + 1;
-        snprintf(
-            req->item_texts[slot_num], out_size, "%s %d",
+        Requester_ChangeItem(
+            req, slot_num, false, "%s %d",
             g_GameFlow.levels[g_CurrentLevel].level_title, g_SaveCounter);
     }
 
@@ -515,9 +509,7 @@ void Savegame_ScanSavedGames(void)
 
                 char *full_path =
                     Memory_Alloc(strlen(SAVES_DIR) + strlen(filename) + 2);
-                size_t out_size =
-                    snprintf(NULL, 0, "%s/%s", SAVES_DIR, filename) + 1;
-                snprintf(full_path, out_size, "%s/%s", SAVES_DIR, filename);
+                sprintf(full_path, "%s/%s", SAVES_DIR, filename);
 
                 MYFILE *fp = NULL;
                 if (!fp) {
@@ -553,8 +545,6 @@ void Savegame_ScanSavedGames(void)
 
     REQUEST_INFO *req = &g_SavegameRequester;
     Requester_ClearTextstrings(req);
-    char savegame_text[MAX_LEVEL_NAME_LENGTH] = { 0 };
-    uint16_t flags = 0;
 
     for (int i = 0; i < req->max_items; i++) {
         SAVEGAME_INFO *savegame_info = &m_SavegameInfo[i];
@@ -563,22 +553,12 @@ void Savegame_ScanSavedGames(void)
             if (savegame_info->counter == g_SaveCounter) {
                 m_NewestSlot = i;
             }
-
-            flags = req->item_flags[req->items_used] & ~RIF_BLOCKED;
-            size_t out_size = snprintf(
-                                  NULL, 0, "%s %d", savegame_info->level_title,
-                                  savegame_info->counter)
-                + 1;
-            snprintf(
-                savegame_text, out_size, "%s %d", savegame_info->level_title,
+            Requester_AddItem(
+                req, false, "%s %d", savegame_info->level_title,
                 savegame_info->counter);
         } else {
-            flags = req->item_flags[req->items_used] | RIF_BLOCKED;
-            size_t out_size =
-                snprintf(NULL, 0, GS(MISC_EMPTY_SLOT_FMT), i + 1) + 1;
-            snprintf(savegame_text, out_size, GS(MISC_EMPTY_SLOT_FMT), i + 1);
+            Requester_AddItem(req, true, GS(MISC_EMPTY_SLOT_FMT), i + 1);
         }
-        Requester_AddItem(req, savegame_text, flags);
     }
 
     if (req->requested >= req->vis_lines) {
@@ -594,48 +574,22 @@ void Savegame_ScanAvailableLevels(REQUEST_INFO *req)
 {
     SAVEGAME_INFO *savegame_info =
         &m_SavegameInfo[g_GameInfo.current_save_slot];
-    req->items_used = 0;
 
     if (!savegame_info->features.select_level) {
-        req->item_flags[req->items_used] |= RIF_BLOCKED;
-        size_t out_size =
-            snprintf(NULL, 0, "%s", GS(PASSPORT_LEGACY_SELECT_LEVEL_2)) + 1;
-        snprintf(
-            req->item_texts[0], out_size, "%s",
-            GS(PASSPORT_LEGACY_SELECT_LEVEL_1));
-        req->items_used++;
-
-        req->item_flags[req->items_used] |= RIF_BLOCKED;
-        out_size = snprintf(NULL, 0, "%s", GS(PASSPORT_LEGACY_SELECT_LEVEL_2));
-        snprintf(
-            req->item_texts[1], out_size, "%s",
-            GS(PASSPORT_LEGACY_SELECT_LEVEL_2));
-        req->items_used++;
+        Requester_AddItem(req, true, "%s", GS(PASSPORT_LEGACY_SELECT_LEVEL_1));
+        Requester_AddItem(req, true, "%s", GS(PASSPORT_LEGACY_SELECT_LEVEL_2));
         req->requested = 0;
         req->line_offset = 0;
         return;
     }
 
-    int story_so_far_pos = 0;
     for (int i = g_GameFlow.first_level_num; i <= savegame_info->level_num;
          i++) {
-        req->item_flags[req->items_used] &= ~RIF_BLOCKED;
-        size_t out_size =
-            snprintf(NULL, 0, "%s", g_GameFlow.levels[i].level_title) + 1;
-        snprintf(
-            req->item_texts[req->items_used], out_size, "%s",
-            g_GameFlow.levels[i].level_title);
-        req->items_used++;
+        Requester_AddItem(req, false, "%s", g_GameFlow.levels[i].level_title);
     }
 
     if (g_InvMode == INV_TITLE_MODE) {
-        req->item_flags[req->items_used] &= ~RIF_BLOCKED;
-        size_t out_size =
-            snprintf(NULL, 0, "%s", GS(PASSPORT_STORY_SO_FAR)) + 1;
-        snprintf(
-            req->item_texts[req->items_used], out_size, "%s",
-            GS(PASSPORT_STORY_SO_FAR));
-        req->items_used++;
+        Requester_AddItem(req, false, "%s", GS(PASSPORT_STORY_SO_FAR));
     }
 
     req->requested = 0;

src/global/types.h

@@ -1836,8 +1836,8 @@ typedef struct REQUEST_INFO {
     uint16_t line_old_offset;
     uint16_t pix_width;
     uint16_t line_height;
-    uint16_t flags;
-    uint32_t *item_flags;
+    bool blockable;
+    bool *item_blocked;
     int16_t x;
     int16_t y;
     char *heading_text;