fix emulator issue #12139 #9204
Conversation
Source/Core/DiscIO/DiscExtractor.cpp
Outdated
| @@ -130,13 +130,15 @@ void ExportDirectory(const Volume& volume, const Partition& partition, const Fil | |||
| const std::string& export_folder, | |||
| const std::function<bool(const std::string& path)>& update_progress) | |||
| { | |||
| File::CreateFullPath(export_folder + '/'); | |||
| File::CreateFullPath(export_folder + | |||
| (directory.IsDirectory() ? "/" + directory.GetName() + "/" : "/")); | |||
There was a problem hiding this comment.
I would probably make sense to put this ternary in a local variable (perhaps even including the export_folder itself), rather than doing it both here and once for every iteration in the loop below.
There was a problem hiding this comment.
Changed. Generally speaking, do we want dolphin to be more CPU-bound or have a smaller memory footprint? I realize this is probably a non-issue here since this operation isn't time-sensitive and not particularly slow to begin with, but I can imagine similar situations elsewhere in the codebase.
There was a problem hiding this comment.
When it comes to the emulation core, being constrained by the CPU is way more common than being constrained by the RAM size, to the point where I can't remember a single time the latter has happened for anyone other than when custom texture prefetching is involved. When it comes to things other than the emulation core... Well, the RAM usage is even lower when not emulated, so using a bit of extra RAM should be no problem. Not that I believe keeping this string around actually increases the memory usage, as the alternative of reconstructing it every time still has to allocate the string for the duration of the lines where it's used.
Source/Core/DiscIO/DiscExtractor.cpp
Outdated
| @@ -130,13 +130,15 @@ void ExportDirectory(const Volume& volume, const Partition& partition, const Fil | |||
| const std::string& export_folder, | |||
| const std::function<bool(const std::string& path)>& update_progress) | |||
| { | |||
| File::CreateFullPath(export_folder + '/'); | |||
| std::string export_root = | |||
There was a problem hiding this comment.
You should be able to make this const as well.
There was a problem hiding this comment.
Changes seem good so far, untested tho.
You do want to squash those commits into one before continuing though; you can use the following to do this (replace upstream with the name of the Dolphin remote and origin with the name of your Fork remote):
git fetch upstream
git reset --soft upstream/master
git commit -m "modify file export to create folder in OS when explicitly extracting a directory"
git push -f origin HEAD
The first command makes sure you get the latest commits from master (from the Dolphin repository) to work on; the second picks all your changes, switches to that master state and puts the changes in the staging area. The third command then commits it so you have one commit total that has all the changes; and the forth command pushes your current branch into your own remote (which should be your Fork)
fbastos1
force-pushed
the
master
branch
2 times, most recently
from
3f5256c
to
70e453c
Compare
This comment has been minimized.
This comment has been minimized.
modify file export to create folder in OS when explicitly extracting a directory