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

Generate the correct Stadium base data for every ROM #1010

Merged
merged 10 commits into from
Sep 30, 2022

Conversation

Rangi42
Copy link
Member

@Rangi42 Rangi42 commented Sep 27, 2022

Fixes #752.

The -e flag was tested and works for all the European ROMs mentioned in #626.

@Rangi42 Rangi42 requested a review from mid-kid September 27, 2022 22:05
@Rangi42
Copy link
Member Author

Rangi42 commented Sep 27, 2022

The base data stores a bit for each bank of the built ROM, indicating whether it matches the bank in the base file (crystal_base0.bin for US, crystal_base1.bin for EU). We don't want to store those whole files, so stadium.c just stores CRCs for each bank of the base .bin files, and compares them with CRCs of the built ROM banks. This runs the risk of CRC collisions, but works in practice for all the official ROMs.

If we want to avoid storing any information derived from those two .bin files, we could do as @mid-kid suggested: store CRCs for the banks that have 1 bits (since those can be derived from the matching ROMs), and store flags for the rest indicating that they don't match. (So the base0_crcs and base1_crcs arrays could store int32_ts instead of uint16_ts, with -1 indicating a non-match.) Then if you build a custom ROM, it might have even fewer banks than the original ROM that match the base .bin data, but it won't have more. (Technically it could but we wouldn't be able to know that from the data available.)

I'd be okay with that change; it's a small update to the code:

 	// Calculate the base data bits using bank CRCs
 	// Bits indicate if the bank CRC matches the base one
 	for (int i = 0; i < BASEDATASIZE - BASEHEADERSIZE; i++) {
 		uint8_t bits = 0;
 		for (int j = 0; j < 8; j++) {
 			int bank = i * 8 + j;
+			if (base_crcs[bank] != -1) {
 				uint16_t crc = calculate_crc(CRC_INIT, file, bank * BANKSIZE, BANKSIZE);
 				bits |= (crc == base_crcs[bank]) << j;
+			}
 		}
 		file[BASEDATAOFF + BASEHEADERSIZE + i] = bits;
 	}

@vulcandth
Copy link
Collaborator

vulcandth commented Sep 29, 2022

If we want to avoid storing any information derived from those two .bin files, we could do as @mid-kid suggested: store CRCs for the banks that have 1 bits (since those can be derived from the matching ROMs), and store flags for the rest indicating that they don't match. (So the base0_crcs and base1_crcs arrays could store int32_ts instead of uint16_ts, with -1 indicating a non-match.) Then if you build a custom ROM, it might have even fewer banks than the original ROM that match the base .bin data, but it won't have more. (Technically it could but we wouldn't be able to know that from the data available.)

Let me see if I understand this correctly.
I'm just going to paraphrase what you just said in my own thoughts for my own understanding; So bear with me:

What you are saying.. is instead of storing every CRC from the crystal_base0.bin and crystal_base1.bin to match against... We instead only store the CRCs of every bank that matches across all of the base data for US and EU respectively.

So what we can essentially do is perform a logical or operation on all of the base data bytes that we already know of that is related to crystal_base0.bin US and the same for all of the base data bytes related to crystal_base1.bin EU. After which we calculate and only store the CRCs for the banks of the resulting OR operation with a bit of 1.. otherwise we store -1 in our base0_crcs and base1_crcs arrays.

Finally tools/stadium.c just needs to calculate the CRC of each bank and compare it to the few CRCs we actually stored.

If I am understanding correctly... Then I agree, I think that is also the better option then storing every CRC from crystal_base0.bin and crystal_base1.bin.

@mid-kid
Copy link
Member

mid-kid commented Sep 29, 2022

@vulcandth The idea is to store a "dummy" CRC value for the banks we can't recover. If the value is a dummy, it's automatically a 1. No need for full ORing.

@Rangi42
Copy link
Member Author

Rangi42 commented Sep 29, 2022

We do already source data from the leaks, since we build CRYSTAL_ps3_010328d.bin and CRYSTAL_ps3_us_revise_010710d.bin. And the crystal_base0/1.bin files would need mentioning in a comment to explain what the 0 and 1 bit flags even mean. So I'd rather just leave all the base bank checksums as-is.

Other changes to make (do them to pokegold as well when applicable):

  • Group the constants by what chunk of data they refer to, instead of whether they're an offset or a size
  • DATAOFF -> STADIUMOFF (or anything more specific than "data")
  • Signature of calculate_checksum and calculate_crc should match memcpy and memset (offset pointer, not base pointer and start index)

@Rangi42
Copy link
Member Author

Rangi42 commented Sep 30, 2022

@mid-kid I've made the changes you requested. Will do the same to pokegold after this merges.

@Rangi42 Rangi42 requested a review from vulcandth September 30, 2022 15:39
@mid-kid
Copy link
Member

mid-kid commented Sep 30, 2022

LGTM now, thanks for making the changes.

Makefile Show resolved Hide resolved
Copy link
Collaborator

@vulcandth vulcandth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this looks fine to me too. Good work.

@Rangi42 Rangi42 merged commit 38578cc into pret:master Sep 30, 2022
@Rangi42 Rangi42 deleted the base-data branch September 30, 2022 20:46
Idain pushed a commit to Idain/pokecrystal that referenced this pull request Sep 11, 2023
Idain pushed a commit to Idain/pokecrystal that referenced this pull request Sep 12, 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.

Reverse-engineer the Crystal "base" data inserted by tools/stadium.c
3 participants