Add memory size config back#279
Open
xchellx wants to merge 1 commit into
Open
Conversation
Brings back the manual memory configuration with an automatic detection setting thats checked by default (for backwards compatibility with the automatic detection changes).
Collaborator
|
@cristian64 I know you have back/forth regarding the ARAM change, but I don't have an issue with merging this for next release. |
Collaborator
I like to resolve the other discussion first, in case it helps with this other discussion. I'm not against this change, but I'm still not sure under which circumstances auto-detection can fail. Can we define the steps that are needed to encounter that case? Right now auto-detection merely checks the values that Dolphin inserted in memory. This should not depend on |
This file contains hidden or 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
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.Suggestion cannot be applied right now. Please check back later.
Brings back the manual memory configuration with an automatic detection setting thats checked by default (for backwards compatibility with the automatic detection changes).
The reasoning: bi2.bin which contains osArenaLo and osArenaHi can be manipulated to report values that can differ from the BAT#U and BAT#L registers and vice versa. One would set Dolphin's memory override to match this. For example, the bi2.bin can set osArenaHi as 66060288 (63 MiB) where BAT0L/BAT0U and BAT2L/BAT2U can be 32 MiB + 32 MiB (64 MiB) and Dolphin memory override is set to 64 MiB. This leads to DME ending its memory view at 83F00000 (66060288, 63 MiB) instead of 0x84000000 (67108864, 64 MiB). A manual override as an optional configurable fallback will act as the Dolphin memory override does, fixing this issue for this edge case.
Further more,epending on Dolphin's INI structures is not reliable as Dolphin only writes to its INIs when it closes. If DME were to use INI settings to get the memory override set in Dolphin and the memory override is changed between game launches, DME cannot know of it until Dolphin closes and reopens.
In a case where the source of information for memory sizes is unreliable, a fallback (optional) slider is a good enough workaround. I admit, the previous slider is not as safe (it can, like bi2.bin and BAT, be manipulated to values that don't match), and I simply just brought it back with a toggable setting to toggle it (default to on/true so previously functionality is not interrupted) but there does not seem to be a way to safe guard these sliders. It simply is a fix to this edge case which I do not know of any other solution.
Finally, yes, the memory override technically is a fix for the ARAM detection problem on GameCube but that is not the intent. I have a separate fix for that. Yes, indeed, if memory reports as 83F00000 to DME but its actually 84000000 behind the scenes, then my ARAM fix (#280) will fail and this PR with a memory override of 64 MiB fixes this bug but again that is not a bug this PR is intended to fix.
I understand there may be a desire to keep the settings UI clean/simple and for DME functionality to continue to be reliable. I am open to any other ways this can be done or for this to be dropped and another path forward to fix this problem.