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

Minor code/repo cleanup; fix dictionary issues in XInputHolder #4

Merged
merged 7 commits into from
Jun 21, 2022
Merged

Minor code/repo cleanup; fix dictionary issues in XInputHolder #4

merged 7 commits into from
Jun 21, 2022

Conversation

TheNathannator
Copy link
Owner

@TheNathannator TheNathannator commented Jan 19, 2022

I've been working on a lot more than just this, but I decided that I need a fresh start from the point at v2.7.0.3. The history of changes that I made was quite messy (over 100 commits that definitely could have taken less) and it was becoming hard to work with.

The dictionary issues that have been reported are another reason for starting over, my other changes would have taken too long to finish and put into a new PR, which meant that issue wasn't getting fixed when it really needed to be.

This was quite a learning experience lol

Summary:

  • Remove ScpDriver-related files
  • Split ControllerStructs into multiple files inside a folder, and move WiiDrums/WiiGuitar into that folder
  • Remove the Wii prefix from WiiGuitar and WiiDrums in the entire codebase
  • Change most mentions of WiinUSoft (namespaces, program strings, etc.) to say WiitarThing instead
  • XInputHolder:
    • Make the ID 0-indexed for better array indexing
    • Protect against dictionary access exceptions
  • Clean up ViGEmBus check by moving it to the MainWindow constructor
    • I didn't know what a constructor was at the time lol
    • ...or much about program flow in general
  • Offer to open the ViGEmBus releases page upon failure to detect it

@TheNathannator
Copy link
Owner Author

TheNathannator commented Jun 21, 2022

Wondering if it would be better to take this pull request and bring it directly to the main repository along with the rest of the changes on your fork, I feel things have been away from there for too long now. I have plans that are probably a little too ambitious for just one pull request, so it'd be good to get this one merged in before that happens and close the issue for converting to ViGEm.

@Aida-Enna
Copy link
Collaborator

Aida-Enna commented Jun 21, 2022

I'm not sure how I missed this, that's 100% my fault. This is good stuff, nice work!

I'll go ahead and merge this in, but the reason we moved to my fork was that the original dev said they had no interest in working on it in Meowmaritus#9 and I wanted to fix some stuff. I don't think they would merge it in (I know, ironic comment on a 5 month old PR), and even if they did then we'd have some people linking there and some people linking here and it would just complicate things IMO.

That's not me saying no, of course, just putting in my 2 cents on it. 👍

@Aida-Enna Aida-Enna merged commit cbf28bf into TheNathannator:master Jun 21, 2022
@TheNathannator
Copy link
Owner Author

Yeah that's fair, just something I was thinking about just now.

Thanks for merging, and I really should have done a reminder sooner lol

@TheNathannator TheNathannator deleted the patch-3 branch June 21, 2022 08:14
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