The class NonCopyable is, like the name says, supposed to disallow
copying. But should it allow moving?
For a long time, NonCopyable used to not allow moving. (It declared
a deleted copy constructor and assigment operator without declaring
a move constructor and assignment operator, making the compiler
implicitly delete the move constructor and assignment operator.)
That's fine if the classes that inherit from NonCopyable don't need
to be movable or if writing the move constructor and assignment
operator by hand is fine, but that's not the case for all classes,
as I discovered when I was working on the DirectoryBlob PR.
Because of that, I decided to make NonCopyable movable in c7602cc,
allowing me to use NonCopyable in DirectoryBlob.h. That was however
an unfortunate decision, because some of the classes that inherit
from NonCopyable have incorrect behavior when moved by default-
generated move constructors and assignment operators, and do not
explicitly delete the move constructors and assignment operators,
relying on NonCopyable being non-movable.
So what can we do about this? There are four solutions that I can
think of:
1. Make NonCopyable non-movable and tell DirectoryBlob to suck it.
2. Keep allowing moving NonCopyable, and expect that classes that
don't support moving will delete the move constructor and
assignment operator manually. Not only is this inconsistent
(having classes disallow copying one way and disallow moving
another way), but deleting the move constructor and assignment
operator manually is too easy to forget compared to how tricky
the resulting problems are.
3. Have one "MovableNonCopyable" and one "NonMovableNonCopyable".
It works, but it feels rather silly...
4. Don't have a NonCopyable class at all. Considering that deleting
the copy constructor and assignment operator only takes two lines
of code, I don't see much of a reason to keep NonCopyable. I
suppose that there was more of a point in having NonCopyable back
in the pre-C++11 days, when it wasn't possible to use "= delete".
I decided to go with the fourth one (like the commit title says).
The implementation of the commit is fairly straight-forward, though
I would like to point out that I skipped adding "= delete" lines
for classes whose only reason for being uncopyable is that they
contain uncopyable classes like File::IOFile and std::unique_ptr,
because the compiler makes such classes uncopyable automatically.
I'm not sure why this hasn't popped up as an error on the buildbots,
but the build fails on my new install of VS2017. The error is C2445:
result type of conditional expression is ambiguous: types 'wxString'
and 'const char [1]' can be converted to multiple common types
"Pad size" just doesn't make much sense. Let's go with "Buffer size"
instead, since the control for it is labeled "Buffer".
(Another possibility is "Pad buffer size", but I'm against that,
because we've stopped referring to controllers as "pads" in almost
all GUI strings.)
Sentret_C posted this comment on Transifex recently:
"What Dolphin refers to as "Table View" and "List View" are
similar to "List View" and "Grid View" in Steam, and I think
the Steam names describe them better."
I agree with that, so here's a commit that changes the names.
When building DolphinWX using cmake, msgmerge will run on the
.po files, updating the formatting of them if they aren't
already in the exact format that msgmerge uses. This means that
running msgmerge on all the .po files is necessary before
committing any updates to the .po files, because otherwise
unstaged changes are going to appear whenever anyone tries to
build that commit using cmake. The translation syncing scripts
that were used by delroth and Parlane did so by invoking `cmake`
and then `make translations`, but that isn't a good solution
for people who sync translations on Windows (i.e. me).
This commit uncomments some existing code for running msgmerge
on all the .po files, and places that code in a new script. Now,
translation syncing scripts just have to call that script before
committing, instead of using msgmerge directly or using cmake.
I'm not sure why the script was called gettextize to begin with.
gettextize is the name of a gettext executable that doesn't seem
to be related to what our gettextize script did.
For instance, we don't want to show TGC files that might be
inside the /files/ directory of a GameCube DirectoryBlob,
and we don't want to show the /sys/main.dol files for extra
partitions of Wii DirectoryBlobs.
Now it's clearer that SetDOL depends on SetApploader
and BuildFST depends on SetDOL.
As a side note, we now load the DOL even if there's
no apploader. (I don't think it matters whether we
do it, but it was easier to implement this way.)
This lets VolumeDirectory/DirectoryBlob skip implementing
various volume functions like GetGameID, GetBanner, etc.
It also lets us view extracted discs in the game list.
This ends up breaking the boot process for Wii
DirectoryBlobs due to workarounds being removed from the
boot process, but that will be fixed later by adding
proper DirectoryBlob support for things like TMDs.
We now expect the directories to be laid out in a certain
format (based on the format that WIT uses) instead of requiring
the user to set the DVD root and apploader path settings.
This is useful for blob types that store Wii data unencrypted
(such as WIA and discs extracted to directories) so that
we don't have to waste CPU time encrypting in the blob code
just to decrypt right afterwards in the volume code.
The old approach to detecting DOL/ELF files doesn't fit
with the new way of implementing extracted discs.
The game list is already doing it in a way that's similar
to the approach that this commit uses.
It's not particularily useful to list the platform here,
and these kinds of messages that use words as parameters
are more likely to be mistranslated than the average string.
Same as the previous commit, except I'm copying strings
in the other direction because the DolphinWX variants
of these strings could use some improvement.
The region mismatch check that we used can give false positives.
Skipping the check won't lead to any harm - games will ignore
save files that have a non-matching fourth game ID character.
According to http://scanlines16.com/en/blog-3/retro-gaming/game-cube/gamecube-korean-master-list/,
Korean GC releases use the following country codes:
- E or W for games in English
- K for games in Korean
- Unknown value for games in Japanese (my guess is that they might
have made the discs bit-for-bit identical to Japanese releases
because the regions of these games are already set to NTSC-J)
As far as I know, the GC has no Taiwanese releases, which is what
the W country code is used for on the Wii. But I could be wrong.
A small note: The country_byte == 'K' check in the code isn't
actually necessary as long as RegionSwitchGC returns NTSC_J
for 'K', but I thought it would be better to not rely on that.
The county code isn't 100% reliable for detecting the region.
For instance, some games released in Korea have the country
code E even though they're region-locked to NTSC-J consoles.
This commit makes the GC disc region detection match the Wii
disc region detection (apart from the region value being in
a different place on the disc).
JosJuice