GCMemcard: Change behavior of TitlePresent() to more closely resemble how saves are actually identified. #8903
+92
−14
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AdmiralCurtiss
force-pushed
the
gcmemcard-file-identity-check
branch
from
07041d6
to
8b31361
Compare
|
Commit should probably have the extended description added to it, so that info is stored in the repository |
AdmiralCurtiss
force-pushed
the
gcmemcard-file-identity-check
branch
from
8b31361
to
c486270
Compare
|
Good point, done. |
BhaaLseN
reviewed
Jun 28, 2020
| if (lhs.m_makercode != rhs.m_makercode) | ||
| return false; | ||
|
|
||
| for (size_t i = 0; i < lhs.m_filename.size(); ++i) |
There was a problem hiding this comment.
Should this perhaps be i < std::min(lhs.m_filename.size(), rhs.m_filename.size())? Or do we make sure that the lhs filename is shorter than (or of equal length to) the rhs filename?
There was a problem hiding this comment.
Scratch that, I just noticed this is an array and not an actual string.
BhaaLseN
approved these changes
Jun 28, 2020
| if (lhs.m_makercode != rhs.m_makercode) | ||
| return false; | ||
|
|
||
| for (size_t i = 0; i < lhs.m_filename.size(); ++i) |
There was a problem hiding this comment.
Scratch that, I just noticed this is an array and not an actual string.
… how saves are actually identified. This modifies GCMemcard::TitlePresent() to match my findings of how the GC BIOS and various games behave when you alter the fields in the directory entry. It looks like for a save to be recognized by a game, the following have to be true: - Game code and maker code must exactly match what the game expects. - Filename is only checked up to the first null byte. All bytes afterwards can be whatever. The BIOS itself does a full compare of the filename when checking for whether it should allow copying a file from one card to another, but behaves oddly in some cases when there's non-null bytes after the first null. See the big comment in `HasSameIdentity()` for details.
AdmiralCurtiss
force-pushed
the
gcmemcard-file-identity-check
branch
from
c486270
to
556e93f
Compare
|
Clarified the description a bit, code was not changed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
This modifies
GCMemcard::TitlePresent()to match my findings of how the GC BIOS and various games behave when you muck with the fields in the directory entry.It looks like for a save to be recognized by a game, the following have to be true:
The BIOS itself does a full compare of the filename when checking for whether it should allow copying a file from one card to another, but behaves oddly in some cases when there's non-null bytes after the first null. See the big comment in
HasSameIdentity()for details.The new
GCMemcardUtils.cppfile may seem a bit excessive for now, but I have some plans for some improvements to the memory card manager that would fit well in there, andGCMemcard.cppis pretty crowded already.I'm also not sure if "Identity" is a good word to describe what we're comparing here; any other suggestions?