meleu / RetroArch-problematic-cheevos

A place to put the savestates of games with cheevos problems when running in RetroArch and no problems in official emulators.
6 stars 10 forks source link

Leaderboards are not getting cancelled / reset on game reset #38

Closed kdecks closed 5 years ago

kdecks commented 5 years ago

This is the intended behavior of RAemus and RetroArch should match it.

More often than not it does not create a problem, but in some cases impossibly high scores are submitted or allow users to cheat leaderboards.

For example using SNES9x 2005 for Super Ghouls n Ghosts leaderboard on reset fills the scores with all 5s and submits.

meleu commented 5 years ago

I suspect it's related with the issue reported by @Jamiras in libretro/RetroArch#6705 (see the quote below), and it's already solved in rcheevos (which is not yet officially integrated in RetroArch).

The inconsistency:

A leaderboard may be defined as follows:

  • STA:0x1234=8_0x5678=9::CAN:0x1234=6_0x5678=0::SUB:0x1234=9_0x5678=4 This says:

  • start if $1234 is 8 and $5678 is 9.

  • cancel if $1234 is 6 or $5678 is 0.

  • submit if $1234 is 9 and $5678 is 4.

Note that start and submit use _ for AND, and cancel uses _ for OR.

The recent changes to RAIntegration make cancel also use _ for AND. 'S' is used for OR, which is consistent with start and submit formats, and more importantly, consistent with achievement formats.

Jamiras commented 5 years ago

RetroArch always used "_" for AND in cancel - it was a previous bug that was resolved by the modifications in the DLL.

A few months ago, we noticed an inconsistency in how RAIntegration deserialized the leaderboard cancel condition. This has recently been corrected in the RAIntregration project.

1.7.4 added support for OR in leaderboards - see http://docs.retroachievements.org/Achievement-Logic-Features/#minimum-required-versions

Jamiras commented 5 years ago

Related code:

When a leaderboard is canceled, it's active flag is set to false. it seems like it should be easy enough to do that in the highlighted code above.

However, I believe the rcheevos implementation should be scrapped and rewritten to use rc_evaluate_lboard to better manage the state transitions and then call rc_reset_lboard on reset to clear out all hits instead of just deactivating the leaderboard.

meleu commented 5 years ago

Thanks for pointing the code. I'll try to submit a PR soon.

meleu commented 5 years ago

The fix was merged.

We'll see the fix alive in the next releases.

Jamiras commented 5 years ago

This still needs to be addressed, but we can worry about that when we get to testing the new cheevos code.

However, I believe the rcheevos implementation should be scrapped and rewritten to use rc_evaluate_lboard to better manage the state transitions and then call rc_reset_lboard on reset to clear out all hits instead of just deactivating the leaderboard.