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

[RFC] Enable rumble thread #189

Closed
wants to merge 3 commits into from
Closed

Conversation

V10lator
Copy link

This is currently untested as I can't compile but it should fix random crashes as well as #183

Signed-off-by: Thomas Rohloff <v10lator@myway.de>
Signed-off-by: Thomas Rohloff <v10lator@myway.de>
Signed-off-by: Thomas Rohloff <v10lator@myway.de>
@Sirius902 Sirius902 mentioned this pull request Apr 19, 2022
@Sirius902
Copy link
Contributor

This does not work in its current state, it doesn't seem like PadMgr_ThreadEntry is ever called so I suspect osCreateThread or something it calls isn't implemented properly.

@V10lator
Copy link
Author

Thanks for the feedback.

To the maintainers: Should I break this pull request into two with the first one having the fixes only? That would allow to add the fixes while we still figure out why the thread won't start.

@V10lator
Copy link
Author

Also to the maintainers: The decomp fixed the thread stacks on their end, too: zeldaret/oot@93096a4#diff-fe40752b17d8d7a41914b7ce858cfade82a6ce155bbb19c3aa05331103ea322eL79 - So maybe it would be a better idea to sync with upstream than to apply my fix?

@Sirius902
Copy link
Contributor

After looking into it more closely the reason this doesn't work is because osCreateThread on SoH is stubbed out in soh/stubs.c and therefore calling it does nothing.

@Kenix3
Copy link
Collaborator

Kenix3 commented Apr 24, 2022

Thanks for the feedback.

To the maintainers: Should I break this pull request into two with the first one having the fixes only? That would allow to add the fixes while we still figure out why the thread won't start.

As Sirius mentioned, the thread doesn't start because the libultraship doesn't expose a reimplementation for osCreateThread.

Up to you if you want to split or not. The correct solution here, though is for libultraship to expose osCreateThread.

@Sirius902 Sirius902 mentioned this pull request May 10, 2022
@Kenix3 Kenix3 closed this Jun 8, 2022
PurpleHato pushed a commit to PurpleHato/Shipwright that referenced this pull request Jun 25, 2022
…ilds-for-now

come on mac all i want you to be is a text file
Malkierian pushed a commit to Malkierian/Shipwright that referenced this pull request Nov 20, 2023
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.

3 participants