Allow custom charmaps

[BlueAnthem37510]

Allows user to set custom character map(s) using the RGBDS character mapping

Should hopefully satisfy issue #33 also? :)

To use this a path to the file containing the character map(s) is added to the command. This file will be copied to the disassembly and included in game.asm

To mark which text should use the custom character map ":cm" is added to the end of the text label otherwise it will use the default to handle text. Additionally ",[index]" may be used after this to specify which character map should be used if there are multiple.

Please see below for example using Dragon Warrior Monsters.

Command line

Symbols

Character map file

Disassembly

game.asm

[ISSOtm]

Hello! Thank you for the PR. I really like the idea!

However, before reviewing the code itself, I would like to iterate on the design (mostly the syntax) a little.

Noteworthy also is that RGBDS v0.9.0 has added multi-value charmaps (CHARMAP "<br>", 13, 10), which may be a little more complicated to handle, since you'd start needing leftmost-longest semantics. (As mentioned above, I haven't looked at the implementation, so I don't know if you have implemented such support.)

[BlueAnthem37510]

Hi @ISSOtm thank you for checking my pr and the suggestions!

I had been waiting for multi character support to be added forever I can't believe I missed that! Makes the text look so much cleaner.

Regarding the syntax I've compromised a little (or perhaps overdoing it?) by allowing "cm" or "charmap" and indexing or the character map name

I added some warnings also if the user makes a typo or an out of range index, I'm happy for this to be handled a different way though.

Let me know what you think!

[ISSOtm]

Thank you for this patch, and my apologies for taking a while to respond.

It's a lot of code, so I've only reviewed a bit of it. Feel free to make more similar changes if you think they'll be appropriate, too!

[BlueAnthem37510]

No worries! I've moved the getting "character map index" into a function and implemented your changes. Hopefully should be more readable now!

[mattcurrie]

@BlueAnthem37510, hey thanks for putting this PR together!

I've just added a test in the master branch which builds a test ROM and then disassembles it and checks the result against some expected disassembly and the original ROM.

The test tries a few different things using character maps, so if you could try merging master into your branch and then hopefully it should just be a case of running make in the /test directory and then updating the files in /test/expected from /test/disassembly to get the test to pass after your changes.

[mattcurrie]

Thanks for those changes! I just found a few minor things that could be tweaked when I tried with my test character map.

[ISSOtm]

Thank you for seeing this through!
I do have a couple of QA changes to request.

[ISSOtm]

Looks good to me!

I'll admit I skimmed the code that actually does the character (reverse) mapping, but I'll assume that you tested it.

Note that there can be issues with some charmaps, for example:

charmap "a", 0
charmap "b", 1
charmap "ab", 2

...and reversing the bytes 00 01 would produce db "ab", which would not-round-trip to 02.

The fix is to split the string (db "a","b"). I would totally understand if you choose not to handle this specific edge case, but I think it would be a good idea to mention it in the manual, so it wouldn't catch other people off-guard. (Possibly inside of a <details> element, to keep it skimmable?)

[mattcurrie]

@ISSOtm, I've just tried that edge case and it seems to be working. I'll merge and update the test to use the charmap.

@BlueAnthem37510 thanks again for your work on this. Great to have this functionality!