diff --git a/AGENTS.md b/AGENTS.md index 0e0d84db7e..5eb8a69160 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -358,6 +358,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 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; }