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

Improve behavior of savestates made while game is assuming the state of the inserted memory card. #9027

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

AdmiralCurtiss
Copy link
Contributor

Fixes https://bugs.dolphin-emu.org/issues/12220

This looks pretty convoluted but I hope the comments make it clear what's happening and why.

@AdmiralCurtiss AdmiralCurtiss force-pushed the memcard-savestate-paranoia branch from f4dc5a6 to 5a775fb Compare August 12, 2020 03:24
Source/Core/Core/HW/EXI/EXI_Channel.cpp Outdated Show resolved Hide resolved
Source/Core/Core/HW/EXI/EXI_Channel.cpp Outdated Show resolved Hide resolved
@AdmiralCurtiss AdmiralCurtiss force-pushed the memcard-savestate-paranoia branch 2 times, most recently from 26f5d7d to c8f027e Compare August 12, 2020 18:46
@AdmiralCurtiss AdmiralCurtiss force-pushed the memcard-savestate-paranoia branch from c8f027e to 07a1542 Compare August 24, 2020 17:35
@JMC47
Copy link
Contributor

JMC47 commented Sep 12, 2020

Considering this prevents save corruption, we should merge this once it is reviewed and good to go.

@AdmiralCurtiss AdmiralCurtiss force-pushed the memcard-savestate-paranoia branch from 07a1542 to 57964e0 Compare December 13, 2020 04:23
@JMC47
Copy link
Contributor

JMC47 commented Jul 20, 2021

Is this still planning to be merged?

@AdmiralCurtiss
Copy link
Contributor Author

Considering I've yet to see anyone yell about corrupted savedata in the many years this issue has potentially existed, it's probably not particularly critical. If you want to merge it sure, if not eh, that's probably okay. I'll rebase it tomorrow just in case.

@AdmiralCurtiss AdmiralCurtiss force-pushed the memcard-savestate-paranoia branch from 57964e0 to 337b0d4 Compare July 20, 2021 19:52
@Rumi-Larry

This comment has been minimized.

@AdmiralCurtiss AdmiralCurtiss force-pushed the memcard-savestate-paranoia branch from 337b0d4 to 68a7885 Compare December 22, 2024 17:13
@AdmiralCurtiss
Copy link
Contributor Author

There was a request for me to rebase this on Discord, so I did.

I will note, however, that looking back on this I'm not 100% sure this whole DisableWrites() -> wait for a few frames approach is even necessary at all. It's probably okay but it would probably also work just fine if we, immediately after loading the state is done but before handing back to the actual emulation logic, just run the

      m_status.EXTINT = 1;
      m_system.GetExpansionInterface().UpdateInterrupts();

code in the relevant CEXIChannel so that the game is notified of the 'unplugged' memory card before it has the chance to run any writes.

I will also note that we should probably fix folder memory cards not storing their card ID / header before considering merging this, as this will make the situtation where you suddenly have a 'different' memory card after loading a savestate much more likely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants