From 59c746dc42b482923019de35a1b903fc15f063c7 Mon Sep 17 00:00:00 2001 From: walkawayy <81546780+walkawayy@users.noreply.github.com> Date: Thu, 27 Jun 2024 15:28:35 -0400 Subject: [PATCH] address code review; rework requester mallocs and flags --- src/game/console_cmd.c | 2 +- src/game/option/option_passport.c | 36 ++++++++-------- src/game/phase/phase_pause.c | 11 ++--- src/game/requester.c | 69 ++++++++++++++++++++++--------- src/game/requester.h | 6 +-- src/game/savegame/savegame.c | 66 +++++------------------------ src/global/types.h | 4 +- 7 files changed, 88 insertions(+), 106 deletions(-) diff --git a/src/game/console_cmd.c b/src/game/console_cmd.c index 8cf572e74..afc30e9d4 100644 --- a/src/game/console_cmd.c +++ b/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; } diff --git a/src/game/option/option_passport.c b/src/game/option/option_passport.c index 3448587aa..71699097a 100644 --- a/src/game/option/option_passport.c +++ b/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) { diff --git a/src/game/phase/phase_pause.c b/src/game/phase/phase_pause.c index a1f03d894..250907f43 100644 --- a/src/game/phase/phase_pause.c +++ b/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 }; diff --git a/src/game/requester.c b/src/game/requester.c index 35c2fb2cd..cd618f1b2 100644 --- a/src/game/requester.c +++ b/src/game/requester.c @@ -8,6 +8,7 @@ #include +#include #include #include #include @@ -15,16 +16,12 @@ #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]); } } diff --git a/src/game/requester.h b/src/game/requester.h index 274b3849f..76b7ae263 100644 --- a/src/game/requester.h +++ b/src/game/requester.h @@ -4,12 +4,12 @@ #include -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); diff --git a/src/game/savegame/savegame.c b/src/game/savegame/savegame.c index 8c761bce6..4e24527d8 100644 --- a/src/game/savegame/savegame.c +++ b/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; diff --git a/src/global/types.h b/src/global/types.h index c426141c0..170ee8ee1 100644 --- a/src/global/types.h +++ b/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;