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

[SDK decompression] IndexOf always returns -1 #5

Open
BrokenR3C0RD opened this issue Jan 8, 2021 · 3 comments
Open

[SDK decompression] IndexOf always returns -1 #5

BrokenR3C0RD opened this issue Jan 8, 2021 · 3 comments
Assignees
Labels
bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request
Projects

Comments

@BrokenR3C0RD
Copy link

Describe the bug
The ARM9#IndexOf function never modifies the index variable, meaning it always returns -1 and never finds the moduleparams, leaving compressed and unusuable data in its place.

To Reproduce
Steps to reproduce the behavior:

  1. Try loading an SDK-compressed NDS file.

Expected behavior
It should decompress the SDK-compressed files.

Screenshots
N./A

Desktop (please complete the following information):
N/A

Additional context
N/A

@pedro-javierf pedro-javierf self-assigned this Jan 9, 2021
@pedro-javierf pedro-javierf added documentation Improvements or additions to documentation enhancement New feature or request labels Jan 9, 2021
@pedro-javierf
Copy link
Owner

I believe this is actually a case of leftover functions in the ARM9.java, as IndexOf() is never used in practice. Thanks for opening this issue as it is a good opportunity to clean and improve the code.

Currently, the decision to decompress or not is based on user input. The user is asked whether the file being loaded is a commercial ROM or not (a homebrew, for example).

The current flow is the following:

NTRGhidraLoader.java: asks the user to choose decompression or not
if decompression is chosen
-->
NDS.java#GetDecompressedARM9() is called

This method checks the static footer (so here really, is where the tool could automatically detect whether a given file has compression or not, without having to rely on user input.).

If the NitroFooter is not null then ARM9.Decompress(MainRom, StaticFooter._start_ModuleParamsOffset); is called.

The ModuleParamsOffset is read from the NitroFooter and passed to the Decompress() method with the following signature: Decompress(byte[] Data, int _start_ModuleParamsOffset) which does not need to use IndexOf() to correctly decompress the data.

public class NitroFooter
	{
		public int NitroCode;
		public int _start_ModuleParamsOffset;
		public int Unknown;
		
		public NitroFooter() { }
		public NitroFooter(BinaryReader er) throws IOException
		{
			NitroCode = er.readNextInt();
			_start_ModuleParamsOffset = er.readNextInt();
			Unknown = er.readNextInt();
		}
		
	}

TLDR; As long as the user chooses the correct option when asked, the loader works correctly. Despite this, there is a lot of room for improvement as the loader could detect if a ROM has compression or not, and act accordingly, without user interaction.

We should start fixing the indexOf() method

@BrokenR3C0RD
Copy link
Author

I do have to note that I did have to fix this bug in a local copy of the code in order to decompress a certain application, and without it it ended up ignoring the entire process. I'm sadly away from my computer at the moment but fixing IndexOf is rather trivial, and actually saves a lot of complexity.

Right now, the function stores an index (is supposed to) when it finds a match, and then breaks and returns that index. It would be much more straightforward to simply return i when the match is found and return -1 at the end of the file.

@pedro-javierf
Copy link
Owner

Interesting, which file were you loading? So far I'm aware NTRGhidra has been tested with over a dozen of games and a bunch of homebrews as well, and the decompressed data for the games tested again a manually-decompressed sample to compare, with good results.

And yes, the indexOf should be trivial to rewrite and would save a lot of lines of code, so you're welcome to contribute whenever possible! Having it working could make possible to remove other parts of code and bring several other optimizations.

@pedro-javierf pedro-javierf added this to To do in Features Jan 12, 2021
@pedro-javierf pedro-javierf added the bug Something isn't working label Nov 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request
Projects
Features
  
To do
Development

No branches or pull requests

2 participants