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

Correctly format a ControllerPak #481

Merged
merged 4 commits into from
Jan 22, 2024

Conversation

networkfusion
Copy link
Contributor

@networkfusion networkfusion commented Jan 11, 2024

Still requires formal testing from someone.

For note, the Original PR was: rasky#7 which includes lots of extra detail.

Linked to (but does not fix) #105

@networkfusion networkfusion force-pushed the format-cpak-properly branch 3 times, most recently from ab483e6 to 7c44ea5 Compare January 11, 2024 02:57
@networkfusion networkfusion marked this pull request as ready for review January 18, 2024 01:30
@networkfusion networkfusion force-pushed the format-cpak-properly branch 2 times, most recently from 00d555b to 2bc1e24 Compare January 19, 2024 12:52
@networkfusion
Copy link
Contributor Author

@meeq and @bryc are you able to check that this works / looks correct?

@bryc
Copy link

bryc commented Jan 20, 2024

The code itself seems fine to me. It correctly implements a full Controller Pak initialization.

But does this have a place in libdragon?

If libdragon were to get a robust C-Pak and repair function, this would largely become obsolete. An argument could also be made that it would be better for a general N64 library to only modify what is absolutely necessary instead of completely overwriting the entire system area.

IMHO this kind of function is more suited for initializing empty C-Pak data in emulators or flashcart C-Pak managers, not homebrew games and such.

@networkfusion
Copy link
Contributor Author

networkfusion commented Jan 20, 2024

The code itself seems fine to me. It correctly implements a full Controller Pak initialization.

But does this have a place in libdragon?

If libdragon were to get a robust C-Pak and repair function, this would largely become obsolete. An argument could also be made that it would be better for a general N64 library to only modify what is absolutely necessary instead of completely overwriting the entire system area.

IMHO this kind of function is more suited for initializing empty C-Pak data in emulators or flashcart C-Pak managers, not homebrew games and such.

My take is to fix what is broken, and then deprecate it (since we should be looking at a new API anyway).

And thanks for the response.

Going further, I would suggest that this becomes an internal function that is only called upon in a replacement API when the repair function has failed.

But since the PR is valid with the suggested changes, It seems fine to merge with unstable.

@rasky
Copy link
Collaborator

rasky commented Jan 21, 2024

Well flashcart cpak manager could be written with libdragon too. I agree that homebrew games shouldn't call this function and that we need to rewrite the whole module. At the same time if this PR works better than what its in now, I think merging it is a good idea anyway

@bryc
Copy link

bryc commented Jan 21, 2024

This is certainly better than what currently sits at format_mempak. Which merely writes an array of bytes taken from old era Mupen64, under the assumption that the emulator is doing it right (it was not).

Anyway, I'd be happy to assist if and when someone wants to work on a proper complete C-Pak lib 😃. Much of the logic in MPKEdit could be adapted (though it's in JS, it should be possible to port). The only thing MPKEdit lacks currently is a 'smart' IndexTable repair routine, which wasn't a priority because the app doesn't interact with live hardware.

Has a compiler flag in case original header is really important to them!
@networkfusion networkfusion force-pushed the format-cpak-properly branch 3 times, most recently from 5ad0b4a to 95b508e Compare January 22, 2024 14:58
@rasky rasky merged commit 5ea3490 into DragonMinded:unstable Jan 22, 2024
4 checks passed
@networkfusion networkfusion deleted the format-cpak-properly branch June 3, 2024 15:20
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