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

Fixed Switch's incorrect button layout & Case Insensitivity #248

Closed
wants to merge 4 commits into from
Closed

Fixed Switch's incorrect button layout & Case Insensitivity #248

wants to merge 4 commits into from

Conversation

Sonic-Geared
Copy link

@Sonic-Geared Sonic-Geared commented Jan 12, 2024

This fixes a issue where the AB/Confirm & Back buttons on NX/Switch controllers would be swapped compared to the official Mania/RSDKv5, as reported in issue #108
And also fixes a problem where the files needed to be Case Insensitive (atleast on Legacy v3 mode, for what is told in the description of it), as reported in issue #214

@Sonic-Geared Sonic-Geared changed the title Switch's swapped AB Buttons Fix Fixed Switch's incorrect button layout Jan 16, 2024
apparently this was broken so i decided to fix it and also complete the todo of looking at the original impl in the other versions and instead of a return false it was a break, amazing ig
@Sonic-Geared Sonic-Geared changed the title Fixed Switch's incorrect button layout Fixed Switch's incorrect button layout & broken LoadFile Jan 16, 2024
@Sonic-Geared Sonic-Geared changed the title Fixed Switch's incorrect button layout & broken LoadFile Fixed Switch's incorrect button layout & Case Insensitivity Jan 16, 2024
Copy link
Member

@Mefiresu Mefiresu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code doesn't seem like it's been tested?

Comment on lines 244 to 249
// Fixes ".ani" ".Ani" bug and any other case differences
char pathLower[0x100];
memset(pathLower, 0, sizeof(pathLower));
for (int32 c = 0; c < strlen(filename); ++c) pathLower[c] = tolower(filename[c]);
memset(pathLower, 0, sizeof(char) * 0x100);
for (int32 c = 0; c < strlen(fullFilePath); ++c) {
pathLower[c] = tolower(fullFilePath[c]);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is equivalent to the old code, I highly doubt it fixes anything.

@@ -263,7 +266,7 @@ bool32 RSDK::LoadFile(FileInfo *info, const char *filename, uint8 fileMode)
}
if (modSettings.activeMod != -1) {
PrintLog(PRINT_NORMAL, "[MOD] Failed to find file %s in active mod %s", filename, modList[m].id.c_str());
// TODO return false? check original impl later
break; // no one ever checked the original implementation... :pensive:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This skips all following mods in the mod list.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i see, weirdly enough that is how the other decomps do the stuff related to that, but yeah i'll try to look into it further

hey, it's me goku!
and i'm a super saiyan!
@Sonic-Geared
Copy link
Author

Sonic-Geared commented Jan 19, 2024

SHIT, accidentally pressed a thing from the review stuff, sorry if it notificates you mephiles :(

@Mefiresu
Copy link
Member

Closing your PR for now because I feel like it doesn't fix anything:

  • Your "case insensitivity" code is still the same as before, you just removed the return and added a bunch of unnecessary comments.
  • Switch uses EGL, not SDL2/GLFW, so this doesn't fix the issue on the Switch build.
  • On other platforms, plugging in a Switch controller will set engine.confirmFlip to true and will flip A/B buttons on all non-Switch controllers that you plug in next (as it's not a per-controller option, but an engine-wide one).

@Mefiresu Mefiresu closed this Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants