Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RetroAchievements - Achievement Progress Tab #11878

Conversation

LillyJadeKatrin
Copy link
Contributor

This is a portion of an integration with the RetroAchievements tools and libraries for connecting to the website, downloading data, unlocking achievements and submitting to leaderboards for games running in Dolphin. In this PR, I add an AchievementsProgressWidget and an AchievementsHeaderWidget to be added to AchievementsWindow, for the player to view data about their current progress and points.

@LillyJadeKatrin LillyJadeKatrin force-pushed the retroachievements-achievements-tab branch from 90fb313 to cba345e Compare June 3, 2023 02:31
@AdmiralCurtiss
Copy link
Contributor

Whatever you're trying to do in the last commit, it's extremely overcomplicated. Please just do something like this instead:

FifoPlayer::GetInstance().SetFileLoadedCallback(
[this] { QueueOnObject(this, &FIFOPlayerWindow::OnFIFOLoaded); });
FifoPlayer::GetInstance().SetFrameWrittenCallback([this] {
QueueOnObject(this, [this] {
UpdateInfo();
UpdateControls();
});
});

@AdmiralCurtiss
Copy link
Contributor

This entire thing also feels like full of race conditions. How exactly are you syncing the GUI access with the backend here? Because I'm not seeing anything.

@MayImilae
Copy link
Contributor

So I gave this a try to give it a look, but apparently the progress tab is greyed out if you aren't signed in. Welp, I can't see it on my own. Screenshots please!

@AdmiralCurtiss
Copy link
Contributor

The tab doesn't work when you're logged in either so uuuh

@LillyJadeKatrin
Copy link
Contributor Author

Oh no, my apologies, I told myself I was going to provide instructions for testing and wound up forgetting by the time I got this far. The progress tab requires you to be logged in and playing a game with achievements, because it's for showing the achievements on the current game, and I have verified locally that at least in the attempts I tried, it does do this.
If you insert the following line at the beginning of AchievementManager::ResolveHash (currently line 427), it will load the achievements for Star Wars: The Clone Wars no matter what game you actually have loaded:
game_hash = {"9445e2aad2843e568229e66eb6cb1a91"};

@LillyJadeKatrin LillyJadeKatrin force-pushed the retroachievements-achievements-tab branch 2 times, most recently from 3eacaff to d376873 Compare June 3, 2023 13:59
@LillyJadeKatrin LillyJadeKatrin force-pushed the retroachievements-achievements-tab branch 8 times, most recently from fded3c6 to 26d0bd2 Compare June 11, 2023 03:31
@LillyJadeKatrin LillyJadeKatrin force-pushed the retroachievements-achievements-tab branch 3 times, most recently from 89a4a4e to ff723b0 Compare June 17, 2023 20:40
@AdmiralCurtiss
Copy link
Contributor

The disabled tab is a UX nightmare, tabs just don't do that. Please find a different solution for that one. Otherwise this seems fine enough to me. It's not winning a prize for beauty but whatever.

a
b

@LillyJadeKatrin
Copy link
Contributor Author

LillyJadeKatrin commented Jul 2, 2023

The disabled tab is a UX nightmare, tabs just don't do that. Please find a different solution for that one. Otherwise this seems fine enough to me. It's not winning a prize for beauty but whatever.

Not sure I follow what you mean about the disabled tab - what exactly is the problem? Would a good solution allow the player to click the tab but the widget then only shows "must have game loaded" or somesuch?

@AdmiralCurtiss
Copy link
Contributor

Disabled tabs just aren't a thing in any modern UI. As you saw in the opening comments of this PR thread, people will just click on it and be confused why it's not switching to the tab.

Yes, the tab just holding a 'no game loaded' message would be perfectly fine. As would be hiding the tab completely if it contains nothing of value.

@LillyJadeKatrin LillyJadeKatrin force-pushed the retroachievements-achievements-tab branch 2 times, most recently from cf61c35 to 4437a98 Compare July 2, 2023 01:52
Added some small methods to AchievementManager to expose useful data for displaying in an achievement UI. Also moved a couple things from private to public for the same purpose.
This widget displays a header on the AchievementsWindow dialog above the tabs that shows the currently logged in user (if there is one) and the game they are playing (if there is one).
This widget is a tab in the AchievementsWindow that displays the player's current achievement progress: which achievements are locked or unlocked, and the progress of achievements that have progress metrics.
AchievementManager now has a SetUpdateCallback method for providing a single universal callback for anytime something important changes in the achievement state, such as logging in/out, game load/close, or events such as achievement unlocks. AchievementsWindow sets this callback in its own init to its UpdateData method so that the AchievementsWindow gets updated when one of these changes takes place.
Expanded the use of the lock mutex already used for loading the player's existing unlock status to guard against races involving the Achievements dialog window reading from data AchievementManager might be in the process of updating. The lock has been exposed publicly and the AchievementsWindow uses it in its UpdateData method, and anywhere else that might modify data used to render that window has also been wrapped with it.
@LillyJadeKatrin LillyJadeKatrin force-pushed the retroachievements-achievements-tab branch from 4437a98 to ccc9d0e Compare July 2, 2023 02:26
@AdmiralCurtiss AdmiralCurtiss merged commit da2784a into dolphin-emu:master Jul 3, 2023
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants