Mirror/ppsspp
1
0 Fork 0
mirror of https://github.com/hrydgard/ppsspp.git synced 2026-10-01 14:58:14 +00:00
Code Issues Packages Projects Releases Wiki Activity

Path: make WithReplacedExtension(old, new) report a mismatch instead of hiding it

Browse Source 
It used to return the path unchanged when the path didn't end in oldExtension,
so a caller that guessed wrong silently went on using the original file - and
"the screenshot next to this savestate" quietly becomes "this savestate".
Every caller had to know to check the extension first, and most didn't.

Now it's [[nodiscard]] bool with an out-param, in the style of ComputePathTo
next door, so the mismatch has to be handled. Changing the signature rather
than the behaviour means no call site can keep the old assumption by accident.
All four callers wanted "skip it" or "fall back", which they now say out loud.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This commit is contained in:
Henrik RydgårdandClaude Opus 5 committed 2026-09-12 11:54:11 -06:00
1 parent 90a85e235b
commit 4ae682283c
8 files changed
+57 -28
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

Common/File/AndroidContentURI.cpp
+7 -5
View File
@@ -82,14 +82,16 @@ AndroidContentURI AndroidContentURI::WithExtraExtension(std::string_view extensi
return uri;
}
AndroidContentURI AndroidContentURI::WithReplacedExtension(const std::string &oldExtension, const std::string &newExtension) const {
bool AndroidContentURI::WithReplacedExtension(const std::string &oldExtension, const std::string &newExtension, AndroidContentURI *out) const {
_dbg_assert_(!oldExtension.empty() && oldExtension[0] == '.');
_dbg_assert_(!newExtension.empty() && newExtension[0] == '.');
AndroidContentURI uri = *this;
if (endsWithNoCase(file, oldExtension)) {
uri.file = file.substr(0, file.size() - oldExtension.size()) + newExtension;
if (!endsWithNoCase(file, oldExtension)) {
return false;
}
return uri;
AndroidContentURI uri = *this;
uri.file = file.substr(0, file.size() - oldExtension.size()) + newExtension;
*out = uri;
return true;
}
AndroidContentURI AndroidContentURI::WithReplacedExtension(const std::string &newExtension) const {
Common/File/AndroidContentURI.h
+2 -1
View File
@@ -32,7 +32,8 @@ public:
AndroidContentURI WithRootFilePath(const std::string &filePath);
AndroidContentURI WithComponent(std::string_view filePath);
AndroidContentURI WithExtraExtension(std::string_view extension); // The ext string contains the dot.
AndroidContentURI WithReplacedExtension(const std::string &oldExtension, const std::string &newExtension) const;
// False, leaving *out alone, if the file doesn't end in oldExtension. See Path's version.
[[nodiscard]] bool WithReplacedExtension(const std::string &oldExtension, const std::string &newExtension, AndroidContentURI *out) const;
AndroidContentURI WithReplacedExtension(const std::string &newExtension) const;
bool CanNavigateUp() const;
Common/File/Path.cpp
+16 -11
View File
@@ -119,20 +119,25 @@ Path Path::WithExtraExtension(std::string_view ext) const {
return Path(path_ + std::string(ext));
}
Path Path::WithReplacedExtension(const std::string &oldExtension, const std::string &newExtension) const {
if (type_ == PathType::CONTENT_URI) {
AndroidContentURI uri(path_);
return Path(uri.WithReplacedExtension(oldExtension, newExtension).ToString());
}
bool Path::WithReplacedExtension(const std::string &oldExtension, const std::string &newExtension, Path *out) const {
_dbg_assert_(!oldExtension.empty() && oldExtension[0] == '.');
_dbg_assert_(!newExtension.empty() && newExtension[0] == '.');
if (endsWithNoCase(path_, oldExtension)) {
std::string newPath = path_.substr(0, path_.size() - oldExtension.size());
return Path(newPath + newExtension);
} else {
return Path(*this);
if (type_ == PathType::CONTENT_URI) {
AndroidContentURI uri(path_);
AndroidContentURI replaced;
if (!uri.WithReplacedExtension(oldExtension, newExtension, &replaced)) {
return false;
}
*out = Path(replaced.ToString());
return true;
}
if (!endsWithNoCase(path_, oldExtension)) {
return false;
}
*out = Path(path_.substr(0, path_.size() - oldExtension.size()) + newExtension);
return true;
}
Path Path::WithReplacedExtension(const std::string &newExtension) const {
Common/File/Path.h
+7 -1
View File
@@ -86,7 +86,13 @@ public:
// File extension manipulation.
Path WithExtraExtension(std::string_view ext) const;
Path WithReplacedExtension(const std::string &oldExtension, const std::string &newExtension) const;
// Swaps one known extension for another, e.g. ".ppst" -> ".jpg". Returns false and leaves *out
// alone if the path doesn't actually end in oldExtension. That case has to be handled: this
// used to return the path unchanged, so a caller that guessed the extension wrong quietly
// carried on with the original file - and "delete the screenshot next to this savestate" then
// means "delete the savestate".
[[nodiscard]] bool WithReplacedExtension(const std::string &oldExtension, const std::string &newExtension, Path *out) const;
// Replaces whatever extension is there, so there's nothing to fail on.
Path WithReplacedExtension(const std::string &newExtension) const;
std::string GetFilename() const; // Really, GetLastComponent. Could be a file or directory. Includes the extension.
UI/GameInfoCache.cpp
+8 -6
View File
@@ -170,8 +170,8 @@ bool GameInfo::Delete() {
const Path &ppstPath = filePath_;
INFO_LOG(Log::System, "Deleting file %s", ppstPath.c_str());
MoveFileToTrashOrDelete(ppstPath);
const Path screenshotPath = filePath_.WithReplacedExtension(".ppst", ".jpg");
if (File::Exists(screenshotPath)) {
Path screenshotPath;
if (filePath_.WithReplacedExtension(".ppst", ".jpg", &screenshotPath) && File::Exists(screenshotPath)) {
MoveFileToTrashOrDelete(screenshotPath);
}
return true;
@@ -766,8 +766,9 @@ handleELF:
// Let's use the screenshot as an icon, too.
if (flags_ & GameInfoFlags::ICON) {
Path screenshotPath = gamePath_.WithReplacedExtension(".ppst", ".jpg");
if (ReadLocalFileToString(screenshotPath, &info_->icon.data, &info_->lock)) {
Path screenshotPath;
if (gamePath_.WithReplacedExtension(".ppst", ".jpg", &screenshotPath) &&
ReadLocalFileToString(screenshotPath, &info_->icon.data, &info_->lock)) {
info_->icon.dataLoaded = true;
}
}
@@ -778,9 +779,10 @@ handleELF:
{
info_->SetTitle(info_->GetFilePath().GetFilename());
if (flags_ & GameInfoFlags::ICON) {
Path screenshotPath = gamePath_.WithReplacedExtension(".ppdmp", ".png");
// Let's use the comparison screenshot as an icon, if it exists.
if (screenshotPath.IsLocalType() && ReadLocalFileToString(screenshotPath, &info_->icon.data, &info_->lock)) {
Path screenshotPath;
if (gamePath_.WithReplacedExtension(".ppdmp", ".png", &screenshotPath) &&
screenshotPath.IsLocalType() && ReadLocalFileToString(screenshotPath, &info_->icon.data, &info_->lock)) {
info_->icon.dataLoaded = true;
}
}
UI/SavedataScreen.cpp
+2 -2
View File
@@ -86,8 +86,8 @@ SavedataView::SavedataView(UIContext &dc, const Path &savePath, IdentifiedFileTy
Add(new Spacer(3.0));
} else {
_dbg_assert_(type == IdentifiedFileType::PPSSPP_SAVESTATE);
Path image_path = savePath.WithReplacedExtension(".ppst", ".jpg");
if (File::Exists(image_path)) {
Path image_path;
if (savePath.WithReplacedExtension(".ppst", ".jpg", &image_path) && File::Exists(image_path)) {
toprow->Add(new AsyncImageFileView(image_path, IS_KEEP_ASPECT, new LinearLayoutParams(480, 272, Margins(10, 0))));
} else {
auto sa = GetI18NCategory(I18NCat::SAVEDATA);
headless/Compare.cpp
+6 -1
View File
@@ -190,7 +190,12 @@ std::string GetTestName(const Path &bootFilename)
}
bool CompareOutput(const Path &bootFilename, const std::string &output, bool verbose, bool printEqualLines) {
Path expect_filename = bootFilename.GetFileExtension() == ".prx" ? bootFilename.WithReplacedExtension(".prx", ".expected") : bootFilename.WithExtraExtension(".expected");
// A .prx test has its expectations next to it as ".expected"; anything else just gets the
// extension tacked on.
Path expect_filename;
if (!bootFilename.WithReplacedExtension(".prx", ".expected", &expect_filename)) {
expect_filename = bootFilename.WithExtraExtension(".expected");
}
std::unique_ptr<FileLoader> expect_loader(ConstructFileLoader(expect_filename));
if (expect_loader->Exists()) {
unittest/UnitTest.cpp
+9 -1
View File
@@ -2236,7 +2236,15 @@ static bool TestPath() {
EXPECT_EQ_INT((Path("") / "/etc/passwd").empty(), false);
EXPECT_EQ_STR(Path("foo.bar/hello").GetFileExtension(), std::string());
EXPECT_EQ_STR(Path("foo.bar/hello.txt").WithReplacedExtension(".txt", ".html").ToString(), std::string("foo.bar/hello.html"));
Path replaced("unset");
EXPECT_EQ_INT(Path("foo.bar/hello.txt").WithReplacedExtension(".txt", ".html", &replaced), true);
EXPECT_EQ_STR(replaced.ToString(), std::string("foo.bar/hello.html"));
// The extension has to actually be there. This used to hand back "foo.bar/hello.txt", so a
// caller asking for the .html next to it would have been pointed at the .txt itself.
EXPECT_EQ_INT(Path("foo.bar/hello.txt").WithReplacedExtension(".png", ".html", &replaced), false);
EXPECT_EQ_STR(replaced.ToString(), std::string("foo.bar/hello.html")); // Untouched by the failure.
// Only the trailing extension counts - a dot earlier in the name isn't one.
EXPECT_EQ_INT(Path("foo.txt/hello").WithReplacedExtension(".txt", ".html", &replaced), false);
EXPECT_EQ_STR(Path("C:\\Yo").NavigateUp().ToString(), std::string("C:"));
#if PPSSPP_PLATFORM(WINDOWS)