RAWeb icon indicating copy to clipboard operation
RAWeb copied to clipboard

add flag to retrieve subset data with patch data for game

Open Jamiras opened this issue 2 years ago • 13 comments

When calling dorequest.php?r=patch with the new s=1 flag, subset data will also be returned (subsets are identified the same way as on the game info page).

    ...
    "Achievements": [ ... ],
    "Leaderboards": [ ... ],
    "Subsets": [
      {
        "ID": "5948",
        "Title": "Bonus",
        "Achievements": [ ... ],
        "Leaderboards": [ ... ],
      },
      {
        "ID": "6836",
        "Title": "Multi",
        "Achievements": [ ... ],
        "Leaderboards": [ ... ],
      }
    ]

This could allow clients the ability to unlock regular and bonus achievements from the base ROM (without patching). The client would have to specifically request and activate the subset achievements.

Supports #707. More thought is required on how to activate them and should be discussed there.

Jamiras avatar Jul 23 '22 17:07 Jamiras

I thought about this for a while and I'd suggest solving this differently.

Considerations:

  1. backwards compatibility with old clients
  2. expose which achievement belongs to which set in the client (like DLCs on other platforms)
  3. allow users to opt-in / out of subsets

The current implementation would not allow for 1 but makes it easier to implement 2 in clients. 3 is assumed to be controlled on the client side.

I'd suggest serving patch data as usual but extending the list of achievements instead. adding a qualifier/type to each of the achievements would allow the client to determine whether an achievement is from the main set or from a subset by filtering.

3 is a matter of control. client, server, or both. we can either allow users to have a setting on the client side (conflicting with 1) or let them control subset behaviour on the server side (via site settings). by serving achievements in one list we can have both but it requires a bit more work on the client side while providing full backwards compatibility for old clients.

if we chose to serve subsets at all times in patch data then the client could still provide an option to decide whether they should be processed. on legacy clients that would be an automatic opt-in at all times. we could check the user agent for older clients to make that an opt-in via site settings to prevent old hardware from suffering through too many checks.

luchaos avatar Aug 07 '22 01:08 luchaos

Adding to the above - the qualifier/type could as well be just the game ID added to the achievements in the list as a "Set ID".

An additional enumeration value that identifies the type of set with a key; could be "main, bonus, subset_x, ...".

"PatchData": {
    "ID": 1000,
    "Title": "Game 1000 (happens to be the Set ID in V1)",
    "Sets": [
        {
            "ID": 1000,
            "key": "main",
            "name": "Main Set",
        },
        {
            "ID": 1001,
            "key": "bonus_x",
            "name": "Bonus Set - this used to be 'Game 1000 [Subset - Bonus]' with ID 1001",
        },
        {
            "ID": 666,
            "key": "pain",
            "name": "Set for masochists",
        },
    ],
    "Achievements": [
        {
            "ID": 123,
            "MemAddr": "...",
            "Title": "Achievement TItle",
            "Description": "Achievement Description",
            "Points": 25,
            "Author": "Author",
            "Modified": 1599999999,
            "Created": 1599999999,
            "BadgeName": "12345",
            "Flags": 3,
            "SetID": 1000,
            "SetKey": "main"
        },
        {
            "ID": 234,
            "MemAddr": "...",
            "Title": "Bonus Achievement TItle",
            "Description": "Bonus Achievement Description",
            "Points": 100,
            "Author": "Author",
            "Modified": 1599999999,
            "Created": 1599999999,
            "BadgeName": "23456",
            "Flags": 3,
            "SetID": 876,
            "SetKey": "bonus_x"
        },
        {
            "ID": 666666,
            "MemAddr": "...",
            "Title": "Pain Achievement TItle",
            "Description": "Pain Achievement Description",
            "Points": 100,
            "Author": "Author",
            "Modified": 1599999999,
            "Created": 1599999999,
            "BadgeName": "34567",
            "Flags": 3,
            "SetID": 666,
            "SetKey": "pain"
        },
    ]
}

In V2 there is no such thing a an intrinsic set for a game - it will have multiple sets attached to it where one is typed as the main set in some form or another; the above would be a suggested implementation for that. The set IDs in the achievements list within patch data would be the actual set ID eventually.

luchaos avatar Aug 07 '22 01:08 luchaos

The current implementation would not allow for [backwards compatibility] but makes it easier to implement [subset grouping] in clients. [opt-in] is assumed to be controlled on the client side.

I assumed backwards compatibility would be to not include the subset achievements as older clients would not be able to filter them or exclude them. And as this is a desirable feature for users, it's a carrot to encourage them to update their old clients.

If merged, how would order be determined? Achievements in each subset have their own DisplayOrder values. Would the subsets be appended, or interleaved into the main set?

With this particular implementation, the older clients wouldn't be asking for the subsets (not passing s=1), so I guess it wouldn't affect the behavior. They'd still only get the primary achievements and no subsets.

There seems to be some merit into making the opt-in be a server setting as a user would not have to opt out through every client, but that could be managed as a value returned with the login response. Actually doing the filter on the server would eliminate any possibility of the client manually enabling the subsets they'd be interested in.

Jamiras avatar Aug 07 '22 01:08 Jamiras

backwards compatibility

I get the carrot argument. I assume we have far more outdated versions of RetroArch out in the wild than we like. Not only will it be hard to communicate to them but it requires additional work in the integration that has to be rolled out. Changing the response structure ties us to maintaining the old versions more than it already does now. By keeping one list we wouldn't introduce such intermediate version steps that have to be maintained.

Would the subsets be appended, or interleaved into the main set

Appended. When implemented the client could chose to separate the lists into a DLC style structure (https://i.redd.it/kojvfe7i7yh41.jpg) The order within those sets still applies within themselves, not interleaved.

older clients wouldn't be asking for the subsets (not passing s=1), so I guess it wouldn't affect the behavior

Personally, I'd force subsets onto the users 😅 Give them everything by default.

They'd still only get the primary achievements and no subsets.

This binds us to keeping the game IDs for subset games' hashes intact. I'd move those hashes to the games' main set instead and get rid of the patching workflow altogether, just keep those hashes for backwards compatibility.

I'd make it opt-out via site instead. If a user unlocked achievements for a subset that they don't want they can turn that set off on the site and reset the achievements in question.

luchaos avatar Aug 07 '22 02:08 luchaos

If merged, how would order be determined? Achievements in each subset have their own DisplayOrder values. Would the subsets be appended, or interleaved into the main set?

I've always pictured it being similar to how PSN handles DLC nioh

televandalist avatar Aug 07 '22 02:08 televandalist

I've always pictured it being similar to how PSN handles DLC

Same - that's what the link above shows https://i.redd.it/kojvfe7i7yh41.jpg

The question now is just whether we'd want to list all of the sets' achievements in one list for old clients (which assumes additional filtering has to be done on the client side) or if we provide that only to new clients by adding subsets as a separate entity in the response.

luchaos avatar Aug 07 '22 02:08 luchaos

If we go with the separate list of subsets as it's currently implemented - is there a need for the s flag?

luchaos avatar Aug 07 '22 02:08 luchaos

Changing the response structure ties us to maintaining the old versions more than it already does now. By keeping one list we wouldn't introduce such intermediate version steps that have to be maintained.

I don't understand this argument. Both solutions introduce new fields for the new data. Mine has a new top-level array (Subsets). Yours has new nested fields (SetID, SetKey). In both cases, the old clients wouldn't be looking for them, so they'd just be ignored.

The main difference between my solution and yours is whether or not the subset data is visible to older clients. With my solution, it wouldn't be. With yours, it would always be (assuming we eliminate the s=1 parameter). The correct solution (which would require client changes) would be to group the subset data into distinct units that could be disabled by the user.

Whether we do that via lists in the response or attributes on the achievements doesn't really matter. Lists seems like a cleaner solution as everything would already be grouped for the client. The only reason I can see for putting everything in the top-level list is to make the subset achievements available to older clients, but as I mentioned above, without the ability to filter and/or disable the subset data, this seems undesirable

This binds us to keeping the game IDs for subset games' hashes intact. I'd move those hashes to the games' main set instead and get rid of the patching workflow altogether, just keep those hashes for backwards compatibility.

Having subsets returned with the main game eliminates the patching workflow.

Because subsets are currently managed as alternate games, the client needs the game ID so it can be passed back in some of the APIs (particularly uploadachievement).

Moving subsets into an actual child element of game is a much larger change, and may be desirable before moving forward with this solution. I just saw an opportunity to leverage the logic that already exists for determining subsets for the webpage in a way as towards implementing one of the things that's always been at the top of my missing feature lists.

Jamiras avatar Aug 07 '22 02:08 Jamiras

If we go with the separate list of subsets as it's currently implemented - is there a need for the s flag?

Not really. Just saves doing additional queries on the server. But as we want users to upgrade clients, ideally that benefit wouldn't last long.

Jamiras avatar Aug 07 '22 02:08 Jamiras

I think this would be big enough of a deal that having users upgrade clients wouldn't be an issue so long as everyone knows what's up. 😁

televandalist avatar Aug 07 '22 02:08 televandalist

The question now is just whether we'd want to list all of the sets' achievements in one list for old clients (which assumes additional filtering has to be done on the client side) or if we provide that only to new clients by adding subsets as a separate entity in the response.

We can't add additional filtering to the client side for old clients. That's the crux of my argument for putting them in a separate list.

The achievement list displayed in an older version of RetroArch would be something like:

* Achievement 1
* Achievement 2
...
* Achievement 49
* Achievement 50
* Achievement A
* Achievement B
...

And there wouldn't be anything indicating that "Achievement A" and "Achievement B" were from a bonus set.

Jamiras avatar Aug 07 '22 02:08 Jamiras

The main difference between my solution and yours is whether or not the subset data is visible to older clients.

I guess that's what it boils down to, you're right. The response structure argument doesn't make a difference, agreed.

I always assumed that I'd implement it with full backwards compatibility for old clients in mind. Serving more achievements to the users as soon as possible seemed favorable to me as that would allow for more unlocks from the get go and users could still opt out if they wanted to. Then again, that means that the opt-out site feature would have to exist right away.

The implemented structure makes it more clear which achievements belong to the main set, too. I like that as it's easier to reason about.

I'm fine with the current approach - happy to have it thought through with your inputs nonetheless, thanks! 👍

luchaos avatar Aug 07 '22 02:08 luchaos

That said, enabling subsets for old clients - if we should stumble upon that that's necessary for some reason - could still be an opt-in feature or a simple user agent check which would then merge the sets into one list.

The more i think about it the more I can see why we should go with the current implementation. It's far more flexible this way.

luchaos avatar Aug 07 '22 02:08 luchaos

There's some discussion in here that's probably worth revisiting when we do start on the feature, but these changes are no longer compatible with the current codebase. Closing.

Jamiras avatar Dec 03 '23 17:12 Jamiras