From f2be35c7cdc97e53f3d9f424ec29d3cbeb6989d5 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sun, 19 Feb 2023 16:30:06 +0100 Subject: [PATCH] VideoCommon: Reword the unknown opcode error message When faced with this error, users often don't try disabling dual core, even though the error message suggests it. Perhaps the message is just too long and lists too many things? To try to improve the situation, I'm rewording the message and making it say different things depending on what settings you are using. --- Source/Core/Core/CoreTiming.cpp | 5 ++ Source/Core/Core/CoreTiming.h | 2 + Source/Core/VideoCommon/CommandProcessor.cpp | 54 ++++++++++++++++---- Source/Core/VideoCommon/CommandProcessor.h | 2 +- Source/Core/VideoCommon/Fifo.h | 1 + Source/Core/VideoCommon/OpcodeDecoding.cpp | 4 +- 6 files changed, 56 insertions(+), 12 deletions(-) diff --git a/Source/Core/Core/CoreTiming.cpp b/Source/Core/Core/CoreTiming.cpp index aa7c501c92c0..2e4e5112c1c9 100644 --- a/Source/Core/Core/CoreTiming.cpp +++ b/Source/Core/Core/CoreTiming.cpp @@ -417,6 +417,11 @@ bool CoreTimingManager::GetVISkip() const return m_throttle_disable_vi_int && g_ActiveConfig.bVISkip && !Core::WantsDeterminism(); } +bool CoreTimingManager::UseSyncOnSkipIdle() const +{ + return m_config_sync_on_skip_idle; +} + void CoreTimingManager::LogPendingEvents() const { auto clone = m_event_queue; diff --git a/Source/Core/Core/CoreTiming.h b/Source/Core/Core/CoreTiming.h index 1ddb13f948f7..525b7e891348 100644 --- a/Source/Core/Core/CoreTiming.h +++ b/Source/Core/Core/CoreTiming.h @@ -150,6 +150,8 @@ class CoreTimingManager TimePoint GetCPUTimePoint(s64 cyclesLate) const; // Used by Dolphin Analytics bool GetVISkip() const; // Used By VideoInterface + bool UseSyncOnSkipIdle() const; + private: Globals m_globals; diff --git a/Source/Core/VideoCommon/CommandProcessor.cpp b/Source/Core/VideoCommon/CommandProcessor.cpp index 75cedcf96f50..955bb337b9f3 100644 --- a/Source/Core/VideoCommon/CommandProcessor.cpp +++ b/Source/Core/VideoCommon/CommandProcessor.cpp @@ -12,6 +12,7 @@ #include "Common/CommonTypes.h" #include "Common/Flag.h" #include "Common/Logging/Log.h" +#include "Common/MsgHandler.h" #include "Core/ConfigManager.h" #include "Core/CoreTiming.h" #include "Core/HW/GPFifo.h" @@ -637,7 +638,8 @@ void CommandProcessorManager::SetCpClearRegister() { } -void CommandProcessorManager::HandleUnknownOpcode(u8 cmd_byte, const u8* buffer, bool preprocess) +void CommandProcessorManager::HandleUnknownOpcode(Core::System& system, u8 cmd_byte, + const u8* buffer, bool preprocess) { const auto& fifo = m_fifo; @@ -693,16 +695,50 @@ void CommandProcessorManager::HandleUnknownOpcode(u8 cmd_byte, const u8* buffer, { m_is_fifo_error_seen = true; - // TODO(Omega): Maybe dump FIFO to file on this error + // The panic alert contains an explanatory part that's worded differently depending on the + // user's settings, so as to offer the most relevant advice to the user. + const char* advice; + if (IsOnThread(system) && !system.GetFifo().UseDeterministicGPUThread()) + { + if (!system.GetCoreTiming().UseSyncOnSkipIdle() && !system.GetFifo().UseSyncGPU()) + { +// The SyncOnSkipIdle setting is only in the Android GUI, so we use the INI name on other platforms. +// +// TODO: Mark the Android string as translatable once we have translations on Android. It's +// currently untranslatable so translators won't try to look up how they translated "Synchronize +// GPU Thread" and "On Idle Skipping" and then not find those strings and become confused. +#ifdef ANDROID + advice = "Please change the \"Synchronize GPU Thread\" setting to \"On Idle Skipping\"! " + "It's currently set to \"Never\", which makes this problem very likely to happen."; +#else + // i18n: Please leave SyncOnSkipIdle and True untranslated. + // The user needs to enter these terms as-is in an INI file. + advice = _trans("Please change the \"SyncOnSkipIdle\" setting to \"True\"! " + "It's currently disabled, which makes this problem very likely to happen."); +#endif + } + else + { + advice = _trans( + "This error is usually caused by the emulated GPU desyncing with the emulated CPU. " + "Turn off the \"Dual Core\" setting to avoid this."); + } + } + else + { + advice = _trans( + "This error is usually caused by the emulated GPU desyncing with the emulated CPU, " + "but your current settings make this unlikely to happen. If this error is stopping the " + "game from working, please report it to the developers."); + } + PanicAlertFmtT("GFX FIFO: Unknown Opcode ({0:#04x} @ {1}, preprocess={2}).\n" - "This means one of the following:\n" - "* The emulated GPU got desynced, disabling dual core can help\n" - "* Command stream corrupted by some spurious memory bug\n" - "* This really is an unknown opcode (unlikely)\n" - "* Some other sort of bug\n\n" - "Further errors will be sent to the Video Backend log and\n" + "\n" + "{3}\n" + "\n" + "Further errors will be sent to the Video Backend log and " "Dolphin will now likely crash or hang.", - cmd_byte, fmt::ptr(buffer), preprocess); + cmd_byte, fmt::ptr(buffer), preprocess, Common::GetStringT(advice)); } } diff --git a/Source/Core/VideoCommon/CommandProcessor.h b/Source/Core/VideoCommon/CommandProcessor.h index 34b1e8701f4c..ccc381b535ef 100644 --- a/Source/Core/VideoCommon/CommandProcessor.h +++ b/Source/Core/VideoCommon/CommandProcessor.h @@ -177,7 +177,7 @@ class CommandProcessorManager void SetCpControlRegister(Core::System& system); void SetCpStatusRegister(Core::System& system); - void HandleUnknownOpcode(u8 cmd_byte, const u8* buffer, bool preprocess); + void HandleUnknownOpcode(Core::System& system, u8 cmd_byte, const u8* buffer, bool preprocess); // This one is shared between gfx thread and emulator thread. // It is only used by the Fifo and by the CommandProcessor. diff --git a/Source/Core/VideoCommon/Fifo.h b/Source/Core/VideoCommon/Fifo.h index 1db91eeb915a..20648562b4a3 100644 --- a/Source/Core/VideoCommon/Fifo.h +++ b/Source/Core/VideoCommon/Fifo.h @@ -54,6 +54,7 @@ class FifoManager final void PauseAndLock(Core::System& system, bool doLock, bool unpauseOnUnlock); void UpdateWantDeterminism(Core::System& system, bool want); bool UseDeterministicGPUThread() const { return m_use_deterministic_gpu_thread; } + bool UseSyncGPU() const { return m_config_sync_gpu; } // In deterministic GPU thread mode this waits for the GPU to be done with pending work. void SyncGPU(SyncGPUReason reason, bool may_move_read_ptr = true); diff --git a/Source/Core/VideoCommon/OpcodeDecoding.cpp b/Source/Core/VideoCommon/OpcodeDecoding.cpp index 9ffb59a27747..4d43c6e66a0d 100644 --- a/Source/Core/VideoCommon/OpcodeDecoding.cpp +++ b/Source/Core/VideoCommon/OpcodeDecoding.cpp @@ -219,8 +219,8 @@ class RunCallback final : public Callback } else { - Core::System::GetInstance().GetCommandProcessor().HandleUnknownOpcode(opcode, data, - is_preprocess); + auto& system = Core::System::GetInstance(); + system.GetCommandProcessor().HandleUnknownOpcode(system, opcode, data, is_preprocess); m_cycles += 1; } }