Conversation
kirjavascript
left a comment
There was a problem hiding this comment.
thanks for the contribution!
I left a few comments but I dont mind addressing them myself
src/header.asm
Outdated
|
|
||
| INES_MIRROR = 0 ; 0 = horizontal mirroring, 1 = vertical mirroring (ignored in MMC1) | ||
| INES_SRAM = 1 ; 1 = battery backed SRAM at $6000-7FFF | ||
| INES_SRAM = SAVE_HIGHSCORES ; 1 = battery backed SRAM at $6000-7FFF |
There was a problem hiding this comment.
we should probably add a HAS_SRAM flag for clarity
There was a problem hiding this comment.
Should that flag be a new addition, or should it replace an existing flag? I think replacing SAVE_HIGHSCORES would make the most sense here. The header should ideally be a set-and-forget kind of thing in my opinion.
There was a problem hiding this comment.
I think it should be an additional flag so the saving highscores can be disabled separately if needed.
In a newer version we might decide to remove or replace it and leaving the name makes it easier to remove.
There was a problem hiding this comment.
I added the HAS_SRAM flag but disabling it is currently lumped with the -s build flag.
This PR upgrades the header from iNES to the forwards-compatible NES2.0 in order to better describe the cart specification:
SAVE_HIGHSCORESflag in the header (previously, SRAM was always enabled in the header)-ibuild flag for legacy iNES header supportDocument workaround for NES2.0 header issues on PowerPakWith the BPS patch now being header-agnostic, this should help with adapting to the transition from iNES to NES2.0. (e.g. as seen in #19)