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

Why not submit these changes as a pull request to the upstream Luma? #1

Closed
cheatfreak47 opened this issue Jul 10, 2019 · 14 comments
Closed

Comments

@cheatfreak47
Copy link

Any particular reason why you haven't submitted a pull to the upstream luma team? I'm sure someone would look over the changes and this stuff could be added to the main version of the CFW instead of needing to use a fork.

(btw, love the majora's mask restoration, keep up the great work!)

@leoetlino
Copy link
Owner

Yeah, I would really prefer this being upstream but upstream has rejected BPS or xdelta patching, and the code is written in C++, not C, so I think it wouldn't be accepted. Maybe I should still ask though...

@cheatfreak47
Copy link
Author

cheatfreak47 commented Jul 11, 2019

I'm guessing that was more against xdelta, but the code format discrepancy is more likely to be what gets your pull rejected or shelved than anything else.

Still, might be worth a shot, maybe if you explain to them why BPS is necessary over IPS they'll hear you out given that you're developing an extensive code mod for a game and that IPS simply isn't working for you.

@leoetlino
Copy link
Owner

leoetlino commented Jul 11, 2019

BPS is a lot simpler than xdelta, but still ~200 lines of code (+ another 100 of utilities), compared to IPS's ~40 lines. Anyway, yeah, I'll reformat the code to conform to the existing code style if I do submit a PR. I really don't want to rewrite it in C, though.

@leoetlino leoetlino pinned this issue Jul 11, 2019
@leoetlino
Copy link
Owner

leoetlino commented Jul 12, 2019

TuxSH told me that loader is extremely memory constrained on the old 3DS, and old 3DS compatibility is a requirement for Luma. C++ wouldn't really be an issue if there are no memory issues.

After some work, I managed to halve the size difference (now +1128 bytes; was +2440 bytes in the public branch).

If you have an old 3DS, would you be willing to help me test this branch?

@leoetlino
Copy link
Owner

@cheatfreak47
Copy link
Author

Sure, I'll test it right now actually.

@cheatfreak47
Copy link
Author

The build you sent here doesn't seem to be able to load BPS patches at all, or if it does, the BPS patches you've provided for Majora's Mask are causing loader to crash. I get the same crash on both n3DS and o3DS using the same files for both.

@cheatfreak47
Copy link
Author

btw feel free to add me on either discord or telegram if you want me to assist in faster debugging and testing.
CheatFreak#3799 👌 / CheatFreak

@cheatfreak47
Copy link
Author

cheatfreak47 commented Jul 13, 2019

Based on my own testing, it seems that indeed it is failing to apply the BPS patch for some reason on both o3DS and n3DS. This is likely the reason that people are having success with the 1.00 patch, since that patch is still using IPS format. (doesn't explain why it's applying it to the base game binary though 🤔 )

I don't think the patch is the problem, as I tried manually patching the code.bin with the bps patch and it worked fine. (I did this by dumping the update's code.bin and applying the BPS manually, and then loading that instead)

@cheatfreak47
Copy link
Author

cheatfreak47 commented Jul 13, 2019

I think maybe this may be a bug with Luma right now actually, it seems that when you try to apply a code patch it isn't loading the game update.

When I manually patch the 1.1 exefs and put it in Luma's title loader, the 1.1 text doesn't show on the title screen, but because the mods are there and functional, it seems that the update's romfs that changes the title screen isn't being accessed.

@leoetlino
Copy link
Owner

After applying the patch, you get a patched 1.0 executable so it isn't supposed to show 1.1, I think.

Do you have the crash reports? I think you're correct about Luma not loading update exefs; several people have said that they were only able to get it to work with the v100 patch (which targets the MM3D 1.0 executable) even though they have the 1.1 update installed, which suggests that Luma is loading the base exefs and applying the patch to MM3D 1.0 and not to the update. On the other hand, someone who only has 1.1 (because their game card came with it) did get the BPS patch that targets MM3D 1.1 / v110 to work.

@leoetlino
Copy link
Owner

Loader (and BPS patching) works on o3DS. Looks like the issue is indeed that loader is ignoring the update exefs for some reason...

@leoetlino
Copy link
Owner

Since the loader bug fix has been submitted and merged upstream, and another PR for BPS patching has also been submitted, I think this can be closed.

@TuxSH
Copy link

TuxSH commented Apr 16, 2020

Merged, sorry for taking too long of a break.

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

No branches or pull requests

3 participants