RetroAchievements - Improved Game Loading #12216
Conversation
|
This PR is still in progress and more commits will be added as work on this continues. |
LillyJadeKatrin
force-pushed
the
retroachievements-rehash
branch
from
a4c812c
to
211f3d2
Compare
Ah, the network calls are but the hash calculation is not, good point - I'll move that to the queue. |
LillyJadeKatrin
force-pushed
the
retroachievements-rehash
branch
13 times, most recently
from
49d3f03
to
345b49e
Compare
LillyJadeKatrin
force-pushed
the
retroachievements-rehash
branch
from
8195799
to
b64928c
Compare
LillyJadeKatrin
force-pushed
the
retroachievements-rehash
branch
3 times, most recently
from
f15e1c1
to
8ee76a8
Compare
LillyJadeKatrin
force-pushed
the
retroachievements-rehash
branch
3 times, most recently
from
994ba47
to
b927895
Compare
LillyJadeKatrin
force-pushed
the
retroachievements-rehash
branch
4 times, most recently
from
091fa6d
to
ba54de2
Compare
| @@ -814,8 +988,7 @@ AchievementManager::ResponseType AchievementManager::VerifyCredentials(const std | |||
| return r_type; | |||
| } | |||
|
|
|||
| AchievementManager::ResponseType | |||
| AchievementManager::ResolveHash(std::array<char, HASH_LENGTH> game_hash) | |||
| AchievementManager::ResponseType AchievementManager::ResolveHash(Hash game_hash, u32* game_id) | |||
There was a problem hiding this comment.
A bit tangential, but:
| AchievementManager::ResponseType AchievementManager::ResolveHash(Hash game_hash, u32* game_id) | |
| AchievementManager::ResponseType AchievementManager::ResolveHash(const Hash& game_hash, u32* game_id) |
would avoid an unnecessary copy.
Also maybe just use a struct for the return value instead of an out pointer, since you can just do:
const auto [response, new_game_id] = ResolveHash(current_hash);which would be a little more straightforward and make the sanitizing behavior completely handled by the function, rather than relying on initializing the value at the point of the call (i.e. consider a refactor, where the initialization for the variable pointed to gets removed. with the current code, this would cause an uninitialized read in one path, whereas a return value prevents that from happening entirely)
There was a problem hiding this comment.
Is this a change you feel I should make across the board? Because that's a lot and probably should go in a separate PR if so.
There was a problem hiding this comment.
@lioncash Would still like your opinion on this, many thanks.
LillyJadeKatrin
force-pushed
the
retroachievements-rehash
branch
2 times, most recently
from
c7f8178
to
9f886cf
Compare
|
|
||
| const auto start_session_response = StartRASession(); | ||
| if (start_session_response != ResponseType::SUCCESS) | ||
| m_loading_volume = DiscIO::CreateVolume(volume->GetBlobReader().CopyReader()); |
There was a problem hiding this comment.
This entire construct makes me sad but I don't have any better ideas either with the currently available functions in rcheevos.
LillyJadeKatrin
force-pushed
the
retroachievements-rehash
branch
2 times, most recently
from
5553282
to
7ce7222
Compare
I think I got all of these? Let me know if I've missed anything. |
LillyJadeKatrin
force-pushed
the
retroachievements-rehash
branch
5 times, most recently
from
19d88e8
to
72b9640
Compare
|
Untested but yeah this looks good now. |
LillyJadeKatrin
force-pushed
the
retroachievements-rehash
branch
from
72b9640
to
0ad153b
Compare
This change splits LoadGameAsync into three methods: two HashGame methods to accept either a string filepath or a volume, and a common LoadGameSync that both HashGames call to actually process the code. In the process, some minor cleanup, and the hash resolution now takes place on the work queue asynchronously.
The Disabled state sits between Game Closed and completely Shutdown - stronger than the former, as it refuses to let a game be opened again until AchievementManager is restored (which only happens upon a fresh core boot) but it isn't completely shut down and will still allow the player to be logged in and access the achievement settings and their (global) achievement header.
If the new game ID is a different title than the previous game ID, the achievement manager is compromised.
LillyJadeKatrin
force-pushed
the
retroachievements-rehash
branch
from
0ad153b
to
2bf6ebc
Compare
The changes in this PR are for improving when and how the AchievementManager loads games so it can properly protect against players switching apps and opening homebrews while properly allowing multi-disc or multi-file support. This will happen by calculating a new hash whenever the currently playing media changes, and shutting down the AchievementManager* if the new hash does not reference the same game as the previous hash.
*This does not load a new game, this closes the AchievementManager entirely and it will not be reopened until all games are closed and a new boot is performed.