Address review comments

7 files changed, 74 insertions, 41 deletions

diff --git a/include/constants/global.h b/include/constants/global.h
index 77ef6f203..7f3c80bb4 100644
--- a/include/constants/global.h
+++ b/include/constants/global.h
@@ -38,6 +38,10 @@
 #define FEMALE 1
 #define GENDER_COUNT 2
 
+#define BARD_SONG_LENGTH       6
+#define NUM_STORYTELLER_TALES  4
+#define NUM_TRADER_ITEMS       4
+
 #define OPTIONS_BUTTON_MODE_HELP         0
 #define OPTIONS_BUTTON_MODE_LR           1
 #define OPTIONS_BUTTON_MODE_L_EQUALS_A   2
@@ -62,6 +66,7 @@
 
 #define MAX_MON_MOVES 4
 
+#define TRAINER_ID_LENGTH 4
 #define PARTY_SIZE 6
 #define MULTI_PARTY_SIZE (PARTY_SIZE / 2)
 
diff --git a/include/global.h b/include/global.h
index 9f46c2eee..2f87a0c29 100644
--- a/include/global.h
+++ b/include/global.h
@@ -312,7 +312,7 @@ struct SecretBaseRecord
     /*0x1A9D*/ u8 gender:1;
     /*0x1A9D*/ u8 sbr_field_1_5:1;
     /*0x1A9D*/ u8 sbr_field_1_6:2;
-    /*0x1A9E*/ u8 trainerName[7]; // TODO: Change PLAYER_NAME_LENGTH + 1 to 7
+    /*0x1A9E*/ u8 trainerName[PLAYER_NAME_LENGTH];
     /*0x1AA5*/ u8 trainerId[4]; // byte 0 is used for determining trainer class
     /*0x1AA9*/ u8 language;
     /*0x1AAA*/ u16 sbr_field_e;
@@ -398,41 +398,68 @@ struct MailStruct
     /*0x20*/ u16 itemId;
 };
 
-struct UnkMauvilleOldManStruct
+struct MauvilleManCommon
 {
-    u8 unk_2D94;
-    u8 unk_2D95;
-    /*0x2D96*/ u16 mauvilleOldMan_ecArray[6];
-    /*0x2DA2*/ u16 mauvilleOldMan_ecArray2[6];
-    /*0x2DAE*/ u8 playerName[PLAYER_NAME_LENGTH + 1 + 1];
-    /*0x2DB6*/ u8 filler_2DB6[0x3];
-    /*0x2DB9*/ u8 playerTrainerId[4];
-    u8 unk_2DBD;
+    u8 id;
+};
+
+struct MauvilleManBard
+{
+    /*0x00*/ u8 id;
+    /*0x02*/ u16 songLyrics[BARD_SONG_LENGTH];
+    /*0x0E*/ u16 temporaryLyrics[BARD_SONG_LENGTH];
+    /*0x1A*/ u8 playerName[PLAYER_NAME_LENGTH + 1];
+    /*0x22*/ u8 filler_2DB6[0x3];
+    /*0x25*/ u8 playerTrainerId[TRAINER_ID_LENGTH];
+    /*0x29*/ bool8 hasChangedSong;
+    /*0x2A*/ u8 language;
 }; /*size = 0x2C*/
 
-struct UnkMauvilleOldManStruct2
+struct MauvilleManStoryteller
+{
+    u8 id;
+    bool8 alreadyRecorded;
+    u8 filler2[2];
+    u8 gameStatIDs[NUM_STORYTELLER_TALES];
+    u8 trainerNames[NUM_STORYTELLER_TALES][PLAYER_NAME_LENGTH];
+    u8 statValues[NUM_STORYTELLER_TALES][4];
+    u8 language[NUM_STORYTELLER_TALES];
+};
+
+struct MauvilleManGiddy
 {
-    u8 filler0;
-    u8 unk1;
-    u8 unk2;
-    u16 mauvilleOldMan_ecArray[10];
-    u8 mauvilleOldMan_ecArray2[12];
-    u8 fillerF[0x2];
+    /*0x00*/ u8 id;
+    /*0x01*/ u8 taleCounter;
+    /*0x02*/ u8 questionNum;
+    /*0x04*/ u16 randomWords[10];
+    /*0x18*/ u8 questionList[8];
+    /*0x20*/ u8 language;
 }; /*size = 0x2C*/
 
+struct MauvilleManHipster
+{
+    u8 id;
+    bool8 alreadySpoken;
+    u8 language;
+};
+
 struct MauvilleOldManTrader
 {
-    u8 unk0;
-    u8 unk1[4];
-    u8 unk5[4][11];
-    u8 unk31;
+    u8 id;
+    u8 decorIds[NUM_TRADER_ITEMS];
+    u8 playerNames[NUM_TRADER_ITEMS][11];
+    u8 alreadyTraded;
+    u8 language[NUM_TRADER_ITEMS];
 };
 
 typedef union OldMan
 {
-    struct UnkMauvilleOldManStruct oldMan1;
-    struct UnkMauvilleOldManStruct2 oldMan2;
+    struct MauvilleManCommon common;
+    struct MauvilleManBard bard;
+    struct MauvilleManGiddy giddy;
+    struct MauvilleManHipster hipster;
     struct MauvilleOldManTrader trader;
+    struct MauvilleManStoryteller storyteller;
     u8 filler[0x40];
 } OldMan;
 
diff --git a/src/daycare.c b/src/daycare.c
index ef9da89fe..277a382c7 100644
--- a/src/daycare.c
+++ b/src/daycare.c
@@ -614,7 +614,7 @@ static void ClearDaycareMonMail(struct DayCareMail *mail)
 {
     s32 i;
 
-    for (i = 0; i < PLAYER_NAME_LENGTH + 1/* + 1*/; i++)
+    for (i = 0; i < PLAYER_NAME_LENGTH + 1; i++)
         mail->OT_name[i] = 0;
     for (i = 0; i < POKEMON_NAME_LENGTH + 1; i++)
         mail->monName[i] = 0;
diff --git a/src/pokemon.c b/src/pokemon.c
index 130709598..09e2ed998 100644
--- a/src/pokemon.c
+++ b/src/pokemon.c
@@ -5769,7 +5769,7 @@ s8 GetFlavorRelationByPersonality(u32 personality, u8 flavor)
 
 bool8 IsTradedMon(struct Pokemon *mon)
 {
-    u8 otName[7 + 1]; // change PLAYER_NAME_LENGTH + 1 to 7
+    u8 otName[PLAYER_NAME_LENGTH];
     u32 otId;
     GetMonData(mon, MON_DATA_OT_NAME, otName);
     otId = GetMonData(mon, MON_DATA_OT_ID, 0);
diff --git a/src/union_room_chat.c b/src/union_room_chat.c
index f4a192b35..d672c6b68 100644
--- a/src/union_room_chat.c
+++ b/src/union_room_chat.c
@@ -405,7 +405,7 @@ static void ChatEntryRoutine_HandleInput(void)
 static void ChatEntryRoutine_Switch(void)
 {
     s16 input;
-    int shouldSwitchPages;
+    bool32 shouldSwitchPages;
 
     switch (sWork->routineState)
     {
@@ -423,9 +423,9 @@ static void ChatEntryRoutine_Switch(void)
         {
         default:
             UnionRoomChat_StartDisplaySubtask(CHATDISPLAYROUTINE_HIDEKBSWAPMENU, 0);
-            shouldSwitchPages = 1;
+            shouldSwitchPages = TRUE;
             if (sWork->currentPage == input || input > UNION_ROOM_KB_PAGE_COUNT)
-                shouldSwitchPages = 0;
+                shouldSwitchPages = FALSE;
             break;
         case MENU_NOTHING_CHOSEN:
             if (JOY_NEW(SELECT_BUTTON))
diff --git a/src/union_room_chat_display.c b/src/union_room_chat_display.c
index f43e40368..54aef8d65 100644
--- a/src/union_room_chat_display.c
+++ b/src/union_room_chat_display.c
@@ -600,7 +600,7 @@ static bool32 DisplaySubtask_UpdateMessageBuffer(u8 *state)
         UnionRoomChat_GetBufferSelectionRegion(&start, &length);
         FillWin1Rect(start, length, PIXEL_FILL(0));
         str = UnionRoomChat_GetMessageEntryBuffer();
-        PrintOnWin1Parameterized(0, str, 3, 1, 2);
+        PrintOnWin1Parameterized(0, str, TEXT_COLOR_LIGHT_GREY, TEXT_COLOR_WHITE, TEXT_COLOR_DARK_GREY);
         CopyWindowToVram(1, 2);
         break;
     case 1:
@@ -629,7 +629,7 @@ static bool32 DisplaySubtask_PrintRegisterWhere(u8 *state)
         str = UnionRoomChat_GetEndOfMessageEntryBuffer();
         length = StringLength_Multibyte(str);
         FillWin1Rect(var0, length, PIXEL_FILL(6));
-        PrintOnWin1Parameterized(var0, str, 0, 4, 5);
+        PrintOnWin1Parameterized(var0, str, TEXT_COLOR_TRANSPARENT, TEXT_COLOR_RED, TEXT_COLOR_LIGHT_RED);
         CopyWindowToVram(1, 2);
         break;
     case 1:
@@ -670,7 +670,7 @@ static bool32 DisplaySubtask_CancelRegister(u8 *state)
         str = UnionRoomChat_GetEndOfMessageEntryBuffer();
         length = StringLength_Multibyte(str);
         FillWin1Rect(x, length, PIXEL_FILL(0));
-        PrintOnWin1Parameterized(x, str, 3, 1, 2);
+        PrintOnWin1Parameterized(x, str, TEXT_COLOR_LIGHT_GREY, TEXT_COLOR_WHITE, TEXT_COLOR_DARK_GREY);
         CopyWindowToVram(1, 2);
         break;
     case 1:
@@ -1080,7 +1080,7 @@ static void PrintOnWin1Parameterized(u16 x, u8 *str, u8 bgColor, u8 fgColor, u8
     u8 color[3];
     u8 strbuf[35];
 
-    if (bgColor != 0)
+    if (bgColor != TEXT_COLOR_TRANSPARENT)
         FillWin1Rect(x, UnionRoomChat_GetMessageEntryCursorPosition() - x, bgColor);
 
     color[0] = bgColor;
@@ -1105,9 +1105,9 @@ static void PrintCurrentKeyboardPage(void)
 
     FillWindowPixelBuffer(2, PIXEL_FILL(15));
     page = GetCurrentKeyboardPage();
-    color[0] = 0;
-    color[1] = 14;
-    color[2] = 13;
+    color[0] = TEXT_COLOR_TRANSPARENT;
+    color[1] = TEXT_DYNAMIC_COLOR_5;
+    color[2] = TEXT_DYNAMIC_COLOR_4;
     if (page != UNION_ROOM_KB_PAGE_COUNT)
     {
         str[0] = EXT_CTRL_CODE_BEGIN;
@@ -1207,9 +1207,10 @@ static void ClearWin3(void)
 }
 
 static void PrintTextOnWin0Colorized(u16 row, u8 *str, u8 colorIdx)
+// colorIdx: 0 = grey, 1 = red, 2 = green, 3 = blue
 {
     u8 color[3];
-    color[0] = 1;
+    color[0] = TEXT_COLOR_WHITE;
     color[1] = colorIdx * 2 + 2;
     color[2] = colorIdx * 2 + 3;
     FillWindowPixelRect(0, PIXEL_FILL(1), 0, row * 15, 168, 15);
diff --git a/src/union_room_chat_objects.c b/src/union_room_chat_objects.c
index e5b739675..f97136a0a 100644
--- a/src/union_room_chat_objects.c
+++ b/src/union_room_chat_objects.c
@@ -162,13 +162,13 @@ static const struct SpriteTemplate sSpriteTemplate_UnionRoomChatIcons = {
 
 bool32 UnionRoomChat_TryAllocSpriteWork(void)
 {
-    u32 i;
-    for (i = 0; i < 5; i++)
+    int i;
+    for (i = 0; i < NELEMS(sSpriteSheets); i++)
         LoadCompressedSpriteSheet(&sSpriteSheets[i]);
 
     LoadSpritePalette(&sSpritePalette);
-    sWork = Alloc(0x18);
-    if (!sWork)
+    sWork = Alloc(sizeof(struct UnionRoomChat3));
+    if (sWork == NULL)
         return FALSE;
 
     return TRUE;
@@ -176,7 +176,7 @@ bool32 UnionRoomChat_TryAllocSpriteWork(void)
 
 void UnionRoomChat_FreeSpriteWork(void)
 {
-    if (sWork)
+    if (sWork != NULL)
         Free(sWork);
 }