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

Invalid default value in SMS header #333

Closed
HerrSchatten opened this issue Sep 8, 2020 · 8 comments
Closed

Invalid default value in SMS header #333

HerrSchatten opened this issue Sep 8, 2020 · 8 comments

Comments

@HerrSchatten
Copy link

If not explicitly specified in SMSHEADER, the upper nibble of the byte at $7FFF defaults to 0, which is an invalid value for region information.
In older versions of WLA-DX the default value used to be 4 (= export SMS), which it should be, as it's the only value any of the BIOSes actually checks for.

@sverx
Copy link

sverx commented Sep 10, 2020

https://wla-dx.readthedocs.io/en/latest/asmdiv.html#smsheader

REGIONCODE should default to 4 when unspecified and ROMSIZE to:

  • $a (if ROM size is 8 KB, SMSHEADER goes at $1FF0)
  • $b (if ROM size is 16 KB, SMSHEADER goes at $3FF0)
  • $c (if ROM size is 32 KB or more, SMSHEADER goes at $7FF0)

because other values are a bad idea and might even trigger the BIOS check bug (see: https://www.smspower.org/Development/ROMHeader#ROMSize0x7fff05Bytes )

vhelin added a commit that referenced this issue Sep 18, 2020
…ike a few commits back. Should fix GitHub issue #333.
@vhelin
Copy link
Owner

vhelin commented Sep 18, 2020

Hi! Sorry for the delay, I've been really busy with other things... I hope it's fixed now, thanks for the report!

@HerrSchatten
Copy link
Author

The region code is fixed now, thanks.

However, romsize only defaults to the correct value for the automatically calculated checksum if smsheader is present (even if there are no values set within smsheader), otherwise it defaults to zero, creating a mismatch between the romsize nibble in the header and the automatically calculated checksum. See sverx's post above for the right default values.

vhelin added a commit that referenced this issue Sep 19, 2020
@vhelin
Copy link
Owner

vhelin commented Sep 19, 2020

Ah, the ROMSIZE value needs to be calculated and written even only .SMSCHECKSUM is defined? I missed that in a hurry. :) Does it work now better?

@Kroc
Copy link

Kroc commented Sep 19, 2020

This has not fixed the checksum being wrong in Sonic1-Z80-ASM, despite no other bytes in the ROM (including the other header properties) being different;

image

Original ROM on top, my build on the bottom. Only these two bytes differ.

@Kroc
Copy link

Kroc commented Sep 19, 2020

Fixed for me, thanks so much!

@vhelin
Copy link
Owner

vhelin commented Sep 19, 2020

I'll add a test for this so if it breaks again in the future we can see that immediately. :)

@HerrSchatten
Copy link
Author

Works perfectly now, thanks.

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

4 participants