Reorganise the data for more flexilbility

[gmarty]

The ROM files of Maniac Mansion are pretty much full of data. It means any modification (e.g. changing the nametable, adding new tiles or new objects...) is unlikely to fit in the original size.
To get more flexibility with the editing process, the ROM needs to be expanded and the data reorganised. After a few discussions with @gzip, it looks like the easiest route to take is to do something similar to what Maniac Mansion Decoded does.

This requires:

• Expanding the ROM to add 256k (done in 2768736)
• Moving nametable and attributes from the room payloads to the new banks and update their offsets (WIP branch relocate-nametable-attrs)
• Patch the NES runtime to use the new bank

The last point is the tricky one.

The runtime ASM patch needs to be compiled to an IPS patch. In an ideal world, the patch would be compiled on commit to avoid having 2 files out of sync (the original ASM code and the IPS patch).
Maniac Mansion Decoded uses snarfblasm and I don't know if it can compile on Mac.
Once the patch is checked in, it can be applied with RomPatcher.js.

The patch needs to be compatible with all the different versions of the original ROM. That means we may end up needing one patch per version.

In order to break this down as much as possible, the reorganising of the tiles and the title screen data can be done later.

[gzip]

For now I rolled a new patch for you which omits all the decoded data from my end and expects encoded data for the nametables and attribute tables, which you'll add starting in bank 0x11 (banks 0 and 7 are duplicated for runtime purposes). It does nothing with the graphics data and related code, and doesn't touch the title since those are pending in this ticket. It still expands the rom but it shouldn't conflict with what you have.

The code change expects you to write the nametable and attribute table addresses in the room data, at bytes 0x08 (currently unused) and 0x0c respectively. (Contrary to the docs on the scumm wiki, 0x0a actually points to the tiles index and palette, so we need to keep that intact.) It also expects you to write the bank number at (currently unused) byte 0x12, which will be the physical bank number minus 16, e.g. 0x11 - 16 = 0x01.

Granted, I haven't been able to test this since I don't have your relocated data in the rom yet, so please provide instructions here on how to build it if you end up with problems (or a patch containing just the new data is also good in that case).

https://github.com/gzip/nes-6502-maniac-mansion-decoded/blob/scumm-nes/Maniac%20Mansion%20(USA)%20Relocated%20v1_0.ips

[gzip]

If you want to try building snarfblasm:
https://visualstudio.microsoft.com/vs/mac/
https://github.com/gzip/snarfblasm

[gmarty]

I'm looking into your patch. I updated the relocate-nametable-attrs branch. I believe the values are now the ones you expect:

• Initial bank is 17
• The header is updated as per your instructions

To generate the ROM, clone the repo, git checkout the relocate-nametable-attrs branch, and install the dependencies (npm install).

Then run:

node experiments/relocateNtAttrs.js -i path/to/Maniac\ Mansion\ \(USA\).nes

You'll get a ROM called relocated-nt-attr.nes that you can patch with your patch.

You mentioned:

banks 0 and 7 are duplicated for runtime purposes

Am I expected to duplicate the banks myself? This is not done at the moment.

The ROM doesn't work, it freezes after the title screens. I can provide you with the patched ROM if you want.

[gzip]

I'm traveling so won't be able to look at the resulting rom for a few days. A few comments in the meantime...

The patch needs to be applied first since otherwise it will wipe out all the new data with the expansion. My comment about not conflcting only applies to the expansion that you were already doing, not to the new data (sorry if that was unclear in the previous comment).

The patch duplicates banks 0 and 7 so you don't need to do that.

[gzip]

I cloned the repo and can generate the rom. Found one issue here:
https://github.com/gmarty/scumm-nes/blob/relocate-nametable-attrs/src/lib/parser/room/parseRoomNametable.js#L56

That's pointing to the tiles table index followed by the palette. The nametable actually starts at offset+17. As a result the index and palette get get cleared here:
https://github.com/gmarty/scumm-nes/blob/relocate-nametable-attrs/experiments/relocateNtAttrs.js#L74

That's not the only problem though, I'm still working through issues my end.

[gzip]

I think you also want to add 0x8000 to the nametable/attribute offsets so that they're memory addresses:
https://github.com/gmarty/scumm-nes/blob/relocate-nametable-attrs/experiments/relocateNtAttrs.js#L97-L98

[gmarty]

Ah I see the problem. I'll work on a fix for both issues.

Another thing I forgot to mention, and in case it's no obvious, but the parser and the internal logic works on PRG (not ROM). Most of the code comes from SCUMMVM that does it that way. The iNES header is stripped before parsing and added back when a NES needs to be created. So most offsets will be off by 16 bytes.

[gmarty]

I pushed a new commit to the WIP relocate-nametable-attrs branch. I think it addresses both issues you mentioned. This time, the ROM boots normally but freezes on the character selection screen. Git pull the new version to try it out.
See https://github.com/gmarty/scumm-nes/blob/relocate-nametable-attrs/experiments/relocateNtAttrs.js

[gzip]

Found another issue, these are both off by one so it causes all kinds of havoc during execution. E.g. the last byte of the new nametable gets clobbered by the new attribute table, which is missing a byte at the end as well. The original data also has the 2 bytes left over.

https://github.com/gmarty/scumm-nes/blob/relocate-nametable-attrs/src/lib/parser/room/parseRoomNametable.js#L57
https://github.com/gmarty/scumm-nes/blob/relocate-nametable-attrs/src/lib/parser/room/parseRoomAttributes.js#L28

[gzip]

That was much more complicated than I had anticipated due to issues on both sides, but I finally got it all working together (including the off by one errors above). The patch can now be applied after your build. The previous patch was incomplete so I've updated the link in the original reply. #9 (comment)

[gmarty]

Thanks for spotting it! I published a fix for the off by 1 offsets.
I managed to make the game work with the nt and attrs relocated to the new banks! The game freezes at some point though. I've not managed to reproduce exactly what makes it freeze, but it happened a few times on the character selection screen and then later in the mansion when entering room 12 (2nd Floor Hallway). There were also very minor graphic glitches on the character screen that happened once.

I'll land the branch with the relocation code and we can enquire the bug and land a fix in a subsequent commit.

[gzip]

It looks like the freeze is related to the MMC1 bank register getting into an unknown state. MMC1 requires 5 writes to change banks and I suspect that nmi is triggering in the middle of a bank switch which then doesn't recover correctly. This causes an unexpected bank to be loaded later on, then RLE data is read from the wrong location and spills into other memory. A typical buffer overflow. The workaround is to reset the register by writing the high bit before changing banks. I've updated the patch.