RetroAchievements: Delay calling LoadApprovedList

[JosJuice]

0c14b0c made Dolphin load a file from the Sys folder the first time AchievementManager::GetInstance() is called. Because Android calls AchievementManager::GetInstance() from setBackgroundExecutionAllowedNative, this had two negative consequences on Android:

1. The first time setBackgroundExecutionAllowedNative gets called is often before directory initialization is done. Getting the path of the Sys folder before directory initialization is done causes a crash.
2. setBackgroundExecutionAllowedNative is called from the GUI thread, and we don't want file I/O on the GUI thread for performance reasons.

This change makes us load the data from the Sys folder the first time the data is needed instead. This also saves us from having to load the data at all when hardcore mode is inactive.

[Dentomologist]

Untested, and I don't know anything about the interaction with Android here, but this looks reasonable.

[Thinking out loud] Before, m_ini_root was thread safe both because of the explicit lock_guard in LoadApprovedList and because it was only loaded in the AchievementManager constructor, which was only called by the static local declared in GetInstance which in turn is guaranteed (by virtue of the rules surrounding initialization of static local variables) to be thread safe.

Now thread safety is managed by the fact that the only user of m_ini_root is FilterApprovedPatches, which is only called by LoadPatches in PatchEngine.cpp and uses AchievementManager's lock. This works, but I think it would make sense to just use the lock directly in FilterApprovedPatches since it's necessary anyway and would prevent problems if we hypothetically called the function from somewhere else and forgot to grab the lock.

[JosJuice]

The locking we had before didn't actually add any thread safety. LoadApprovedList was guarding m_ini_root using m_lock, but FilterApprovedPatches wasn't using m_lock at all, so the use of m_lock in LoadApprovedList was pointless. But regardless, I've made FilterApprovedPatches use m_lock like you suggested so that nothing goes wrong if we call FilterApprovedPatches from multiple threads in the future.

[AdmiralCurtiss]

This seems fine and apparently fixes a crash that I can't verify (the Android code flow eludes me), so I guess we should merge it.

[JosJuice]

The locking we had before didn't actually add any thread safety. LoadApprovedList was guarding m_ini_root using m_lock, but FilterApprovedPatches wasn't using m_lock at all, so the use of m_lock in LoadApprovedList was pointless.

I found out now that the caller of FilterApprovedPatches was locking. That's very strange design, but anyway it seems reasonable to lock inside FilterApprovedPatches.