hardware.inc icon indicating copy to clipboard operation
hardware.inc copied to clipboard

rHDMAx alternative names

Open zlago opened this issue 2 years ago • 12 comments

some registers have alternative, more "user friendly" names (rKEY1, rNRxx, CGB palette registers, CGB banking registers) but since rHDMAx registers dont have alternative names and (according to hardware.inc and pandocs) are big-endian, this could result in some nasty suprises if someone doesnt double check the order my suggested alternative names for rHDMAx registers would be: rHDMA_SRC_HI, rHDMA_SRC_LO, rHDMA_DST_HI, rHDMA_DST_LO, and rHDMA_LEN

zlago avatar Nov 11 '22 10:11 zlago

after looking into it a bit more, here are more suggestions/questions i have:

  • the CGB banking register are.. not exactly intuitive, how about adding rSWBK/rWBK for the WRAM bank?
  • would anyone use a constant for the unreliable WX=0 and WX=166?
  • a constant for the size of a CGB palette (4 colors)
  • CGB palette constants have flag versions for their alternative names, but some other constants dont (rSPD but no SPDF_PREPARE or SPDF_DBLSPEED)
  • why is it KEY1F_DBLSPEED and not just KEY1F_SPEED?
  • a constant for $e4? something like BGP_DEFAULT?

zlago avatar Nov 13 '22 14:11 zlago

Do we have any registers with an underscore in their name? (On that point, this might overlap #26.)

If we are renaming those registers, I'd go the extra mile and call them rVBANK and rWBANK, for example.

hardware.inc aims to provide constants that would be used often, so providing some for fringe cases is imo not worth it. Mentioning the values in the doc comment if they aren't already would be good.

At that point, why not one for the number of palettes, or the byte size of a colour? Why that specifically?

Arguably, KEY1 bit 7 can be treated as a boolean indicating whether double-speed mode is active, so the name seems logical.

That constant is nothing particular, and if we start being opinionated, @aaaaaa123456789 won't let me hear the end of it.

ISSOtm avatar Nov 13 '22 14:11 ISSOtm

  • uhh poor wording on my part i assume
  • okay rVBANK and rWBANK makes much more sense than more arbitrary names
  • just wanted to ask if a constant for an edge case could belong in hardware.inc
  • i was thinking of CGB palette constants in general
  • okay
  • rBGP = $e4 is used for SGB VRAM transfers, and we could also define an LCDC constant for that

zlago avatar Nov 13 '22 15:11 zlago

  • i was thinking of CGB palette constants in general

That is what I was replying to.

  • rBGP = $e4 is used for SGB VRAM transfers, and we could also define an LCDC constant for that

Okay, maybe a BGP_IDENTITY constant could make sense, then?

ISSOtm avatar Nov 13 '22 16:11 ISSOtm

That makes little sense, because (1) it's the identity for all palettes, not just BGP and (2) it's just a random constant. Unless the goal of hardware.inc is to pollute people's namespaces as much as possible...

Please remember this is not a "library of good stuff"; it's a file meant for hardware constants. You could argue that the default palette mapping is a hardware constant, but that's the worst slippery slope ever: how long until every register gets its constant?

aaaaaa123456789 avatar Nov 13 '22 16:11 aaaaaa123456789

I didn't want to include it as "the default mapping" either, but as "the palette you will want for your SGB VRAM transfers", I think it makes sense. I don't think that rationale is a slippery slope, either?

ISSOtm avatar Nov 13 '22 16:11 ISSOtm

okay so to make sure were all on the same page

  • no one has anything against the suggested rHDMAx alternative names
  • banking registers will get alternative names
  • possibly some constants for Color RAM?
  • $e4 clearly marked as a SGB VRAM transfer constant
  • seemingly no response to my (as i understand it, poorly worded) question about why only some flag/byte constants have versions for alternative names? (rBCPS/BGPI has BCPSF_AUTOINC and BGPIF_AUTOINC, while rKEY1/rSPD has KEY1F_PREPARE but not SPDF_PREPARE)

zlago avatar Nov 13 '22 17:11 zlago

I didn't want to include it as "the default mapping" either, but as "the palette you will want for your SGB VRAM transfers", I think it makes sense. I don't think that rationale is a slippery slope, either?

This is just a convenience, and at worst, a convention. If you always have your palette set to some other color permutation, you can perfectly set up the SGB data to use that permutation so that you don't have to switch palettes. (I wonder if this is more efficient.) Likewise, you can, as we discussed earlier, manipulate that value for effects — the fact that few games can take advantage of those effects doesn't make them not exist.

aaaaaa123456789 avatar Nov 13 '22 18:11 aaaaaa123456789

Since I haven't commented on the register name issue: I'd rather not have hardware.inc invent register names. As I said: it is not a "library of good stuff". The alternate names that are already there have to remain there for backward compatibility, and at the same time, they were already in use by the community. But the goal should never be to sanction names that don't exist yet: it is a terrible idea for a project like this to become a source of descended-from-above invented conventions that two or three people decide in a random backroom for everyone to use.

If you want to create your own names for your own projects, there's a million better ways that don't intrude on other people's code.

aaaaaa123456789 avatar Nov 13 '22 18:11 aaaaaa123456789

Do we have any registers with an underscore in their name? (On that point, this might overlap #26.)

only now did i realise you meant my suggested rHDMAx alternatives.. as it turns out, when writting them, i was looking at flag constants and not register constants.. yikes!

and i agree with ax6, should we close the issue?

zlago avatar Nov 14 '22 15:11 zlago

There is, in fact, a good reason to invent register names: what good are they if they're opaque? I can give LCDC and STAT a pass for being at least somewhat evocative of their topic, but HDMA1–5 really aren't.

Besides, we already have a precedent, in the form of the audio registers, and I remember the addition being well-received.

ISSOtm avatar Nov 15 '22 19:11 ISSOtm

how about an "experimental" update? the constants get a clear "these may be removed at any point" label, and depending on user feedback they will either stay, or be deprecated/removed?

zlago avatar Nov 15 '22 19:11 zlago