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

Support Multiple Charmaps/Disable Charmap #256

Closed
RenaKunisaki opened this issue Mar 13, 2018 · 14 comments · Fixed by #403
Closed

Support Multiple Charmaps/Disable Charmap #256

RenaKunisaki opened this issue Mar 13, 2018 · 14 comments · Fixed by #403
Labels
enhancement Typically new features; lesser priority than bugs rgbasm This affects RGBASM

Comments

@RenaKunisaki
Copy link

Right now it looks like charmap overrides any previous mapping including ASCII. This means you can't have more than one mapping, or use ASCII and a custom map in the same ROM (eg for debug output form emulators). Would be great to have a way to specify which mapping to use for a given string or group of strings.

@aaaaaa123456789
Copy link
Member

How would you do this?

@Sanqui
Copy link
Member

Sanqui commented Mar 16, 2018

One way to do this would be a setcharmap directive, which would switch the behavior of charmap and db. A disadvantage is that it introduces state.

    setcharmap ascii
    charmap "A", $41
    charmap "B", $42
    
    setcharmap game
    charmap "A", $10
    charmap "B", $11
    setcharmap game
    db "Game string@"
    setcharmap ascii
    db "ASCII encoded string"

Then you could make macros such as

ascii: MACRO
    setcharmap ascii
    db \1
    setcharmap game
ENDM

    db "regular text"
    ascii "ascii text"

@dbrotz
Copy link
Contributor

dbrotz commented Aug 29, 2019

setcharmap looks like a good idea to me, but I have some questions.

What about the "default charmap" that exists before you use setcharmap? How do you return to it after switching? Should we just give it the name "default"?

; implicit default charmap
charmap "A", $10
charmap "B", $11

setcharmap other
charmap "A", $20
charmap "B", $21
db "A"

setcharmap default
db "A"

Another thing to consider is the state that a new charmap starts off with. Does it inherit the current charmap's mapping or start with some predefined initial mapping (e.g. ASCII)?

@ISSOtm
Copy link
Member

ISSOtm commented Aug 29, 2019

I would agree with hardcoding the default charmap to default (ie. that charmap is implicitly created and loaded by the assembler when beginning assemby).

I think creating a new charmap should inherit from the current one, that way if you want to base it off the default mappings you simply do setcharmap default beforehand, and it allows making "base" charmaps that deduplicate some charmaps' common parts.

@dbrotz
Copy link
Contributor

dbrotz commented Aug 29, 2019

That sounds good to me.

Another thought I just had is that we might want to distinguish between creating a new charmap and switching to an existing one. The benefit is that it might help catch errors where someone thinks they're creating a charmap but they're switching to an existing one or vice versa. The downside is that there would be more directives to remember.

Specifically, newcharmap x could be used to initially create the new charmap called "x" and then setcharmap x would switch to it. If you used newcharmap x twice or if you used setcharmap x before newcharmap x, you would get an error. The alternative would be to just have setcharmap x and create the "x" charmap on its first use and switch to it on later uses.

@dbrotz
Copy link
Contributor

dbrotz commented Aug 30, 2019

Also, I don't think my suggestion of "default" is the best idea. It could be confusing because it sounds like it would be the charmap that the assembler begins with, without any changes. "main" would be a better name for it.

@ISSOtm
Copy link
Member

ISSOtm commented Aug 30, 2019

Well, that charmap is the one the assembler begins with, isn't it?

The problem with requiring a newcharmap is that it might not be backwards-compatible. Maybe a clearer name for setcharmap would be usecharmap, or switchcharmap? That way it stays akin to what it is now and would just be a context switch.

It does mean that you can keep defining a charmap as you go, which isn't very clean, but that's already what happens with the current system.

Lastly, you can decide to break backwards compatibility in that regard (hopefully in a mild manner), and keep this feature for 0.4.0.

@dbrotz
Copy link
Contributor

dbrotz commented Aug 30, 2019

Yes, but to me, "default" sounds like it's what the assembler begins with, without any modifications by the user. I was thinking that you would start with the "main" charmap already in existence and be able to use charmap to alter it, to maintain backward compatibility. You would only use newcharmap if you wanted to have more than one charmap.

@ISSOtm
Copy link
Member

ISSOtm commented Aug 30, 2019

The thing I'm slightly worried about is, to maintain backwards compatibility, the main charmap would be special-cased to be modifiable at any time.

@aaaaaa123456789
Copy link
Member

aaaaaa123456789 commented Aug 30, 2019

Under the risk of making suggestions to an issue that already has a pull that closes it, how about this?

  1. Define charmaps as a block, just like everything else is defined:

     game: CHARMAP ; reusing the keyword shouldn't be a problem, but change it if needed
       charmap "A", $01
       charmap "B", $02
     CEND
    
  2. Use charmaps with setcharmap, as proposed; use the assembler's default charmap by passing no name at all (which removes the need to hardcode a name):

       setcharmap game
       db "AB" ; db $01, $02
       setcharmap
       db "AB" ; db $41, $42
    

Note that the assembler's default charmap would still be modifiable via charmap statements. This isn't really a problem for ASCII-users, as they can declare a new empty charmap for ASCII.

This proposal eliminates accidental charmap definitions, as definitions and usage look nothing alike. It still has the problem of state, though.

EDIT: one way of eliminating the state issue would be to introduce yet another block that creates a charmap scope:

  charmap game
    db "AB" ; db $01, $02
  endc
  db "AB" ; db $41, $42

This would only really work for occasional usage of alternate charmaps, though. It also introduces yet another overload for charmap (0 arguments: begin charmap declaration block, 1 argument: begin charmap scope block, 2+ arguments: declare character), but that shouldn't be an issue; if it is, use separate keywords.

@dbrotz
Copy link
Contributor

dbrotz commented Aug 30, 2019

I think the charmap definition block could be good. It looks odd that you have to repeat CHARMAP on each line inside the block, though. It would still be nice to be able to inherit another charmap somehow, as well.

@ISSOtm
Copy link
Member

ISSOtm commented Aug 30, 2019

Either the charmap could inherit from the current one (charmap text then jp_text: CHARMAP), or explicitly inherit from one (jp_text: CHARMAP text).

@dbrotz
Copy link
Contributor

dbrotz commented Aug 31, 2019

When I tried putting charmapdef : T_LABEL ':' T_POP_CHARMAP in the grammar, it added an extra shift/reduce conflict.
There really isn't any reason to put a label before the current CHARMAP directive, but it was allowed. Using the same keyword will break any code did something like this:

label: charmap "a", 1

Is it worth using a new keyword to avoid that?

@ISSOtm
Copy link
Member

ISSOtm commented Aug 31, 2019

Using a new keyword would make more sense (so as not to overload the charmap keyword with too much sense), but it could break code that used such a macro.

I'm starting to doubt the practicality of introducing a new variant of this construct, actually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Typically new features; lesser priority than bugs rgbasm This affects RGBASM
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants