libretro / RetroArch

Cross-platform, sophisticated frontend for the libretro API. Licensed GPLv3.
http://www.libretro.com
GNU General Public License v3.0
10.3k stars 1.84k forks source link

Retroachievements savestate in Hardcore mode #8725

Closed FeRcHuLeS closed 4 years ago

FeRcHuLeS commented 5 years ago

Description

This feature is disabled by default for obvious reasons, so my point is to save state with no load state in Hardcore mode because we usually savestates in final bosses, endings and to save highscores, Actually it can be saved a state by disabling Hardcore mode and can't continue your game session in Hardcore mode because you can't go back to that mode without the game is reset.

So just savestate in Hardcore mode and keep disabled the load state option as always, this fact don't let gamers to cheat for achievements, and the states saved could only be loaded in Normal mode or with Retroachievements disabled.

Expected behavior

-Save state only in Hardcore mode

Actual behavior

-Lack of it

Version

orbea commented 5 years ago

@FeRcHuLeS Can you please try the recent 1.7.7 release or a current nightly? I think this may of changed since 1.7.6.

FeRcHuLeS commented 5 years ago

I don think so, cause in the 1.7.7 general changelog there is nothing related:

– CHEEVOS: Fix crash when reading memory that is out of range. – CHEEVOS: New Cheevos implementation enabled by default. – CHEEVOS: Pop-up badges when an achievement is triggered.

This is a fair request, please consider adding it

orbea commented 5 years ago

The changelog should not be considered to be complete, there are far too many changes and fixes for that. Please test the new release or a current nightly, thanks!

FeRcHuLeS commented 5 years ago

I've just tried v1.7.7 and is not present as I thought, and something called to my attention is the slow things are now with this version, next/previous state slot, saving, loading are as slow as a nice turtle.

orbea commented 5 years ago

Would you please make a new issue for the slow behavior you are experiencing? That way more people will be likely to see it. Please fill out the template and make sure to tell us more about your hardware and steps you take to reproduce this. You could try a fresh configuration file., maybe some setting is causing the behavior?

Since you confirmed the original issue is fixed I am going to close this issue, thanks for testing!

FeRcHuLeS commented 5 years ago

The original issue is not fixed at all, in fact is a lack that make us play twice the game if we want to savestate and get Hardcore achievements, there was another post here (deleted?) about the lack of it as well, "only featured emulators have and we dream Retroarch had" was the premise.

andres-asm commented 5 years ago

You can go into achievements, and pause hardcore, then you can save state. Is that not enough?

Sure what you want can be done, it's just annoying because most stuff taps into the same define and conditionals.

LordArchantos commented 5 years ago

Some people want it to be able to, during a hardcore playthrough, e.g. save state right before a tricky boss so they can later go on normal mode to practice it, without having to stop their current hardcore playthrough, as resuming from paused hardcore naturally resets the game.

https://retroachievements.org/viewtopic.php?t=7172

orbea commented 5 years ago

So in hardcore mode save states should be allowed and load states should not?

FeRcHuLeS commented 5 years ago

Exactly, that's the point of save states because pausing "hardcore mode" allow to savestates then you can't continue your game after that, because going back to hardcore mode resets the game, So simple in few words:

-Save states with retroachievements off -Load states with retroachievements off -Save states in normal mode -Load states in normal mode -Save states in hardcore mode -"Can't load states in hardcore mode"

andres-asm commented 5 years ago

Completely and utterly trivial to implement. Feel free to cherry-pick.

https://git.retromods.org/radius/RetroArch/commit/ef2abb3bb382a3042eba79978e8901d1b2a6bac1

orbea commented 5 years ago

@FeRcHuLeS Can you test the current master and see if the behavior is now correct?

FeRcHuLeS commented 5 years ago

I'm not familiar with that, I can't do such a thing and was expecting a nightly release to test it only, thanks guys.

orbea commented 5 years ago

Testing the latest nightly release should work just as well!

http://retroarch.com/?page=platforms

FeRcHuLeS commented 5 years ago

It's working as expected, the message iin hardcore mode ".....savestate and rewind are disabled....." after running a game was not updated, we are not going to waste time anymore playing twice.

Hardcore mode pauses...!!! if a previous game with no achievements savestates/loadstates?? Hardcore mode should keep instead of pausing for a game that has achievements, maybe still have not figured out a way to implement this, keep the great work RETROARCH IS AMAZING.

andres-asm commented 5 years ago

I don't really understand what you said there. Hardcore pause means you can save and load states, if you unpause hardcore game gets reset, that's all.

FeRcHuLeS commented 5 years ago

This is the case: X game has no achievements supported yet Y game has achievements support

With hardcore mode on X game you can save/load states then run Y game and harcore mode is paused , what I expect is to keep hardcore mode on instead of pausing this mode, Y game is another game that has nothing to do with X game, and if you didn't notice the message that popped up "A savestate was loaded, Achievements hardcore mode disabled for the current session. Restart to enable hardcore mode" then, while playing you'll get normal achievements when expecting Hardcore instead. and you could get the hardest ones and surprise... your achivements are not hardcore.

LordArchantos commented 5 years ago

In hopes of clarifying at least one thing here, I can confirm saving state works in hardcore mode now.

I think what FeRcHuLeS is saying above, is that when you play any game that does not have cheevos, and you load state, RetroArch will essentially pause hardcore mode automatically in the background without a notification. If after such use of load state in a cheevoless game you then switch to a game that does have cheevos, hardcore mode is still paused and RetroArch gives a notification of it.

andres-asm commented 5 years ago
$ git diff
diff --git a/cheevos-new/cheevos.c b/cheevos-new/cheevos.c
index 65186cd6c2..fc77092c07 100644
--- a/cheevos-new/cheevos.c
+++ b/cheevos-new/cheevos.c
@@ -953,6 +953,7 @@ bool rcheevos_unload(void)

       rcheevos_loaded            = false;
       rcheevos_hardcore_paused   = false;
+      rcheevos_state_loaded_flag = false;
    }

    return true;
diff --git a/cheevos/cheevos.c b/cheevos/cheevos.c
index 7c271793d7..8a05ca3499 100644
--- a/cheevos/cheevos.c
+++ b/cheevos/cheevos.c
@@ -2264,6 +2264,7 @@ bool cheevos_unload(void)

    cheevos_loaded     = false;
    cheevos_hardcore_paused = false;
+   cheevos_state_loaded_flag = false;

    return true;
 }
diff --git a/retroarch.c b/retroarch.c
index 1331327727..bd33c71b4d 100644
--- a/retroarch.c
+++ b/retroarch.c
@@ -4148,7 +4148,7 @@ static enum runloop_state runloop_check_state(
 #endif
    {
       char s[128];
-         bool rewinding = false;
+      bool rewinding = false;
       unsigned t     = 0;

       s[0]           = '\0';

that should fix it

andres-asm commented 5 years ago

hmmm no some of the logic that combines old and new is flawed

Try this diff instead. It's more verbose but it doesn't make weird assumptions that could flip a flag somewhere.

Works for me at least.

diff --git a/cheevos-new/cheevos.c b/cheevos-new/cheevos.c
index 65186cd6c2..fc77092c07 100644
--- a/cheevos-new/cheevos.c
+++ b/cheevos-new/cheevos.c
@@ -953,6 +953,7 @@ bool rcheevos_unload(void)

       rcheevos_loaded            = false;
       rcheevos_hardcore_paused   = false;
+      rcheevos_state_loaded_flag = false;
    }

    return true;
diff --git a/cheevos/cheevos.c b/cheevos/cheevos.c
index 7c271793d7..8a05ca3499 100644
--- a/cheevos/cheevos.c
+++ b/cheevos/cheevos.c
@@ -2264,6 +2264,7 @@ bool cheevos_unload(void)

    cheevos_loaded     = false;
    cheevos_hardcore_paused = false;
+   cheevos_state_loaded_flag = false;

    return true;
 }
diff --git a/retroarch.c b/retroarch.c
index 1331327727..d5e48a6ba1 100644
--- a/retroarch.c
+++ b/retroarch.c
@@ -4134,21 +4134,36 @@ static enum runloop_state runloop_check_state(

 #ifdef HAVE_CHEEVOS
    /* RCHEEVOS TODO: remove the 'rcheevos_*' below */
-   rcheevos_hardcore_active = cheevos_hardcore_active =  settings->bools.cheevos_enable
+   rcheevos_hardcore_active = settings->bools.cheevos_enable
       && settings->bools.cheevos_hardcore_mode_enable
-      && (rcheevos_loaded || cheevos_loaded) && !(rcheevos_hardcore_paused || cheevos_hardcore_paused);
+      && rcheevos_loaded && rcheevos_hardcore_paused;

-   if ((rcheevos_hardcore_active || cheevos_hardcore_active) && (rcheevos_state_loaded_flag || cheevos_state_loaded_flag))
+   cheevos_hardcore_active = settings->bools.cheevos_enable
+      && settings->bools.cheevos_hardcore_mode_enable
+      && cheevos_loaded && cheevos_hardcore_paused;
+
+   if (!settings->bools.cheevos_old_enable)
    {
-      rcheevos_hardcore_paused = cheevos_hardcore_paused = true;
-      runloop_msg_queue_push(msg_hash_to_str(MSG_CHEEVOS_HARDCORE_MODE_DISABLED), 0, 180, true, NULL, MESSAGE_QUEUE_ICON_DEFAULT, MESSAGE_QUEUE_CATEGORY_INFO);
+      if (rcheevos_hardcore_active && rcheevos_state_loaded_flag)
+      {
+         rcheevos_hardcore_paused = true;
+         runloop_msg_queue_push(msg_hash_to_str(MSG_CHEEVOS_HARDCORE_MODE_DISABLED), 0, 180, true, NULL, MESSAGE_QUEUE_ICON_DEFAULT, MESSAGE_QUEUE_CATEGORY_INFO);
+      }
+   }
+   else
+   {
+      if (cheevos_hardcore_active && cheevos_state_loaded_flag)
+      {
+         cheevos_hardcore_paused = true;
+         runloop_msg_queue_push(msg_hash_to_str(MSG_CHEEVOS_HARDCORE_MODE_DISABLED), 0, 180, true, NULL, MESSAGE_QUEUE_ICON_DEFAULT, MESSAGE_QUEUE_CATEGORY_INFO);
+      }
    }

    if (!(rcheevos_hardcore_active || cheevos_hardcore_active))
 #endif
    {
       char s[128];
-     bool rewinding = false;
+      bool rewinding = false;
       unsigned t     = 0;

       s[0]           = '\0';
inactive123 commented 5 years ago

Do I need to commit this diff or can people test this diff for themselves?

andres-asm commented 5 years ago

I figure they should test before pushing stuff to master https://git.retromods.org/radius/RetroArch/commit/3b7c1abe22fd4ef78c18360691a77b24557adb29

andres-asm commented 5 years ago

Builds here https://git.retromods.org/radius/RetroArch/tags/%238725

LordArchantos commented 5 years ago

The build above works for me too, games start in hardcore mode now when switching after load state.

inactive123 commented 5 years ago

Pushed it to master then.

LordArchantos commented 5 years ago

Not sure how I missed it, I was certain I tested it and this didn't happen, but on that test build and the current nightly (9f297f5f7c) RetroArch unfortunately also allows load state in hardcore mode. My apologies.

EDIT: Also if I now load a state in hardcore, and then go pause hardcore mode, RetroArch will start to infinitely spam the notification that a savestate has been loaded and hardcore mode is paused, and won't stop until closing the core.

LordArchantos commented 5 years ago

Currently in nightly builds the system is actually working the wrong way around; in hardcore mode you can both save and load state, but in normal mode you can only save state.

andres-asm commented 5 years ago

Ugh... ffs it was fine on the testing build right?

On Mon, May 20, 2019 at 10:23 AM LordArchantos notifications@github.com wrote:

Currently in nightly builds the system is actually working the wrong way around; in hardcore mode you can both save and load state, but in normal mode you can only save state.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/libretro/RetroArch/issues/8725?email_source=notifications&email_token=AANEFUFJW2ZVVND5KHRGQGDPWK67RA5CNFSM4HLSSD22YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVZFZVY#issuecomment-494034135, or mute the thread https://github.com/notifications/unsubscribe-auth/AANEFUF56OMQ5MWOJWZDOFDPWK67RANCNFSM4HLSSD2Q .

LordArchantos commented 5 years ago

That's what I thought, but testing it again now it has the above problem. I think it's probably because of messing around with loading states and hardcore pause, that I didn't realise that the load state availability between hardcore and normal had actually switched around. :/

andres-asm commented 5 years ago

Hmmm that's pretty weird as far as I can tell the code is correct.

https://git.retromods.org/radius/RetroArch/commit/35910e8785f55975667a9b2247c4da7092ad47dd#4bab8348ea20688bf5d4ccdc1fcc5c34044ddb1c_1779_1770

if (!(rcheevos_hardcore_active || cheevos_hardcore_active))
    show_load_state
andres-asm commented 5 years ago

actually thinking about it it should be AND not or Like

if (!(rcheevos_hardcore_active && cheevos_hardcore_active))

Try that I guess

andres-asm commented 5 years ago

Issue should be reopened I guess

LordArchantos commented 5 years ago

I just also found out with further testing, that if I disable hardcore mode from the achievements "main menu", and then start a game with cheevos, load state is available. It's only unavailable in normal mode when you use pause hardcore...

andres-asm commented 5 years ago

but you disabled hardcore mode from the main menu that's proper behavior

LordArchantos commented 5 years ago

Yeah, I know, just thought it might maybe help track down the problem since it only occurs with pausing hardcore. Also sadly I'm unable to make my own build, but if you can put one together with the above "or-and change" I can test it.

inactive123 commented 5 years ago

I'd appreciate it if @meleu or @leiradel could come here and try solving this issue for real.

andres-asm commented 5 years ago

@LordArchantos ok reproduced, the conditional behavior is correct though so the issue isn't there.

andres-asm commented 5 years ago

LOL what an oversight! I wonder how is that more stuff didn't break

Please try this build with the same redist I linked the other day retroarch.zip

diff --git a/retroarch.c b/retroarch.c
index 7085aceb36..0a3eeb3fb2 100644
--- a/retroarch.c
+++ b/retroarch.c
@@ -4167,11 +4167,11 @@ static enum runloop_state runloop_check_state(
    /* RCHEEVOS TODO: remove the 'rcheevos_*' below */
    rcheevos_hardcore_active = settings->bools.cheevos_enable
       && settings->bools.cheevos_hardcore_mode_enable
-      && rcheevos_loaded && rcheevos_hardcore_paused;
+      && rcheevos_loaded && !rcheevos_hardcore_paused;

    cheevos_hardcore_active = settings->bools.cheevos_enable
       && settings->bools.cheevos_hardcore_mode_enable
-      && cheevos_loaded && cheevos_hardcore_paused;
+      && cheevos_loaded && !cheevos_hardcore_paused;

    if (!settings->bools.cheevos_old_enable)
    {
andres-asm commented 5 years ago

better diff & build

diff --git a/cheevos/cheevos.c b/cheevos/cheevos.c
index 8a05ca3499..615b610255 100644
--- a/cheevos/cheevos.c
+++ b/cheevos/cheevos.c
@@ -2282,6 +2282,9 @@ bool cheevos_toggle_hardcore_mode(void)
       const char *msg = msg_hash_to_str(
             MSG_CHEEVOS_HARDCORE_MODE_ENABLE);

+      /* reset the state loaded flag in case it was set */
+      cheevos_state_loaded_flag = false;
+
       /* send reset core cmd to avoid any user
        * savestate previusly loaded. */
       command_event(CMD_EVENT_RESET, NULL);
diff --git a/command.c b/command.c
index 38456a7d79..918e57d358 100755
--- a/command.c
+++ b/command.c
@@ -1730,7 +1730,10 @@ static bool command_event_main_state(unsigned cmd)
             {
 #ifdef HAVE_CHEEVOS
                /* RCHEEVOS TODO: remove duplication below */
-               rcheevos_state_loaded_flag = cheevos_state_loaded_flag = true;
+               if (cheevos_hardcore_active)
+                  cheevos_state_loaded_flag = true;
+               if (rcheevos_hardcore_active)
+                  rcheevos_state_loaded_flag = true;
 #endif
                ret = true;
 #ifdef HAVE_NETWORKING
diff --git a/retroarch.c b/retroarch.c
index 7085aceb36..0a3eeb3fb2 100644
--- a/retroarch.c
+++ b/retroarch.c
@@ -4167,11 +4167,11 @@ static enum runloop_state runloop_check_state(
    /* RCHEEVOS TODO: remove the 'rcheevos_*' below */
    rcheevos_hardcore_active = settings->bools.cheevos_enable
       && settings->bools.cheevos_hardcore_mode_enable
-      && rcheevos_loaded && rcheevos_hardcore_paused;
+      && rcheevos_loaded && !rcheevos_hardcore_paused;

    cheevos_hardcore_active = settings->bools.cheevos_enable
       && settings->bools.cheevos_hardcore_mode_enable
-      && cheevos_loaded && cheevos_hardcore_paused;
+      && cheevos_loaded && !cheevos_hardcore_paused;

    if (!settings->bools.cheevos_old_enable)
    {

retroarch.zip

This logic with two implementations available is a bitch. Maybe both should share the same state variables.

andres-asm commented 5 years ago

Ok, simplified it all a bit.

$ git diff
diff --git a/cheevos/cheevos.c b/cheevos/cheevos.c
index 8a05ca3499..615b610255 100644
--- a/cheevos/cheevos.c
+++ b/cheevos/cheevos.c
@@ -2282,6 +2282,9 @@ bool cheevos_toggle_hardcore_mode(void)
       const char *msg = msg_hash_to_str(
             MSG_CHEEVOS_HARDCORE_MODE_ENABLE);

+      /* reset the state loaded flag in case it was set */
+      cheevos_state_loaded_flag = false;
+
       /* send reset core cmd to avoid any user
        * savestate previusly loaded. */
       command_event(CMD_EVENT_RESET, NULL);
diff --git a/command.c b/command.c
index 38456a7d79..918e57d358 100755
--- a/command.c
+++ b/command.c
@@ -1730,7 +1730,10 @@ static bool command_event_main_state(unsigned cmd)
             {
 #ifdef HAVE_CHEEVOS
                /* RCHEEVOS TODO: remove duplication below */
-               rcheevos_state_loaded_flag = cheevos_state_loaded_flag = true;
+               if (cheevos_hardcore_active)
+                  cheevos_state_loaded_flag = true;
+               if (rcheevos_hardcore_active)
+                  rcheevos_state_loaded_flag = true;
 #endif
                ret = true;
 #ifdef HAVE_NETWORKING
diff --git a/retroarch.c b/retroarch.c
index 7085aceb36..ef68bd84cd 100644
--- a/retroarch.c
+++ b/retroarch.c
@@ -4167,28 +4167,22 @@ static enum runloop_state runloop_check_state(
    /* RCHEEVOS TODO: remove the 'rcheevos_*' below */
    rcheevos_hardcore_active = settings->bools.cheevos_enable
       && settings->bools.cheevos_hardcore_mode_enable
-      && rcheevos_loaded && rcheevos_hardcore_paused;
+      && rcheevos_loaded && !rcheevos_hardcore_paused;

    cheevos_hardcore_active = settings->bools.cheevos_enable
       && settings->bools.cheevos_hardcore_mode_enable
-      && cheevos_loaded && cheevos_hardcore_paused;
+      && cheevos_loaded && !cheevos_hardcore_paused;

-   if (!settings->bools.cheevos_old_enable)
-   {
-      if (rcheevos_hardcore_active && rcheevos_state_loaded_flag)
-      {
-         rcheevos_hardcore_paused = true;
-         runloop_msg_queue_push(msg_hash_to_str(MSG_CHEEVOS_HARDCORE_MODE_DISABLED), 0, 180, true, NULL, MESSAGE_QUEUE_ICON_DEFAULT, MESSAGE_QUEUE_CATEGORY_INFO);
-      }
-   }
-   else
-   {
-      if (cheevos_hardcore_active && cheevos_state_loaded_flag)
-      {
-         cheevos_hardcore_paused = true;
-         runloop_msg_queue_push(msg_hash_to_str(MSG_CHEEVOS_HARDCORE_MODE_DISABLED), 0, 180, true, NULL, MESSAGE_QUEUE_ICON_DEFAULT, MESSAGE_QUEUE_CATEGORY_INFO);
-      }
-   }
+   rcheevos_hardcore_paused = !settings->bools.cheevos_old_enable
+      && rcheevos_hardcore_active
+      && rcheevos_state_loaded_flag;
+
+   cheevos_hardcore_paused = settings->bools.cheevos_old_enable
+      && cheevos_hardcore_active
+      && cheevos_state_loaded_flag;
+
+   if (rcheevos_hardcore_paused || cheevos_hardcore_paused)
+      runloop_msg_queue_push(msg_hash_to_str(MSG_CHEEVOS_HARDCORE_MODE_DISABLED), 0, 180, true, NULL, MESSAGE_QUEUE_ICON_DEFAULT, MESSAGE_QUEUE_CATEGORY_INFO);

    if (!(rcheevos_hardcore_active || cheevos_hardcore_active))
 #endif

That seems to work properly for both issues mentioned here

andres-asm commented 5 years ago

It's bugged somewhere else, it's no good Can't really simplify like that the conditionals have to be completely separate it seems.

Pushed yet another commit that seems to fix everything to my repo.

andres-asm commented 5 years ago

retroarch.zip

inactive123 commented 5 years ago

OK, cherry-picked this -

https://github.com/libretro/RetroArch/commit/eef4ada025775c17270f36be923e462456e67bb8

andres-asm commented 5 years ago

@FeRcHuLeS @LordArchantos these are my results Hardcore mode enabled:

image

Hardcore paused:

image

No achievements:

image

Tried loading state on that one, then I loaded a game with achievements again, no warning. Tried loading a game with no achievements again after that one, rewind works.

So the original issue and all other issues raised here seem fixed.

meleu commented 5 years ago

hi friends, I was really busy with RL in the last few days. I hope to solve this over this week (if radius didn't solve it already).

FeRcHuLeS commented 5 years ago

I've loaded states for a game with no cheevos, then run a game with cheevos and hardcore mode is not paused, working as expected to.

On the other hand Pause hardcore mode option is not working anymore and swithching to off then to on from the tools menu the message stating: "Achievements hardcore mode enabled, savestate and rewind were disabled" shows 2 times between 4 times Reset messages, and the message is lying now I suggest something like this ".....savestate enabled but loadstate and rewind were disabled"

FeRcHuLeS commented 5 years ago

I forgot to mention that I used latest nightly x86 32 bits date 2019_05_20 to test it.

andres-asm commented 5 years ago

Pause hardcore is working here. Probably you have a commit before it was fixed.

LordArchantos commented 5 years ago

Tested nightly b8d27362e4, everything seems to now function correctly for me too.

Jamiras commented 4 years ago

Everything mentioned here appears to be working correctly in 1.8.1: