-
-
Notifications
You must be signed in to change notification settings - Fork 99
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
feat(multiset): send sets to emulators #2857
Open
wescopeland
wants to merge
25
commits into
RetroAchievements:master
Choose a base branch
from
wescopeland:multiset-patch-data
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 18 commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
a801b78
refactor(AchievementFlag): use a backed enum
wescopeland 76b0af4
fix: game page flag
wescopeland cb43c1a
feat(multiset): add action to resolve sets for emulators
wescopeland b9c13d7
Merge branch 'master' into multiset-resolve-sets-action
wescopeland e26484c
ci: fallback install
wescopeland f0c5dbd
Merge branch 'multiset-resolve-sets-action' of https://github.com/wes…
wescopeland f241d39
chore: revert ci change
wescopeland d550f43
feat: include core game ids
wescopeland 40cc59a
chore: clarify
wescopeland c2d9446
feat(multiset): send sets to emulators
wescopeland 216e613
Merge branch 'master' into multiset-resolve-sets-action
wescopeland 7b598ab
Merge branch 'multiset-resolve-sets-action' into multiset-patch-data
wescopeland 0e17857
Merge branch 'master' into multiset-resolve-sets-action
wescopeland 1ae1571
Merge branch 'multiset-resolve-sets-action' into multiset-patch-data
wescopeland 7279a62
Merge branch 'master' into multiset-patch-data
wescopeland c939cb8
refactor: use an inject action, change namespace
wescopeland 75f2b68
test: add coverage
wescopeland 472ff2d
chore: remove todo
wescopeland c0daa17
fix: address pr feedback
wescopeland d885e96
Merge branch 'master' into multiset-patch-data
wescopeland 8919e5c
fix: use correct rp
wescopeland 0eed34b
fix: adjust root data
wescopeland 081ad25
test: more edge case coverage
wescopeland 7f162a9
Merge branch 'master' into multiset-patch-data
wescopeland 3b904a4
Merge branch 'master' into multiset-patch-data
wescopeland File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,245 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace App\Connect\Actions; | ||
|
||
use App\Models\Achievement; | ||
use App\Models\Game; | ||
use App\Models\GameAchievementSet; | ||
use App\Models\GameHash; | ||
use App\Models\PlayerGame; | ||
use App\Models\User; | ||
use App\Platform\Enums\AchievementFlag; | ||
use Illuminate\Database\Eloquent\Collection; | ||
use InvalidArgumentException; | ||
|
||
// OPEN QUESTION when given a GameHash, should the root Achievements & Leaderboards be returned as `null`? | ||
|
||
class BuildClientPatchDataAction | ||
{ | ||
/** | ||
* Assembles a patch data package of all components needed by emulators: | ||
* - Basic game information (title, system, etc.) | ||
* - Achievement definitions and unlock conditions | ||
* - Leaderboard configurations | ||
* - Rich presence script | ||
* | ||
* Modern rcheevos integrations send the game hash. Legacy integrations send only | ||
* the game. We need to support constructing the patch data package for both situations. | ||
* | ||
* @param GameHash|null $gameHash The game hash to build patch data for | ||
* @param Game|null $game The game to build patch data for | ||
* @param User|null $user The current user requesting the patch data (for player count calculations) | ||
* @param AchievementFlag|null $flag Optional flag to filter the achievements by (eg: only official achievements) | ||
* @throws InvalidArgumentException when neither $gameHash nor $game is provided | ||
*/ | ||
public function execute( | ||
?GameHash $gameHash = null, | ||
?Game $game = null, | ||
?User $user = null, | ||
?AchievementFlag $flag = null, | ||
): array { | ||
if (!$gameHash && !$game) { | ||
throw new InvalidArgumentException('Either gameHash or game must be provided to build patch data.'); | ||
} | ||
|
||
$coreGame = $gameHash->game ?? $game; | ||
$coreAchievementSet = GameAchievementSet::where('game_id', $coreGame->id) | ||
->core() | ||
->with('achievementSet.achievements.developer') | ||
->first(); | ||
|
||
$gamePlayerCount = $this->calculateGamePlayerCount($coreGame, $user); | ||
|
||
return [ | ||
'Success' => true, | ||
'PatchData' => [ | ||
...$this->buildBaseGameData($coreGame), | ||
|
||
'Achievements' => $coreAchievementSet | ||
? $this->buildAchievementsData($coreAchievementSet, $gamePlayerCount, $flag) | ||
: [], | ||
|
||
'Leaderboards' => $this->buildLeaderboardsData($coreGame), | ||
|
||
// Don't even send a 'Sets' value to legacy clients. | ||
...($gameHash ? ['Sets' => $this->buildSetsData($gameHash, $user, $gamePlayerCount, $flag)] : []), | ||
], | ||
]; | ||
} | ||
|
||
/** | ||
* Builds achievement set data for multiset support. | ||
* | ||
* @param GameHash|null $gameHash The game hash to build set data for | ||
* @param User|null $user The current user requesting the data | ||
* @param int $gamePlayerCount Total player count for rarity calculations | ||
* @param AchievementFlag|null $flag Optional flag to filter the achievements by (eg: only official achievements) | ||
*/ | ||
private function buildSetsData( | ||
?GameHash $gameHash, | ||
?User $user, | ||
int $gamePlayerCount, | ||
?AchievementFlag $flag, | ||
): array { | ||
if (!$gameHash || !$user) { | ||
return []; | ||
} | ||
|
||
$resolvedSets = (new ResolveAchievementSetsAction())->execute($gameHash, $user); | ||
|
||
// Don't fetch the games in the loop. Grab them all in a single query. | ||
$coreGameIds = $resolvedSets->pluck('core_game_id')->unique(); | ||
$games = Game::whereIn('ID', $coreGameIds)->get()->keyBy('ID'); | ||
|
||
$sets = []; | ||
foreach ($resolvedSets as $resolvedSet) { | ||
$setGame = $games[$resolvedSet->core_game_id]; | ||
|
||
$sets[] = [ | ||
'GameAchievementSetID' => $resolvedSet->id, | ||
'CoreGameID' => $resolvedSet->core_game_id, | ||
'Title' => $resolvedSet->title, | ||
'Type' => $resolvedSet->type->value, | ||
'ImageIcon' => $setGame->ImageIcon, | ||
'ImageIconURL' => media_asset($setGame->ImageIcon), | ||
'Achievements' => $this->buildAchievementsData($resolvedSet, $gamePlayerCount, $flag), | ||
'Leaderboards' => $this->buildLeaderboardsData($setGame), | ||
]; | ||
} | ||
|
||
return $sets; | ||
} | ||
|
||
/** | ||
* Builds achievement information needed by emulators. | ||
* | ||
* @param GameAchievementSet $gameAchievementSet The achievement set to build achievement data for | ||
* @param int $gamePlayerCount The total number of players (minimum of 1 to prevent division by zero) | ||
* @param AchievementFlag|null $flag Optional flag to filter the achievements by (eg: only official achievements) | ||
*/ | ||
private function buildAchievementsData( | ||
GameAchievementSet $gameAchievementSet, | ||
int $gamePlayerCount, | ||
?AchievementFlag $flag, | ||
): array { | ||
/** @var Collection<int, Achievement> $achievements */ | ||
$achievements = $gameAchievementSet->achievementSet | ||
->achievements() | ||
->with('developer') | ||
->orderBy('DisplayOrder') // explicit display order | ||
->orderBy('ID') // tiebreaker on creation sequence | ||
->get(); | ||
|
||
if ($flag) { | ||
$achievements = $achievements->where('Flags', '=', $flag->value); | ||
} | ||
|
||
$achievementsData = []; | ||
|
||
foreach ($achievements as $achievement) { | ||
// If an achievement has an invalid flag, skip it. | ||
if (!AchievementFlag::tryFrom($achievement->Flags)) { | ||
continue; | ||
} | ||
|
||
// Calculate rarity assuming it will be used when the player unlocks the achievement, | ||
// which implies they haven't already unlocked it. | ||
$rarity = min(100.0, round((float) ($achievement->unlocks_total + 1) * 100 / $gamePlayerCount, 2)); | ||
$rarityHardcore = min(100.0, round((float) ($achievement->unlocks_hardcore_total + 1) * 100 / $gamePlayerCount, 2)); | ||
|
||
$achievementsData[] = [ | ||
'ID' => $achievement->id, | ||
'MemAddr' => $achievement->MemAddr, | ||
'Title' => $achievement->title, | ||
'Description' => $achievement->description, | ||
'Points' => $achievement->points, | ||
'Author' => $achievement->developer->display_name ?? '', | ||
'Modified' => $achievement->DateModified->unix(), | ||
'Created' => $achievement->DateCreated->unix(), | ||
'BadgeName' => $achievement->BadgeName, | ||
'Flags' => $achievement->Flags, | ||
'Type' => $achievement->type, | ||
'Rarity' => $rarity, | ||
'RarityHardcore' => $rarityHardcore, | ||
'BadgeURL' => $achievement->badge_unlocked_url, | ||
'BadgeLockedURL' => $achievement->badge_locked_url, | ||
]; | ||
} | ||
|
||
return $achievementsData; | ||
} | ||
|
||
/** | ||
* Builds the basic game information needed by emulators. | ||
*/ | ||
private function buildBaseGameData(Game $game): array | ||
{ | ||
return [ | ||
'ID' => $game->id, | ||
'Title' => $game->title, | ||
'ImageIcon' => $game->ImageIcon, | ||
'RichPresencePatch' => $game->RichPresencePatch, | ||
'ConsoleID' => $game->ConsoleID, | ||
'ImageIconURL' => media_asset($game->ImageIcon), | ||
]; | ||
} | ||
|
||
/** | ||
* Builds leaderboard information needed by emulators. | ||
*/ | ||
private function buildLeaderboardsData(Game $game): array | ||
{ | ||
$leaderboardsData = []; | ||
|
||
// TODO detach leaderboards from games | ||
$leaderboards = $game->leaderboards() | ||
->orderBy('DisplayOrder') // explicit display order | ||
->orderBy('ID') // tiebreaker on creation sequence | ||
->get(); | ||
|
||
foreach ($leaderboards as $leaderboard) { | ||
$leaderboardsData[] = [ | ||
'ID' => $leaderboard->id, | ||
'Mem' => $leaderboard->Mem, | ||
'Format' => $leaderboard->Format, | ||
'LowerIsBetter' => $leaderboard->LowerIsBetter, | ||
'Title' => $leaderboard->title, | ||
'Description' => $leaderboard->Description, | ||
'Hidden' => ($leaderboard->DisplayOrder < 0), | ||
]; | ||
} | ||
|
||
return $leaderboardsData; | ||
} | ||
|
||
/** | ||
* Calculates the total number of players for the game, which ultimately gets used in | ||
* achievement rarity calculations. | ||
* | ||
* This method adds 1 to the total if the requesting user hasn't played the game yet, | ||
* which ensures accurate rarity predictions for when they unlock achievements. | ||
* | ||
* @param Game $game The game to calculate player count for | ||
* @param User|null $user The current user requesting the data | ||
* | ||
* @return int The total number of players (minimum of 1 to prevent division by zero) | ||
*/ | ||
private function calculateGamePlayerCount(Game $game, ?User $user): int | ||
{ | ||
$gamePlayerCount = $game->players_total; | ||
|
||
if ($user) { | ||
$hasPlayerGame = PlayerGame::whereUserId($user->id) | ||
->whereGameId($game->id) | ||
->exists(); | ||
|
||
if (!$hasPlayerGame) { | ||
$gamePlayerCount++; | ||
} | ||
} | ||
|
||
return max(1, $gamePlayerCount); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace App\Connect\Actions; | ||
|
||
use App\Enums\ClientSupportLevel; | ||
use App\Platform\Services\UserAgentService; | ||
|
||
class GetClientSupportLevelAction | ||
{ | ||
public function execute(string $userAgent): ClientSupportLevel | ||
{ | ||
$userAgentService = new UserAgentService(); | ||
|
||
return $userAgentService->getSupportLevel($userAgent); | ||
} | ||
} |
79 changes: 79 additions & 0 deletions
79
app/Connect/Actions/InjectPatchClientSupportLevelDataAction.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace App\Connect\Actions; | ||
|
||
use App\Enums\ClientSupportLevel; | ||
use App\Models\Achievement; | ||
use App\Models\Game; | ||
use App\Models\GameHash; | ||
use App\Platform\Enums\AchievementFlag; | ||
use Carbon\Carbon; | ||
use InvalidArgumentException; | ||
|
||
class InjectPatchClientSupportLevelDataAction | ||
{ | ||
/** | ||
* @param array $constructedPatchData Return value of BuildClientPatchDataAction::execute() | ||
* @param ClientSupportLevel $clientSupportLevel The current support level of the user's client/emulator | ||
* @param GameHash|null $gameHash The game hash patch data was possibly built for | ||
* @param Game|null $game The game patch data was possibly built for | ||
*/ | ||
public function execute( | ||
array $constructedPatchData, | ||
ClientSupportLevel $clientSupportLevel, | ||
?GameHash $gameHash = null, | ||
?Game $game = null, | ||
): array { | ||
if (!$gameHash && !$game) { | ||
throw new InvalidArgumentException('Either gameHash or game must be provided to return a patch data response.'); | ||
} | ||
|
||
$coreGame = $gameHash->game ?? $game; | ||
$canAddWarningAchievement = $coreGame->achievements_published < 0; // will never be true. change to > when ready | ||
|
||
if ($clientSupportLevel !== ClientSupportLevel::Full && $canAddWarningAchievement) { | ||
// We intentionally place the warning achievement at the top of the list. | ||
$constructedPatchData['Achievements'] = [ | ||
$this->buildClientSupportWarningAchievement($clientSupportLevel), | ||
...$constructedPatchData['Achievements'], | ||
]; | ||
} | ||
|
||
if ($clientSupportLevel === ClientSupportLevel::Unknown) { | ||
$constructedPatchData['Warning'] = 'The server does not recognize this client and will not allow hardcore unlocks. Please send a message to RAdmin on the RetroAchievements website for information on how to submit your emulator for hardcore consideration.'; | ||
} | ||
|
||
return $constructedPatchData; | ||
} | ||
|
||
/** | ||
* This warning achievement should appear at the top of the emulator's achievements | ||
* list. It should automatically unlock after a few seconds of patch data retrieval. | ||
* The intention is to notify a user that they are using an outdated client | ||
* and need to update, as well as what the repercussions of their continued | ||
* play session with their current client might be. | ||
*/ | ||
private function buildClientSupportWarningAchievement(ClientSupportLevel $clientSupportLevel): array | ||
{ | ||
return [ | ||
'ID' => Achievement::CLIENT_WARNING_ID, | ||
'MemAddr' => '1=1.300.', // pop after 5 seconds | ||
'Title' => ($clientSupportLevel === ClientSupportLevel::Outdated) ? | ||
'Warning: Outdated Emulator (please update)' : 'Warning: Unknown Emulator', | ||
'Description' => 'Hardcore unlocks cannot be earned using this emulator.', | ||
'Points' => 0, | ||
'Author' => '', | ||
'Modified' => Carbon::now()->unix(), | ||
'Created' => Carbon::now()->unix(), | ||
'BadgeName' => '00000', | ||
'Flags' => AchievementFlag::OfficialCore->value, | ||
'Type' => null, | ||
'Rarity' => 0.0, | ||
'RarityHardcore' => 0.0, | ||
'BadgeURL' => media_asset("Badge/00000.png"), | ||
'BadgeLockedURL' => media_asset("Badge/00000_lock.png"), | ||
]; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a discussion point I'm not sure how to best handle. I also referenced this in a test:
RAWeb/tests/Feature/Connect/Actions/BuildClientPatchDataActionTest.php
Lines 402 to 407 in 75f2b68
Given that the user is using a modern client and we send a list of sets to that client from
r=patch
, what should the root-level data be?Achievements
andLeaderboards
even matter?Title
,ImageIcon
, andImageIconURL
always point to the core game?RichPresencePatch
take on the RP script of the "subset game"?For reference, here is a full data structure of the response in the test case above:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that if the client is requesting patch data by a subset hash, the top-level data should be from the core set, and the subset as a child record.
The response structure still has root-level fields, it would make sense to return the core data at the root level.
For legacy compatibility, when requesting a subset by game_id, the subset information should be returned at the top level, and no child records should be returned.
If we're going to eliminate the top-level records, maybe we should have a new API (i.e.
patch2
) rather than trying to overload the existing API.I think subsets shouldn't have their own RP scripts. This may need further discussion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It sounds like elimination of the top-level records is probably not the right way to go (which also aligns with how this PR is currently implemented). We just need to make sure the top-level values being sent to emulators are correct.
Just to confirm my understanding:
In other words, let's say I load "Dragon Quest III [Subset - Bonus]". Is it correct that the top-level
Title
,ImageIcon
,ImageIconURL
,Achievements
, andLeaderboards
should be from "Dragon Quest III", not "Dragon Quest III [Subset - Bonus]"?Agreed - this fallback functionality should already be present in this PR.
This is where I feel less sure. Let's have a follow-up chat, but I can imagine some developers will get frustrated if their RP scripts attached to subsets are suddenly considered invalid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes.
Are there non-exclusive subsets with unique RP?
Subset:
Core:
Subset:
Core:
Subset:
Core:
Found one. Though I could argue that the core RP is better. Maybe the prefix could be merged into that?
Subset:
Core:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps since "specialty" and "exclusive" subsets require loading unique hashes, they could also have unique RP, but generic "bonus" sets would not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All this logic should now be implemented in latest, with additional tests added to verify it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thought related to RP: how are we going to maintain/create sessions for the subsets? Will
ping
be overloaded to send all game_ids that are active? Will we just run the active subset logic on the server like we do for building the patch data?I thin this comes down to "specialty" and "exclusive" subsets again. Do we still need "active players" for generic bonus subsets?
Do we still need subset sessions in the database for associating unlocks, or can they be tied to the core subset?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My gut feelings are:
ping
should never consider more than 1 game ID as being active.ID
that's sent back frompatch
at the root level.We should probably tackle this piece in a subsequent PR.