gbdev / hardware.inc

RGBDS include file for Game Boy hardware definitions
Creative Commons Zero v1.0 Universal
119 stars 21 forks source link

Defining F values based on B values #42

Open Rangi42 opened 3 months ago

Rangi42 commented 3 months ago

The hardware.inc convention is for <REG>B_<FIELD> to be a 0-7 bit value, and <REG>F_<FIELD> to be the corresponding mask value. A few examples:

DEF LCDCF_OBJ16   EQU %00000100 ; OBJ Construction
DEF LCDCB_OBJ16   EQU 2 ; OBJ Construction

DEF PADF_START  EQU $08
DEF PADB_START  EQU $3

DEF OAMF_PRI        EQU %10000000 ; Priority
DEF OAMB_PRI        EQU 7 ; Priority

I'd like to make these consistent in one of three ways:

  1. Always use binary, e.g. %00000100.
  2. Always use hex, e.g. $08.
  3. Define the F values in terms of the B values, e.g. DEF OAMF_PRI EQU 1 << OAMB_PRI.

The advantage of option 3 would be avoiding redundancy or mismatch between the values, making the connection obvious.

The advantages of 1 or 2 would be explicitly showing the numeric mask value (although that could still be a comment next to the 1 << B, in either bin or hex).

My own preference is for option 3, but I wanted to get feedback from the maintainers here before PRing anything.

(If that is the preference, I'd even consider going further and having a macro like DEF_FLAG OAMB_PRI EQU 7, OAMF_PRI to reduce the line count.)

ISSOtm commented 3 months ago

I think single-bit values should use 1 << B syntax, and that the macro is a good idea because it would severely reduce the line count, and thus make the file more convenient to browse, with no significant loss in readability.

@aaaaaa123456789 sugested hw_flag 7, OAMB_PRI, OAMF_PRI, whose definition is more trivial; however, that syntax obfuscates somewhat which constant has what value, whereas hw_flag OAMB_PRI equ 7, OAMF_PRI should at least make it obvious what the former's value is.

(Also, the name hw_flag is less likely to conflict with a user-defined macro name.)

aaaaaa123456789 commented 3 months ago

If the names of the constants don't make it clear which is which, the constants themselves are confusing (i.e., unrecognisable on use) and shouldn't be defined at all! In other words, which value is which is made clear by the constants' names.

ISSOtm commented 3 months ago

That's a very good point!

I find that the current names are clear, but not intuitive. Because B is for “bit” (index), whereas F is for “flag” (bitmask). (But, see also #26.)

I just think that it should remain easier to learn that scheme, since it does take a little getting used to.

Rangi42 commented 3 months ago

Personally I find the B and F markers hard to notice, in the middle of the names. Conventional Hungarian notation would have them as a prefix, or the projects I'm used to have similar markers as suffixes.

If backwards compatibility weren't so important I'd suggest _BIT and _MASK.

nitro2k01 commented 2 months ago

My hardware-ex.inc project (which is not yet public in a usable form) aims to extend hardware.inc to support Mega Duck and Analogue Pocket's .pocket format. For that project I preferred method 3 since constants can have up to three definitions, and it's nice that each unique value is only defined in a single place. I'd strongly suggest this method.

As for prefix vs suffix, I personally prefer prefix, but this is not a strong opinion.