Skip to content

Commit

Permalink
Audit uses of IsRunning and GetState
Browse files Browse the repository at this point in the history
Some pieces of code are calling IsRunning because there's some
particular action that only makes sense when emulation is running, for
instance showing the state of the emulated CPU. IsRunning is appropriate
to use for this. Then there are pieces of code that are calling
IsRunning because there's some particular thing they must avoid doing
e.g. when the CPU thread is running or IOS is running. IsRunning isn't
quite appropriate for this. Such code should also be checking for the
states Starting and Stopping. Keep in mind that:

* When the state is Starting, the state can asynchronously change to
  Running at any time.
* When we try to stop the core, the state gets set to Stopping before we
  take any action to actually stop things.

This commit adds a new method Core::IsUninitialized, and changes all
callers of IsRunning and GetState that look to me like they should be
changed.
  • Loading branch information
JosJuice committed Jun 21, 2024
1 parent 962230f commit 72cf2bd
Show file tree
Hide file tree
Showing 33 changed files with 134 additions and 106 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -381,10 +381,16 @@ public static native void Run(String[] path, boolean riivolution, String savesta
*/
public static native boolean IsRunning();

public static native boolean IsRunningAndStarted();

/**
* Returns true if emulation is running and not paused.
*/
public static native boolean IsRunningAndUnpaused();

/**
* Returns true if emulation is fully shut down.
*/
public static native boolean IsUninitialized();

/**
* Writes out the JitBlock Cache log dump
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class Settings : Closeable {

if (isGameSpecific) {
// Loading game INIs while the core is running will mess with the game INIs loaded by the core
check(!NativeLibrary.IsRunning()) { "Attempted to load game INI while emulating" }
check(NativeLibrary.IsUninitialized()) { "Attempted to load game INI while emulating" }
NativeConfig.loadGameInis(gameId, revision)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,5 @@ class RunRunnable(
override val setting: AbstractSetting? = null

override val isEditable: Boolean
get() = worksDuringEmulation || !NativeLibrary.IsRunning()
get() = worksDuringEmulation || NativeLibrary.IsUninitialized()
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ abstract class SettingsItem {

open val isEditable: Boolean
get() {
if (!NativeLibrary.IsRunning()) return true
if (NativeLibrary.IsUninitialized()) return true
val setting = setting
return setting != null && setting.isRuntimeEditable
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class SettingsFragmentPresenter(
} else if (
menuTag == MenuTag.GRAPHICS
&& this.gameId.isNullOrEmpty()
&& !NativeLibrary.IsRunning()
&& NativeLibrary.IsUninitialized()
&& GpuDriverHelper.supportsCustomDriverLoading()
) {
this.gpuDriver =
Expand Down Expand Up @@ -1303,7 +1303,7 @@ class SettingsFragmentPresenter(

if (
this.gpuDriver != null && this.gameId.isNullOrEmpty()
&& !NativeLibrary.IsRunning()
&& NativeLibrary.IsUninitialized()
&& GpuDriverHelper.supportsCustomDriverLoading()
) {
sl.add(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,11 +180,11 @@ class EmulationFragment : Fragment(), SurfaceHolder.Callback {

private fun run(isActivityRecreated: Boolean) {
if (isActivityRecreated) {
if (NativeLibrary.IsRunning()) {
if (NativeLibrary.IsUninitialized()) {
loadPreviousTemporaryState = true
} else {
loadPreviousTemporaryState = false
deleteFile(temporaryStateFilePath)
} else {
loadPreviousTemporaryState = true
}
} else {
Log.debug("[EmulationFragment] activity resumed or fresh start")
Expand All @@ -203,7 +203,7 @@ class EmulationFragment : Fragment(), SurfaceHolder.Callback {

private fun runWithValidSurface() {
runWhenSurfaceIsValid = false
if (!NativeLibrary.IsRunning()) {
if (NativeLibrary.IsUninitialized()) {
NativeLibrary.SetIsBooting()
val emulationThread = Thread({
if (loadPreviousTemporaryState) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ class InputOverlay(context: Context?, attrs: AttributeSet?) : SurfaceView(contex

fun initTouchPointer() {
// Check if we have all the data we need yet
val aspectRatioAvailable = NativeLibrary.IsRunningAndStarted()
val aspectRatioAvailable = NativeLibrary.IsRunning()
if (!aspectRatioAvailable || surfacePosition == null)
return

Expand Down
17 changes: 8 additions & 9 deletions Source/Android/jni/MainAndroid.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,7 @@ void Host_Message(HostMessageID id)
}
else if (id == HostMessageID::WMUserStop)
{
if (Core::IsRunning(Core::System::GetInstance()))
Core::QueueHostJob(&Core::Stop);
Core::QueueHostJob(&Core::Stop);
}
}

Expand Down Expand Up @@ -271,13 +270,6 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SetIsBooting
}

JNIEXPORT jboolean JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_IsRunning(JNIEnv*, jclass)
{
return s_is_booting.IsSet() ||
static_cast<jboolean>(Core::IsRunning(Core::System::GetInstance()));
}

JNIEXPORT jboolean JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_IsRunningAndStarted(JNIEnv*,
jclass)
{
return static_cast<jboolean>(Core::IsRunning(Core::System::GetInstance()));
}
Expand All @@ -288,6 +280,13 @@ Java_org_dolphinemu_dolphinemu_NativeLibrary_IsRunningAndUnpaused(JNIEnv*, jclas
return static_cast<jboolean>(Core::GetState(Core::System::GetInstance()) == Core::State::Running);
}

JNIEXPORT jboolean JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_IsUninitialized(JNIEnv*,
jclass)
{
return static_cast<jboolean>(Core::IsUninitialized(Core::System::GetInstance()) &&
!s_is_booting.IsSet());
}

JNIEXPORT jstring JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_GetVersionString(JNIEnv* env,
jclass)
{
Expand Down
4 changes: 2 additions & 2 deletions Source/Core/Core/ConfigLoaders/BaseConfigLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ namespace ConfigLoaders
{
void SaveToSYSCONF(Config::LayerType layer, std::function<bool(const Config::Location&)> predicate)
{
if (Core::IsRunning(Core::System::GetInstance()))
if (!Core::IsUninitialized(Core::System::GetInstance()))
return;

IOS::HLE::Kernel ios;
Expand Down Expand Up @@ -183,7 +183,7 @@ class BaseConfigLayerLoader final : public Config::ConfigLayerLoader
private:
void LoadFromSYSCONF(Config::Layer* layer)
{
if (Core::IsRunning(Core::System::GetInstance()))
if (!Core::IsUninitialized(Core::System::GetInstance()))
return;

IOS::HLE::Kernel ios;
Expand Down
7 changes: 6 additions & 1 deletion Source/Core/Core/Core.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,11 @@ bool IsRunningOrStarting(Core::System& system)
return state == State::Running || state == State::Starting;
}

bool IsUninitialized(Core::System& system)
{
return s_state.load() == State::Uninitialized;
}

bool IsCPUThread()
{
return tls_is_cpu_thread;
Expand All @@ -237,7 +242,7 @@ bool Init(Core::System& system, std::unique_ptr<BootParameters> boot, const Wind
{
if (s_emu_thread.joinable())
{
if (IsRunning(system))
if (!IsUninitialized(system))
{
PanicAlertFmtT("Emu Thread already running");
return false;
Expand Down
5 changes: 5 additions & 0 deletions Source/Core/Core/Core.h
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,13 @@ void UndeclareAsHostThread();

std::string StopMessage(bool main_thread, std::string_view message);

// Returns true when GetState returns Running or Paused.
bool IsRunning(Core::System& system);
// Returns true when GetState returns Starting, Running or Paused.
bool IsRunningOrStarting(Core::System& system);
// Returns true when GetState returns Uninitialized.
bool IsUninitialized(Core::System& system);

bool IsCPUThread(); // this tells us whether we are the CPU thread.
bool IsGPUThread();
bool IsHostThread();
Expand Down
11 changes: 5 additions & 6 deletions Source/Core/DolphinQt/Achievements/AchievementSettingsWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,8 @@ void AchievementSettingsWidget::OnControllerInterfaceConfigure()

void AchievementSettingsWidget::LoadSettings()
{
Core::System& system = Core::System::GetInstance();

bool enabled = Config::Get(Config::RA_ENABLED);
bool hardcore_enabled = Config::Get(Config::RA_HARDCORE_ENABLED);
bool logged_out = Config::Get(Config::RA_API_TOKEN).empty();
Expand All @@ -174,18 +176,15 @@ void AchievementSettingsWidget::LoadSettings()
SignalBlocking(m_common_password_input)->setVisible(logged_out);
SignalBlocking(m_common_password_input)->setEnabled(enabled);
SignalBlocking(m_common_login_button)->setVisible(logged_out);
SignalBlocking(m_common_login_button)
->setEnabled(enabled && !Core::IsRunning(Core::System::GetInstance()));
SignalBlocking(m_common_login_button)->setEnabled(enabled && Core::IsUninitialized(system));
SignalBlocking(m_common_logout_button)->setVisible(!logged_out);
SignalBlocking(m_common_logout_button)->setEnabled(enabled);

SignalBlocking(m_common_hardcore_enabled_input)
->setChecked(Config::Get(Config::RA_HARDCORE_ENABLED));
auto& system = Core::System::GetInstance();
SignalBlocking(m_common_hardcore_enabled_input)
->setEnabled(enabled &&
(hardcore_enabled || (Core::GetState(system) == Core::State::Uninitialized &&
!system.GetMovie().IsPlayingInput())));
->setEnabled(enabled && (hardcore_enabled || (Core::IsUninitialized(system) &&
!system.GetMovie().IsPlayingInput())));

SignalBlocking(m_common_unofficial_enabled_input)
->setChecked(Config::Get(Config::RA_UNOFFICIAL_ENABLED));
Expand Down
3 changes: 1 addition & 2 deletions Source/Core/DolphinQt/CheatSearchFactoryWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,7 @@ void CheatSearchFactoryWidget::OnNewSearchClicked()
if (m_standard_address_space->isChecked())
{
auto& system = Core::System::GetInstance();
const Core::State core_state = Core::GetState(system);
if (core_state != Core::State::Running && core_state != Core::State::Paused)
if (!Core::IsRunning(system))
{
ModalMessageBox::warning(
this, tr("No game running."),
Expand Down
5 changes: 3 additions & 2 deletions Source/Core/DolphinQt/CheatsManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,9 @@ void CheatsManager::RefreshCodeTabs(Core::State state, bool force)
if (!force && (state == Core::State::Starting || state == Core::State::Stopping))
return;

const auto& game_id =
state != Core::State::Uninitialized ? SConfig::GetInstance().GetGameID() : std::string();
const auto& game_id = state == Core::State::Running || state == Core::State::Paused ?
SConfig::GetInstance().GetGameID() :
std::string();
const auto& game_tdb_id = SConfig::GetInstance().GetGameTDBID();
const u16 revision = SConfig::GetInstance().GetRevision();

Expand Down
5 changes: 3 additions & 2 deletions Source/Core/DolphinQt/Config/CheatWarningWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ CheatWarningWidget::CheatWarningWidget(const std::string& game_id, bool restart_

connect(&Settings::Instance(), &Settings::EnableCheatsChanged, this,
[this] { Update(Core::IsRunning(Core::System::GetInstance())); });
connect(&Settings::Instance(), &Settings::EmulationStateChanged, this,
[this](Core::State state) { Update(state == Core::State::Running); });
connect(&Settings::Instance(), &Settings::EmulationStateChanged, this, [this](Core::State state) {
Update(state == Core::State::Running || state == Core::State::Paused);
});

Update(Core::IsRunning(Core::System::GetInstance()));
}
Expand Down
3 changes: 1 addition & 2 deletions Source/Core/DolphinQt/Config/Graphics/AdvancedWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ AdvancedWidget::AdvancedWidget(GraphicsWindow* parent)
});

OnBackendChanged();
OnEmulationStateChanged(Core::GetState(Core::System::GetInstance()) !=
Core::State::Uninitialized);
OnEmulationStateChanged(!Core::IsUninitialized(Core::System::GetInstance()));
}

void AdvancedWidget::CreateWidgets()
Expand Down
6 changes: 3 additions & 3 deletions Source/Core/DolphinQt/Config/Graphics/GeneralWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@ GeneralWidget::GeneralWidget(GraphicsWindow* parent)
connect(&Settings::Instance(), &Settings::EmulationStateChanged, this, [this](Core::State state) {
OnEmulationStateChanged(state != Core::State::Uninitialized);
});
OnEmulationStateChanged(Core::GetState(Core::System::GetInstance()) !=
Core::State::Uninitialized);
OnEmulationStateChanged(!Core::IsUninitialized(Core::System::GetInstance()));
}

void GeneralWidget::CreateWidgets()
Expand Down Expand Up @@ -361,7 +360,8 @@ void GeneralWidget::OnBackendChanged(const QString& backend_name)
const bool supports_adapters = !adapters.empty();

m_adapter_combo->setCurrentIndex(g_Config.iAdapter);
m_adapter_combo->setEnabled(supports_adapters && !Core::IsRunning(Core::System::GetInstance()));
m_adapter_combo->setEnabled(supports_adapters &&
Core::IsUninitialized(Core::System::GetInstance()));

static constexpr char TR_ADAPTER_AVAILABLE_DESCRIPTION[] =
QT_TR_NOOP("Selects a hardware adapter to use.<br><br>"
Expand Down
2 changes: 1 addition & 1 deletion Source/Core/DolphinQt/Debugger/AssemblerWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -791,7 +791,7 @@ bool AssemblerWidget::SaveEditor(AsmEditor* editor)

void AssemblerWidget::OnEmulationStateChanged(Core::State state)
{
m_inject->setEnabled(state != Core::State::Uninitialized);
m_inject->setEnabled(state == Core::State::Running || state == Core::State::Paused);
}

void AssemblerWidget::OnTabClose(int index)
Expand Down
2 changes: 1 addition & 1 deletion Source/Core/DolphinQt/Debugger/BreakpointWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ void BreakpointWidget::UpdateButtonsEnabled()
if (!isVisible())
return;

const bool is_initialised = Core::GetState(m_system) != Core::State::Uninitialized;
const bool is_initialised = Core::IsRunning(m_system);
m_new->setEnabled(is_initialised);
m_load->setEnabled(is_initialised);
m_save->setEnabled(is_initialised);
Expand Down
2 changes: 1 addition & 1 deletion Source/Core/DolphinQt/Debugger/CodeViewWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,7 @@ void CodeViewWidget::OnContextMenu()
QMenu* menu = new QMenu(this);
menu->setAttribute(Qt::WA_DeleteOnClose, true);

const bool running = Core::GetState(m_system) != Core::State::Uninitialized;
const bool running = Core::IsRunning(m_system);
const bool paused = Core::GetState(m_system) == Core::State::Paused;

const u32 addr = GetContextAddress();
Expand Down
2 changes: 1 addition & 1 deletion Source/Core/DolphinQt/Debugger/ThreadWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ void ThreadWidget::Update()

auto& system = Core::System::GetInstance();
const auto emu_state = Core::GetState(system);
if (emu_state == Core::State::Stopping)
if (emu_state == Core::State::Stopping || emu_state == Core::State::Uninitialized)
{
m_thread_table->setRowCount(0);
UpdateThreadContext({});
Expand Down
5 changes: 3 additions & 2 deletions Source/Core/DolphinQt/Debugger/WatchWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -195,10 +195,11 @@ void WatchWidget::Update()

QBrush brush = QPalette().brush(QPalette::Text);

if (!Core::IsRunning(m_system) || !PowerPC::MMU::HostIsRAMAddress(guard, entry.address))
const bool core_is_running = Core::IsRunning(m_system);
if (!core_is_running || !PowerPC::MMU::HostIsRAMAddress(guard, entry.address))
brush.setColor(Qt::red);

if (Core::IsRunning(m_system))
if (core_is_running)
{
if (PowerPC::MMU::HostIsRAMAddress(guard, entry.address))
{
Expand Down
27 changes: 16 additions & 11 deletions Source/Core/DolphinQt/FIFO/FIFOPlayerWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,14 @@ FIFOPlayerWindow::FIFOPlayerWindow(FifoPlayer& fifo_player, FifoRecorder& fifo_r
});

connect(&Settings::Instance(), &Settings::EmulationStateChanged, this, [this](Core::State state) {
if (state == Core::State::Running && m_emu_state != Core::State::Paused)
OnEmulationStarted();
else if (state == Core::State::Uninitialized)
OnEmulationStopped();
m_emu_state = state;
if (state != m_emu_state)
{
if (state == Core::State::Running && m_emu_state != Core::State::Paused)
OnEmulationStarted();
else if (state == Core::State::Uninitialized)
OnEmulationStopped();
m_emu_state = state;
}
});

installEventFilter(this);
Expand Down Expand Up @@ -376,9 +379,11 @@ void FIFOPlayerWindow::UpdateLimits()

void FIFOPlayerWindow::UpdateControls()
{
bool running = Core::IsRunning(Core::System::GetInstance());
bool is_recording = m_fifo_recorder.IsRecording();
bool is_playing = m_fifo_player.IsPlaying();
Core::System& system = Core::System::GetInstance();
const bool core_is_uninitialized = Core::IsUninitialized(system);
const bool core_is_running = Core::IsRunning(system);
const bool is_recording = m_fifo_recorder.IsRecording();
const bool is_playing = m_fifo_player.IsPlaying();

m_frame_range_from->setEnabled(is_playing);
m_frame_range_from_label->setEnabled(is_playing);
Expand All @@ -394,10 +399,10 @@ void FIFOPlayerWindow::UpdateControls()
m_frame_record_count_label->setEnabled(enable_frame_record_count);
m_frame_record_count->setEnabled(enable_frame_record_count);

m_load->setEnabled(!running);
m_record->setEnabled(running && !is_playing);
m_load->setEnabled(core_is_uninitialized);
m_record->setEnabled(core_is_running && !is_playing);

m_stop->setVisible(running && is_recording);
m_stop->setVisible(core_is_running && is_recording);
m_record->setVisible(!m_stop->isVisible());

m_save->setEnabled(m_fifo_recorder.IsRecordingDone());
Expand Down
Loading

0 comments on commit 72cf2bd

Please sign in to comment.