-
-
Notifications
You must be signed in to change notification settings - Fork 109
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
SIO0 overhaul #1673
base: main
Are you sure you want to change the base?
SIO0 overhaul #1673
Conversation
This one is ready for review. Moved a lot of things around to split the card and pad code away from the sio class. |
You're making Linux sad :) |
Looks like there's still an asan error, with a mismatched new/delete type. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1673 +/- ##
=======================================
Coverage ? 9.87%
=======================================
Files ? 448
Lines ? 133030
Branches ? 0
=======================================
Hits ? 13143
Misses ? 119887
Partials ? 0 ☔ View full report in Codecov by Sentry. |
std::memcpy(data.get(), g_emulator->m_sio->getMcdData(1), SIO::c_cardSize); | ||
std::memcpy(data.get() + SIO::c_cardSize, g_emulator->m_sio->getMcdData(2), SIO::c_cardSize); | ||
std::unique_ptr<uint8_t[]> data = std::make_unique<uint8_t[]>(MemoryCards::c_cardSize * 2); | ||
std::memcpy(data.get(), g_emulator->m_memoryCards->getMcdData(0), MemoryCards::c_cardSize); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're here doing some refactoring, we might as well use proper enums for the memory cards indices, maybe?
// skip id 8 | ||
// skip id 9 | ||
// skip id 10 | ||
// skip id 11 | ||
typedef Protobuf::FieldRef<Protobuf::UInt32, TYPESTRING("pad_state"), 12> SIOPadState; | ||
// skip id 12 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, uuuuh, what happens if a savestate is done while an SIO read is in progress? This doesn't seem we're reloading into a proper state, able to continue gracefully. Or we never did, maybe, for memory cards at least?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll revisit the save state code. Admittedly, I had plans to eventually redo the SIO side of pads, so this escaped my focus. Thank you for catching this. I know pad issues have been noted regarding savestates back in #1173 maybe I can knock this one out in the process.
DraftPR, trying to shore up various changes I had worked up in preparation of adding multitap support. My initial branch became sorely outdated and conflicted, this branch attempts to cherry pick some of these changes and bring the branch back in line with head. Needs some additional cleanup; get rid of vectors, mentions of multitap, test various things, etc.