From 270279d4193d9a09b4525903f5f0471dd3d203fd Mon Sep 17 00:00:00 2001 From: Jeod <47716344+JeodC@users.noreply.github.com> Date: Sun, 2 Aug 2026 12:40:55 -0400 Subject: BK: add null terminator to dialogs that may be missing them (#247) --- src/factories/bk64/DialogFactory.cpp | 103 +++++++++++++++++++++++++---------- 1 file changed, 73 insertions(+), 30 deletions(-) diff --git a/src/factories/bk64/DialogFactory.cpp b/src/factories/bk64/DialogFactory.cpp index 4a1ef30..e4aa925 100644 --- a/src/factories/bk64/DialogFactory.cpp +++ b/src/factories/bk64/DialogFactory.cpp @@ -9,6 +9,8 @@ #define DIALOG_HEADER_1 0x01 #define DIALOG_HEADER_2 0x03 #define DIALOG_HEADER_3 0x00 +#define DIALOG_CMD_CLOSE 0x04 +#define DIALOG_TERMINATOR_SIZE 3 #define FORMAT_HEX(x, w) \ std::hex << std::uppercase << std::setfill('0') << std::setw(w) << x << std::nouppercase << std::dec @@ -166,44 +168,82 @@ ExportResult BK64::DialogModdingExporter::Export(std::ostream& write, std::share return std::nullopt; } -// One language: bottom box strings, then top box strings. -static DialogLang ParseLangBlock(LUS::BinaryReader& reader) { - DialogLang lang; +// Give empty dialogs the terminator the game expects +static void EnsureTerminator(std::vector& box) { + if (box.empty()) { + box.push_back({ DIALOG_CMD_CLOSE, std::string(1, '\0') }); + } +} + +static bool HasBytes(LUS::BinaryReader& reader, size_t count) { + return reader.GetBaseAddress() + count <= reader.GetLength(); +} + +// Read one box's entries. A count byte the blob can't back would otherwise walk the reader off the +// end, and whatever memory follows gets baked into the o2r as dialog text. Returns false once the +// blob is exhausted so the caller stops rather than parsing the rest from nothing. +static bool ReadEntries(LUS::BinaryReader& reader, uint8_t count, std::vector& box, + const std::string& symbol, const char* boxName) { + for (uint8_t i = 0; i < count; i++) { + // cmd + length, then the string itself + if (!HasBytes(reader, 2)) { + SPDLOG_WARN("[BK64] Dialog {}: {} entry {} of {} runs past the end of the blob; dropping the rest.", + symbol, boxName, i + 1, count); + return false; + } - auto bottomSize = reader.ReadUByte(); - for (uint8_t i = 0; i < bottomSize; i++) { DialogString dialogString; dialogString.cmd = reader.ReadUByte(); auto strLen = reader.ReadUByte(); + + if (!HasBytes(reader, strLen)) { + SPDLOG_WARN("[BK64] Dialog {}: {} entry {} of {} claims {} bytes the blob doesn't hold; dropping the rest.", + symbol, boxName, i + 1, count, strLen); + return false; + } + dialogString.str = reader.ReadString(strLen); - lang.bottom.push_back(dialogString); + box.push_back(dialogString); } + return true; +} - // Banjo's Backpack writes a 04 01 00 separator between bottom and top - // sections unconditionally, even when bottomSize is 0. When present, - // skip it so topSize reads correctly. Only check after an empty bottom - // to avoid false matches in vanilla dialogs. - if (bottomSize == 0) { - uint32_t savedPos = reader.GetBaseAddress(); - if (savedPos + 3 <= reader.GetLength()) { - uint8_t b0 = reader.ReadUByte(); - uint8_t b1 = reader.ReadUByte(); - uint8_t b2 = reader.ReadUByte(); - if (b0 != 0x04 || b1 != 0x01 || b2 != 0x00) { - reader.Seek(savedPos, LUS::SeekOffsetType::Start); - } +// One language: bottom box strings, then top box strings. +static DialogLang ParseLangBlock(LUS::BinaryReader& reader, const std::string& symbol) { + DialogLang lang; + + uint8_t bottomSize = 0; + bool bottomComplete = false; + + if (HasBytes(reader, 1)) { + bottomSize = reader.ReadUByte(); + bottomComplete = ReadEntries(reader, bottomSize, lang.bottom, symbol, "bottom"); + } else { + SPDLOG_WARN("[BK64] Dialog {}: blob ends before the bottom box count.", symbol); + } + + // Banjo's Backpack still writes the bottom box's terminator entry after a zero count. Consume + // it or it gets read as topSize (0x04), inventing four junk top entries and running off the + // blob; EnsureTerminator puts the entry back below. A real top box can't open with these bytes. + if (bottomComplete && bottomSize == 0 && HasBytes(reader, DIALOG_TERMINATOR_SIZE)) { + const uint32_t savedPos = reader.GetBaseAddress(); + const uint8_t b0 = reader.ReadUByte(); + const uint8_t b1 = reader.ReadUByte(); + const uint8_t b2 = reader.ReadUByte(); + if (b0 != DIALOG_CMD_CLOSE || b1 != 0x01 || b2 != 0x00) { + reader.Seek(savedPos, LUS::SeekOffsetType::Start); } } - auto topSize = reader.ReadUByte(); - for (uint8_t i = 0; i < topSize; i++) { - DialogString dialogString; - dialogString.cmd = reader.ReadUByte(); - auto strLen = reader.ReadUByte(); - dialogString.str = reader.ReadString(strLen); - lang.top.push_back(dialogString); + // A truncated bottom means every offset past it is guesswork, so don't invent a top from it. + if (bottomComplete && HasBytes(reader, 1)) { + const uint8_t topSize = reader.ReadUByte(); + ReadEntries(reader, topSize, lang.top, symbol, "top"); } + EnsureTerminator(lang.bottom); + EnsureTerminator(lang.top); + return lang; } @@ -219,7 +259,7 @@ std::optional> DialogFactory::parse(std::vector(std::move(lang.bottom), std::move(lang.top)); } @@ -230,13 +270,13 @@ std::optional> DialogFactory::parse(std::vector extraLangs; extraLangs.push_back(std::move(french)); @@ -286,6 +326,9 @@ std::optional> DialogFactory::parse_modding(std::ve top.push_back(dialogString); } + EnsureTerminator(bottom); + EnsureTerminator(top); + return std::make_shared(bottom, top); } -- cgit v1.2.3