bfirsh / bfirsh/jsnes

MMC1 (mapper 1): SUROM 512KB games crash — fixed bank at $C000 doesn’t follow the 256KB page-select bit

Open
#732 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
6.4k
Forks
857
PR merge metrics
No merged PRs in 30d

Description

## Summary

512KB SUROM games (MMC1 / mapper 1, 32×16KB PRG banks, CHR-RAM) crash with an invalid opcode after running for a while. The root cause is that the **fixed PRG bank at `$C000` does not follow the 256KB page-select bit**, plus two related bank-calculation bugs in `setReg`.

- **jsnes version:** 2.1.0
- **Mapper:** 1 (MMC1, SUROM configuration)
- **Test ROM:** Dragon Quest IV (J) — `PRG = 512KB (32×16KB)`, `CHR = 0 (CHR-RAM)`, `mapper = 1`
- **File:** `src/mappers/mapper1.js`

## Symptom

The game boots and runs for ~890 frames, then throws:

```
Game crashed, invalid opcode at address $b1d9
```

(The address varies depending on which fix is partially applied.)

## Root cause

Per the [NESdev wiki MMC1 page](https://www.nesdev.org/wiki/MMC1):

> The 256 KB PRG bank selection applies to all the PRG area, **including the normally "fixed" bank**.

On SUROM, the 256KB page-select bit (`romSelectionReg0`, bit 4 of reg 1) selects which 256KB half of the ROM is visible — and it applies to the fixed `$C000-$FFFF` bank too:

- Lower page (P=0): `$C000` should map to bank 15 (last bank of the lower 256KB)
- Upper page (P=1): `$C000` should map to bank 31 (last bank of the upper 256KB)

`setReg` only ever remaps the switchable bank at `$8000` and leaves `$C000` pinned to bank 31. When the game switches to the lower page, page-dependent jump tables / vectors in `$C000-$FFFF` desync, a far-call target gets corrupted, execution jumps into a data region, and the CPU hits an invalid opcode.

## Three bugs found in `mapper1.js` `setReg` (`default` case)

**1. 16KB-mode bank doubling.** `baseBank` is already in 16KB-bank units, but it's multiplied by 2 before being passed to `loadRomBank` (which takes a 16KB index):

```js
// current
bank = baseBank * 2 + (value & 0xf); // baseBank=16 -> 32, then % 32 -> bank 0 (wrong)
// fixed
bank = baseBank + (value & 0xf);
```

**2. Page-select (reg 1) write doesn't remap PRG.** Games that write reg 3 (bank number) before reg 1 (page bit) never get the page change applied, because only the reg-3 write triggers a PRG load.

**3. Fixed `$C000` bank doesn't follow the page bit (the main bug).**

## Suggested fix

Factor the PRG-load logic out of the `default` case into an `applyPrgBank()` method, call it from **both** the reg-1 (`case 1`) and reg-3 (`default`) writes, and make it remap the fixed bank for SUROM:

```js
// in setReg case 1, after setting romSelectionReg0:
this.applyPrgBank();

// in setReg default:
this.romBankSelect = value & 0xf;
this.applyPrgBank();

applyPrgBank() {
let bank;
let baseBank = 0;
if (this.nes.rom.romCount >= 32) {
if (this.vromSwitchingSize === 0) {
if (this.romSelectionReg0 === 1) baseBank = 16;
} else {
baseBank = (this.romSelectionReg0 | (this.romSelectionReg1 << 1)) << 3;
}
} else if (this.nes.rom.romCount >= 16) {
if (this.romSelectionReg0 === 1) baseBank = 8;
}

const value = this.romBankSelect;
if (this.prgSwitchingSize === 0) {
bank = baseBank + (value & 0xf); // fix #1: no *2
this.load32kRomBank(bank, 0x8000);
} else {
bank = baseBank + (value & 0xf); // fix #1
if (this.prgSwitchingArea === 0) this.loadRomBank(bank, 0xc000);
else this.loadRomBank(bank, 0x8000);
}

// fix #3: SUROM fixed bank follows the 256KB page bit
if (this.nes.rom.romCount >= 32 &&
this.vromSwitchingSize === 0 &&
this.prgSwitchingSize === 1 &&
this.prgSwitchingArea === 1) {
this.loadRomBank(this.romSelectionReg0 === 1 ? 31 : 15, 0xc000);
}
}
```

Remember to add `this.romBankSelect = 0;` to the constructor and include it in `toJSON`/`fromJSON`.

## Verification

With all three fixes, the test ROM runs 3000+ frames with no crash. A synthetic MMC1 test ROM also confirms both 16KB and upper-page bank switching work correctly.

(The fixed-bank value `15`/`31` is specific to the 512KB/32-bank case; a fully general implementation would compute it from `romCount` and the page width.)

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in src/mappers/mapper1.js, tracing setReg plus the constructor and toJSON/fromJSON state handling. Verify the MMC1 fixes with Dragon Quest IV (J) and the synthetic MMC1 ROM; done means both 16KB and upper-page switching work and Dragon Quest IV runs for 3000+ frames without an invalid opcode.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
game-dev
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.