From b4a9462d9277f0824c4a7f95c353d32d39c11556 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 18 Oct 2026 13:40:18 +0200 Subject: [PATCH 1/2] CwCheat: Ignore invisible characters, and show parse errors - Fixes #14955 Cheats copied from websites can carry invisible Unicode characters, like a pop directional formatting mark after each address. That broke the hex parse of every line, and a cheat with no valid lines didn't show up at all. The parser now drops such characters (a new SanitizeString mode), and the cheat screen shows the first few parse errors instead of silently leaving cheats out. Co-Authored-By: Claude Opus 5.5 (1M context) --- Common/StringUtils.cpp | 52 +++++++++++++++++++++++++++++----------- Common/StringUtils.h | 1 + Core/CwCheat.cpp | 12 ++++++---- Core/CwCheat.h | 3 ++- UI/CwCheatScreen.cpp | 19 ++++++++++++++- UI/CwCheatScreen.h | 1 + unittest/TestCwCheat.cpp | 35 ++++++++++++++++++++++++++- 7 files changed, 101 insertions(+), 22 deletions(-) diff --git a/Common/StringUtils.cpp b/Common/StringUtils.cpp index 23bb1626a3..4ec97b7171 100644 --- a/Common/StringUtils.cpp +++ b/Common/StringUtils.cpp @@ -38,6 +38,7 @@ #include #include "Common/Buffer.h" +#include "Common/Data/Encoding/Utf8.h" #include "Common/StringUtils.h" size_t truncate_cpy(char *dest, size_t destSize, const char *src) { @@ -105,9 +106,44 @@ int CountChar(std::string_view haystack, char needle) { return count; } +static void ApplyLengthLimits(std::string *str, int minLength, int maxLength) { + if (minLength > 0) { + if ((int)str->size() < minLength) { + // Just reject it by returning an empty string, as we can't really + // conjure up new characters here. + str->clear(); + return; + } + } + + if (maxLength >= 0) { + // TODO: Cut at whole UTF-8 chars! + if ((int)str->size() > maxLength) { + str->resize(maxLength); + } + } +} + std::string SanitizeString(std::string_view input, StringRestriction restriction, int minLength, int maxLength) { if (restriction == StringRestriction::None) { return std::string(input); + } else if (restriction == StringRestriction::NoInvisibleFormatting) { + std::string out; + out.reserve(input.size()); + UTF8 utf(input); + while (!utf.end()) { + const int start = utf.byteIndex(); + const uint32_t c = utf.next(); + if (c == 0x00A0 || c == 0x202F) { + out.push_back(' '); + } else if (c == 0x00AD || (c >= 0x200B && c <= 0x200F) || (c >= 0x202A && c <= 0x202E) || (c >= 0x2060 && c <= 0x206F) || c == 0xFEFF) { + // Soft hyphen, zero-width characters, direction marks, embeddings and isolates, BOM. + } else { + out.append(input.data() + start, utf.byteIndex() - start); + } + } + ApplyLengthLimits(&out, minLength, maxLength); + return out; } // First, remove any chars not in A-Za-z0-9_-. This will effectively get rid of any Unicode char, emojis etc too. std::string sanitized; @@ -116,6 +152,7 @@ std::string SanitizeString(std::string_view input, StringRestriction restriction for (char c : input) { switch (restriction) { case StringRestriction::None: + case StringRestriction::NoInvisibleFormatting: sanitized.push_back(c); break; case StringRestriction::AlphaNumDashUnderscore: @@ -155,20 +192,7 @@ std::string SanitizeString(std::string_view input, StringRestriction restriction } } - if (minLength > 0) { - if ((int)sanitized.size() < minLength) { - // Just reject it by returning an empty string, as we can't really - // conjure up new characters here. - return std::string(); - } - } - - if (maxLength >= 0) { - // TODO: Cut at whole UTF-8 chars! - if ((int)sanitized.size() > maxLength) { - sanitized.resize(maxLength); - } - } + ApplyLengthLimits(&sanitized, minLength, maxLength); if (restriction == StringRestriction::NoLineBreaksOrSpecials) { // Additionally, cut off the string if we find an overlong UTF-8 character, such as in Jak & Daxter's title. diff --git a/Common/StringUtils.h b/Common/StringUtils.h index 45e6f51805..fc3c422d22 100644 --- a/Common/StringUtils.h +++ b/Common/StringUtils.h @@ -91,6 +91,7 @@ enum class StringRestriction { FileName, NoLineBreaksOrSpecials, // Used for savedata UI. Removes line breaks, backslashes and similar. ConvertToUnixEndings, + NoInvisibleFormatting, // Drops zero-width characters, direction marks and similar, turns no-break spaces into spaces. }; std::string SanitizeString(std::string_view username, StringRestriction restriction, int minLength = 0, int maxLength = -1); diff --git a/Core/CwCheat.cpp b/Core/CwCheat.cpp index 3be8712a7d..96bb8fa6d9 100644 --- a/Core/CwCheat.cpp +++ b/Core/CwCheat.cpp @@ -68,10 +68,9 @@ bool CheatFileParser::Parse() { if (!tempLine) continue; - // Detect UTF-8 BOM sequence, and ignore it. - if (lineNumber == 1 && memcmp(tempLine, "\xEF\xBB\xBF", 3) == 0) - tempLine += 3; - std::string line = TrimString(tempLine); + // Cheats copied from websites can carry invisible characters (like direction marks) that would + // break the hex values. This also drops the UTF-8 BOM. + std::string line = TrimString(SanitizeString(tempLine, StringRestriction::NoInvisibleFormatting)); // Minimum length 5 is shortest possible _ lines name of the game "_G N+" // and a minimum of 1 displayable character in cheat name string "_C0 1" @@ -351,9 +350,12 @@ void CWCheatEngine::ParseCheats() { cheats_ = parser.GetCheats(); } -std::vector CWCheatEngine::FileInfo() const { +std::vector CWCheatEngine::FileInfo(std::vector *errors) const { CheatFileParser parser(filename_, gameID_); parser.Parse(); + if (errors) { + *errors = parser.GetErrors(); + } return parser.GetFileInfo(); } diff --git a/Core/CwCheat.h b/Core/CwCheat.h index e8e82b7069..4d56915980 100644 --- a/Core/CwCheat.h +++ b/Core/CwCheat.h @@ -57,7 +57,8 @@ struct CheatOperation; class CWCheatEngine { public: CWCheatEngine(std::string_view gameID); - std::vector FileInfo() const; + // errors, if given, gets the lines that didn't parse. + std::vector FileInfo(std::vector *errors = nullptr) const; void ParseCheats(); void CreateCheatFile(); const Path &CheatFilename() const { diff --git a/UI/CwCheatScreen.cpp b/UI/CwCheatScreen.cpp index bab863f56a..e2b68bd9a7 100644 --- a/UI/CwCheatScreen.cpp +++ b/UI/CwCheatScreen.cpp @@ -183,7 +183,7 @@ bool CwCheatScreen::TryLoadCheatInfo() { } fileCheckCounter_ = 0; - fileInfo_ = engine_->FileInfo(); + fileInfo_ = engine_->FileInfo(&parseErrors_); // Let's also trigger a reload, in case it changed. g_Config.bReloadCheats = true; @@ -286,6 +286,23 @@ void CwCheatScreen::CreateContentViews(UI::ViewGroup *parent) { rightColumn->Add(new Spacer(8.0f)); } + if (!parseErrors_.empty()) { + // Cheats with a bad line are left out of the list, so say why. + const size_t maxShown = 5; + std::string details; + for (size_t i = 0; i < parseErrors_.size() && i < maxShown; i++) { + if (i > 0) { + details += '\n'; + } + details += parseErrors_[i]; + } + if (parseErrors_.size() > maxShown) { + details += "\n..."; + } + rightColumn->Add(new NoticeView(NoticeLevel::WARN, cw->T("Some lines in the cheat file could not be read"), details)); + rightColumn->Add(new Spacer(8.0f)); + } + rightColumn->Add(new ItemHeader(cw->T("Cheats"))); bool prevIsTitle = false; diff --git a/UI/CwCheatScreen.h b/UI/CwCheatScreen.h index 84e2525bdc..e24e407a27 100644 --- a/UI/CwCheatScreen.h +++ b/UI/CwCheatScreen.h @@ -73,6 +73,7 @@ private: CWCheatEngine *engine_ = nullptr; std::vector fileInfo_; + std::vector parseErrors_; std::string gameID_; int fileCheckCounter_ = 0; uint64_t fileCheckHash_ = 0; diff --git a/unittest/TestCwCheat.cpp b/unittest/TestCwCheat.cpp index 54c6490a7e..36013258ac 100644 --- a/unittest/TestCwCheat.cpp +++ b/unittest/TestCwCheat.cpp @@ -21,6 +21,7 @@ #include "Common/File/FileUtil.h" #include "Common/File/Path.h" +#include "Common/StringUtils.h" #include "Core/CwCheat.h" #include "Core/MemMap.h" #include "Core/MemMapHelpers.h" @@ -213,6 +214,38 @@ static bool TestCheatParseErrors() { return true; } +// Cheats pasted from websites, with a BOM, no _S line and a U+202C (pop directional formatting) +// after every address (#14955). +static bool TestCheatInvisibleCharacters() { + const Path filename = UnitTestTempPath("unittest_cheats_invisible.ini"); + const char *content = "\xEF\xBB\xBF\n_C1 Blade Change\xE2\x80\xAC\n_L 0x002F614A\xE2\x80\xAC 0x0000003D\n_L\xC2\xA0" "0x002F614B 0x00000018\n"; + if (!File::WriteStringToFile(true, content, filename)) { + return false; + } + bool parsed; + std::vector cheats; + std::vector info; + { + CheatFileParser parser(filename, "TEST00000"); + parsed = parser.Parse(); + cheats = parser.GetCheats(); + info = parser.GetFileInfo(); + } + File::Delete(filename); + EXPECT_TRUE(parsed); + EXPECT_EQ_INT((int)cheats.size(), 1); + EXPECT_EQ_INT((int)cheats[0].lines.size(), 2); + EXPECT_EQ_HEX(cheats[0].lines[0].part1, 0x002F614A); + EXPECT_EQ_HEX(cheats[0].lines[1].part2, 0x00000018); + EXPECT_EQ_INT((int)info.size(), 1); + EXPECT_EQ_STR(info[0].name, std::string("Blade Change")); + + // The length limits still apply, after the characters are dropped. + EXPECT_EQ_STR(SanitizeString("a\xE2\x80\x8B" "bcd", StringRestriction::NoInvisibleFormatting, 0, 3), std::string("abc")); + EXPECT_EQ_STR(SanitizeString("a\xE2\x80\x8B" "b", StringRestriction::NoInvisibleFormatting, 3), std::string()); + return true; +} + bool TestCwCheat() { // Other tests may leave currentMIPS pointing elsewhere; the cheat engine needs the real one. currentMIPS = &mipsr4k; @@ -221,7 +254,7 @@ bool TestCwCheat() { printf("Memory::Init failed\n"); return false; } - const bool success = RunCheatTests() && TestCheatParseErrors(); + const bool success = RunCheatTests() && TestCheatParseErrors() && TestCheatInvisibleCharacters(); Memory::Shutdown(); return success; } From ba127e2a43fad5dedb9e2fc7c14c36409a3b9f31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 18 Oct 2026 13:40:18 +0200 Subject: [PATCH 2/2] AGENTS.md: Prefer an early return to a large else block Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 2165cc6c32..c5646ae265 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -357,6 +357,10 @@ private: But generally follow the surrounding style. Braces are preferred on the same line. Braces are always used even when they could be omitted due the inner part being just a single line. +If a large `else {}` block can be avoided with an early return when writing new code, do that +instead. Don't wrap existing code in a new `else` and reindent it, either - it makes the diff +much bigger than the change. + ### Comments Keep comments tight. Say the thing once; don't restate what the code already shows, and don't