diff --git a/Source/Core/Core/HW/EXI/EXI.cpp b/Source/Core/Core/HW/EXI/EXI.cpp index 0c11ad8e056d..a45481e03ab8 100644 --- a/Source/Core/Core/HW/EXI/EXI.cpp +++ b/Source/Core/Core/HW/EXI/EXI.cpp @@ -152,15 +152,15 @@ static void ChangeDeviceCallback(u64 userdata, s64 cyclesLate) } void ChangeDevice(const u8 channel, const TEXIDevices device_type, const u8 device_num, - CoreTiming::FromThread from_thread) + CoreTiming::FromThread from_thread, u32 delay) { // Let the hardware see no device for 1 second - CoreTiming::ScheduleEvent(0, changeDevice, + CoreTiming::ScheduleEvent(delay, changeDevice, ((u64)channel << 32) | ((u64)EXIDEVICE_NONE << 16) | device_num, from_thread); - CoreTiming::ScheduleEvent(SystemTimers::GetTicksPerSecond(), changeDevice, - ((u64)channel << 32) | ((u64)device_type << 16) | device_num, - from_thread); + CoreTiming::ScheduleEvent( + SystemTimers::GetTicksPerSecond() + static_cast(delay), changeDevice, + ((u64)channel << 32) | ((u64)device_type << 16) | device_num, from_thread); } CEXIChannel* GetChannel(u32 index) diff --git a/Source/Core/Core/HW/EXI/EXI.h b/Source/Core/Core/HW/EXI/EXI.h index 27e806d47261..823474fc6f63 100644 --- a/Source/Core/Core/HW/EXI/EXI.h +++ b/Source/Core/Core/HW/EXI/EXI.h @@ -41,7 +41,8 @@ void UpdateInterrupts(); void ScheduleUpdateInterrupts(CoreTiming::FromThread from, int cycles_late); void ChangeDevice(const u8 channel, const TEXIDevices device_type, const u8 device_num, - CoreTiming::FromThread from_thread = CoreTiming::FromThread::NON_CPU); + CoreTiming::FromThread from_thread = CoreTiming::FromThread::NON_CPU, + u32 delay = 0); CEXIChannel* GetChannel(u32 index); diff --git a/Source/Core/Core/HW/EXI/EXI_Channel.cpp b/Source/Core/Core/HW/EXI/EXI_Channel.cpp index 266fa9b73f53..b80babe48f35 100644 --- a/Source/Core/Core/HW/EXI/EXI_Channel.cpp +++ b/Source/Core/Core/HW/EXI/EXI_Channel.cpp @@ -13,7 +13,9 @@ #include "Core/CoreTiming.h" #include "Core/HW/EXI/EXI.h" #include "Core/HW/EXI/EXI_Device.h" +#include "Core/HW/EXI/EXI_DeviceMemoryCard.h" #include "Core/HW/MMIO.h" +#include "Core/HW/SystemTimers.h" #include "Core/Movie.h" namespace ExpansionInterface @@ -238,48 +240,86 @@ void CEXIChannel::DoState(PointerWrap& p) p.Do(m_dma_length); p.Do(m_control); p.Do(m_imm_data); - - Memcard::HeaderData old_header_data = m_memcard_header_data; p.DoPOD(m_memcard_header_data); for (int device_index = 0; device_index < NUM_DEVICES; ++device_index) { - std::unique_ptr& device = m_devices[device_index]; - TEXIDevices type = device->m_device_type; + TEXIDevices type = m_devices[device_index]->m_device_type; p.Do(type); - if (type == device->m_device_type) - { - device->DoState(p); - } - else - { - std::unique_ptr save_device = - EXIDevice_Create(type, m_channel_id, m_memcard_header_data); - save_device->DoState(p); - AddDevice(std::move(save_device), device_index, false); - } + if (type != m_devices[device_index]->m_device_type) + AddDevice(EXIDevice_Create(type, m_channel_id, m_memcard_header_data), device_index, false); + + m_devices[device_index]->DoState(p); - if (type == EXIDEVICE_MEMORYCARDFOLDER && old_header_data != m_memcard_header_data && - !Movie::IsMovieActive()) + bool is_memcard = type == EXIDEVICE_MEMORYCARD || type == EXIDEVICE_MEMORYCARDFOLDER; + if (is_memcard && !Movie::IsMovieActive()) { - // We have loaded a savestate that has a GCI folder memcard that is different to the virtual - // card that is currently active. In order to prevent the game from recognizing this card as a - // 'different' memory card and preventing saving on it, we need to reinitialize the GCI folder - // card here with the loaded header data. - // We're intentionally calling ExpansionInterface::ChangeDevice() here instead of changing it - // directly so we don't switch immediately but after a delay, as if changed in the GUI. This - // should prevent games from assuming any stale data about the memory card, such as location - // of the individual save blocks, which may be different on the reinitialized card. - // Additionally, we immediately force the memory card to None so that any 'in-flight' writes - // (if someone managed to savestate while saving...) don't happen to hit the card. - // TODO: It might actually be enough to just switch to the card with the - // notify_presence_changed flag set to true? Not sure how software behaves if the previous and - // the new device type are identical in this case. I assume there is a reason we have this - // grace period when switching in the GUI. - AddDevice(EXIDEVICE_NONE, device_index); - ExpansionInterface::ChangeDevice(m_channel_id, EXIDEVICE_MEMORYCARDFOLDER, device_index, - CoreTiming::FromThread::CPU); + // We are processing a savestate that has a memory card plugged into the system, but don't + // want to actually store the memory card in the savestate, so loading older savestates + // doesn't confusingly revert in-game saves done since then. + + // Handling this properly is somewhat complex. When loading a state, the game still needs to + // see the memory card device as it was (or at least close enough) when the state was made, + // but at the same time we need to tell the game that the data on the card may have been + // changed and it should not assume that the data (especially the file system blocks) it may + // or may not have read before is still valid. + + // To accomplish this we do the following: + + CEXIMemoryCard* card_device = static_cast(m_devices[device_index].get()); + + if (p.GetMode() != PointerWrap::MODE_READ) + { + // In any of the non-modifying modes, we just read the file system blocks from the currently + // active memory card and push them into p, so we have a reference of how the card file + // system looked at the time of savestate creation. + std::array card_file_system_data; + bool read_success = + card_device->ReadFromMemcard(0, 0xa000, card_file_system_data.data()) == 0xa000; + p.Do(read_success); + if (read_success) + p.DoPOD(card_file_system_data); + } + else + { + // When loading a state, we compare this previously written file system block data with the + // data currently in the card. If it mismatches in any way, we make sure to notify the game. + bool should_notify_game = false; + bool has_card_file_system_data; + p.Do(has_card_file_system_data); + if (has_card_file_system_data) + { + std::array state_file_system_data; + p.DoPOD(state_file_system_data); + std::array card_file_system_data; + bool read_success = + card_device->ReadFromMemcard(0, 0xa000, card_file_system_data.data()) == 0xa000; + should_notify_game = !(read_success && state_file_system_data == card_file_system_data); + } + else + { + should_notify_game = true; + } + + if (should_notify_game) + { + // We want to tell the game that the card is different, but don't want to immediately + // destroy the memory card device, as there may be pending operations running on the CPU + // side (at the very least, I've seen memory accesses to 0 when switching the device + // immediately in F-Zero GX). In order to ensure data on the card isn't corrupted by + // these, we disable writes to the memory card device. + card_device->DisableWrites(); + + // And then we force a re-load of the memory card by switching to a null device a frame + // from now, then back to the memory card a few frames later. This also makes sure that + // the GCI folder header matches what the game expects, so the game never recognizes it as + // a 'different' memory card and prevents saving on it. + ExpansionInterface::ChangeDevice(m_channel_id, type, device_index, + CoreTiming::FromThread::CPU, + SystemTimers::GetTicksPerSecond() / 60); + } + } } } } diff --git a/Source/Core/Core/State.cpp b/Source/Core/Core/State.cpp index 789d08f275f1..9385cb4bc6c7 100644 --- a/Source/Core/Core/State.cpp +++ b/Source/Core/Core/State.cpp @@ -74,7 +74,7 @@ static Common::Event g_compressAndDumpStateSyncEvent; static std::thread g_save_thread; // Don't forget to increase this after doing changes on the savestate system -constexpr u32 STATE_VERSION = 122; // Last changed in PR 8571 +constexpr u32 STATE_VERSION = 123; // Last changed in PR 9027 // Maps savestate versions to Dolphin versions. // Versions after 42 don't need to be added to this list,