1. Comparing string_views does not behave the same as strncmp
in the case where SConfig's game ID is longer than 6 chars.
2. DTMHeader::GetGameID wasn't excluding null bytes for game IDs
shorter than 6 chars.
3. == was accidentally used instead of !=.
This issue is both severe and surprisingly difficult to find
the root cause of, so I think it would make sense to add a simple
hotfix for now. https://bugs.dolphin-emu.org/issues/12327
Fallback Region
A user-selected fallback to use instead of the default PAL
This is used for unknown region or region free titles to give them
the ability to force region to use. This replaces the current fallback region
of PAL. This can be useful if a user is trying to play a region free
tilte that is originally NTSC and expects to be run at NTSC speeds. This
may be done when a user attempts to dump a WAD of their own without
understanding the settings they have chosen, or could be an intentional
decision by a developer of a ROM hack that can be injected into a
Virtual Console WAD.
Remove using System Menu region being checked in GetFallbackRegion
Use DiscIO::Region instead of std::String for fallback
Add explanation text for Fallback Region
Unfortunately, adding a DEBUG_ASSERT_MSG_FMT isn't actually possible
right now because of compiler bugs:
https://github.com/dolphin-emu/dolphin/pull/9284
We could require a newer version of GCC (10) but that would require
updating GCC on the build machines.
For what it's worth, older versions of GCC (8, 9) are broken in
many ways: adding constexpr to some Matrix functions causes GCC 8
to generate bugged code that causes the Wii IR pointer to disappear,
which means that the generated builds are already unusable
(see https://dolp.in/i12324).
Additionally, we've already had to add workarounds for those versions
in the format macros to fix compilation bugs. This time, it looks like
workarounds won't cut it; even applying the workaround
described in https://github.com/fmtlib/fmt/pull/1580 does not help.
On Windows, when the Rename function fails to replace an existing file
it will now retry the operation multiple times with increasingly long
delays between attempts. This fixes transient rename failures.
I've been getting sporadic yet annoyingly frequent errors saying:
'IOS_FS: Failed to rename temporary FST file'
These typically appear on startup but I've also gotten them randomly.
Investigation shows this happens when the Windows ReplaceFile function
returns the error ERROR_UNABLE_TO_REMOVE_REPLACED. That happens in the
context of using ReplaceFile to perform an atomic file overwrite, which
is required when saving updates to a file to avoid corruption. The
error mainly happens with the /Wii/fst.bin file but I've seen it
happen with multiple other files as well.
I haven't been able to definitively pin down why the error occurs,
though online discussions suggest antivirus scanning may be a major
culprit. That said, I've excluded the Dolphin folder from Windows
Defender scans to no avail and don't have any other antivirus running,
so this is likely to be a problem others are experiencing as well.
The number and duration of retry delays is arbitrary but I feel like a
combined second or so in the worst case is an acceptable tradeoff for
the reduction (actually elimination in my experience) of those errors.
This is even more true when you consider the time it takes to read and
dismiss the error dialogs.
Converts the remaining PowerPC code over to fmt-capable logging.
Now, all that's left to convert over are the lingering remnants within
the frontend code.
The way Config::Get works in master, it first calls
Config::GetActiveLayerForConfig which searches for the
setting in all layers, and then calls Config::Layer::Get
which searches for the same setting again within the given
layer. We can remove this second search by combining the
logic of Config::GetActiveLayerForConfig and
Config::Layer::Get into one function.
Disable -Wstringop-truncation warnings as they result in many false
positives.
In most (all?) cases where std::strncpy is used, we want to fill the
entire buffer or match emulated code that also ignores the null
terminator, so the warnings are not useful.
Given that Dolphin itself mostly uses std::string, they do not
really help catch any bugs.
gameID isn't null terminated since it is just an std::array<char, 6>
and .data() returns a char* so {fmt} would go way beyond the bounds of
the array when it attempts to determine the length of the string.
The fix is to pass a std::string_view to {fmt}. This commit adds
a GetGameID() function that can also be used to simplify
string comparisons.
Having extra warnings enabled for everything including external
libraries produces an overwhelming amount of warnings in code that
isn't even part of our codebase.
Move the various warning flags to Source/CMakeLists.txt to get rid
of those useless warnings.
Note that the Source CMakeLists.txt is already where the MSVC warnings
are defined, so this commit improves consistency as well.
Cel-damage depends on lighting being calculated for the first channel
even though there is no color in the vertex format (defaults to the
material color). If lighting for the channel is not enabled, the vertex
will use the default color as before.
The default value of the color is determined by the number of elements in
the vertex format. This fixes the grey cubes in Super Mario Sunshine.
If the color channel count is zero, we set the color to black before the
end of the vertex shader. It's possible that this would be undefined
behavior on hardware if a vertex color index that was greater than the
channel count was used within TEV.
PR #9260 made the MsgAlert macros use lambdas so that local
constexpr variables can be added while keeping the ability to
return a boolean from the macros.
Unfortunately, C++17 forbids referring to structured bindings in lambda
captures. This is fixed in P1091R3 but we cannot rely on C++20 yet...
In CreateDescriptorSetLayouts(), one less dynamic binding is created when
bSupportsGeometryShaders=false. Reduce the dynamicOffsetCount argument by
one in that case. Avoids this validation error:
Attempting to bind 3 descriptorSets with 2 dynamic descriptors, but
dynamicOffsetCount is 3. It should exactly match the number of dynamic
descriptors. The Vulkan spec states: dynamicOffsetCount must be equal to
the total number of dynamic descriptors in pDescriptorSets
Signed-off-by: Jonathan Marek <jonathan@marek.ca>
[Applied clang-format]
Signed-off-by: Léo Lam <leo@leolam.fr>
Unfortunately, {fmt} allows passing too many arguments to a format call
without raising any runtime or compile-time error [1].
As this is a common source of bugs since we started migrating to {fmt},
this commit adds some custom logic to validate the number of
replacement fields in format strings in addition to {fmt}'s own checks.
[1] https://github.com/fmtlib/fmt/issues/492
We want to use positional arguments in translatable strings
that have more than one argument so that translators can change
the order of them, but the question is: Should we also use
positional arguments in translatable strings with only one
argument? I think it makes most sense that way, partially
so that translators don't even have to be aware of the
non-positional syntax and partially because "translatable
strings use positional arguments" is an easier rule for us
to remember than "transitional strings which have more than
one argument use positional arguments". But let me know if
you have a different opinion.
tr calls with more than one argument would have a 0x04 byte
in the returned string when no translation was found
(which always is the case when using Dolphin in English).
They are unused, since there is no C++ code that touches
these settings. See the discussion in PR 9152, which is
a PR that adds a lot more Android-specific settings.
I moved it from the main settings screen to the in-game menu
in PR 8439 so that it could be changed while a game is running,
but now that the main settings can be accessed while a game is
running, there's no reason to not put it in the main settings.
https://bugs.dolphin-emu.org/issues/12067
Adds an interface that uses fmt under the hood, which is much more
flexible than printf, particularly for localization purposes, given fmt
supports positional formatters in a cross-platform manner out of the box
with no configuration necessary.
This is already handled by SurfaceDestroyed. In the worst case,
the extra code could even race with SurfaceDestroyed if they
are triggered at the same time, but this is highly improbable.
Time for yet another new iteration of working around the
"surface destruction during boot" problem...
This time, the strategy is to use a mutex in MainAndroid.cpp.
Now that we've converted all of the shader generators over to using fmt,
we can drop the old Write() member function and perform a rename
operation on the WriteFmt() to turn it into the new Write() function.
All changes within this are the removal of a <cstdarg> header, since the
previous printf-based Write() required it, and renaming. No functional
changes are made at all.
This assumption is false both in portrait mode (where it only
covers the top half of the screen) and when using two apps at once.
Fixes https://bugs.dolphin-emu.org/issues/12307.
Noticed missing include as a build failure on gcc-11:
```
[ 15%] Building CXX object Source/Core/Common/CMakeFiles/common.dir/Config/Config.cpp.o
Source/Core/Common/Config/Config.cpp:23:24:
error: 'unique_lock' in namespace 'std' does not name a template type
23 | using WriteLock = std::unique_lock<std::shared_mutex>;
| ^~~~~~~~~~~
Source/Core/Common/Config/Config.cpp:11:1:
note: 'std::unique_lock' is defined in header '<mutex>';
did you forget to '#include <mutex>'?
```
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
Completes the migration over to using the fmt-formatting WriteFmt
function. The next PR will rename all usages of WriteFmt, while
simultaneously getting rid of the old printf code.
JosJuice