Use hardware.inc from gbdev

[vulcandth]

@ISSOtm mentioned this on discord today, that we should be using hardware.inc from https://github.com/gbdev/hardware.inc/blob/master/hardware.inc. Looking at the discord history, It sems this was discussed in the past, but I didn't see a clear definitive reason why or why not use it. A couple points on why not seemed to do with extra "noise", that it might be based on MBC5, and how pokecrystal uses more constants related to individual bits.

Anyways, I'm going to let you all who have more experience weigh in on this. I'm just opening this to start a discussion. If we decide to take action with this, i'll help out.

[mid-kid]

IIRC one of the issues was that it doesn't have all the defines that we need, but if we need them, then they're likely very useful to other projects as well, so we should be contributing them upstream as well. Until then it's fine to have a modified hardware.inc in this repository.

Other than that it's just porting busywork.

[vulcandth]

Would it be hard to use hardware.inc and then define new pret constants in a separate file? Although I agree, it could just be busy work.

[aaaaaa123456789]

I've always been a strong advocate of just taking whatever you need from hardware.inc instead of polluting your namespace with lots of constants you have no intentions of using. After all, hardware.inc describes hardware that has been out of manufacturing for over 20 years; there's no chance for that hardware to change, so keeping "up to date" with changes upstream is irrelevant here.
While it would be a very bad idea to use hardware.inc's constant names for other things, I don't see the harm in leaving most of them out.

And yes, the focus on MBC5 is problematic. I have no idea of why hardware.inc makes this choice, as not even all homebrew uses MBC5. But it does, and it would collide with the MBC3+RTC cart that Pokémon Crystal uses.

[ISSOtm]

The advantages are not much, but still exist:

• Consistency with other projects enable easier code reuse (especially across wikis and forum posts). Many constants for the same things have different names.
• Given that it's relied on by many other projects, it will stay up to date with breaking updates to RGBDS without needing extra attention—simply a matter of reducing the code yall are directly responsible of. This is what "keeping up-to-date" I was arguing in favor of.
• Conversely, constants (such as the bit constants mentioned in the OP) used in pokecrystal but not currently in hardware.inc are likely to be useful to other projects as well, so the cooperation goes both ways. (I realize after typing this that this is actually exactly's mid-kid's comment, oops.)

I'm not sure what focus hardware.inc might be having? Grepping -i for MBC5 yields the cartridge types (which are obviously legitimate), and a single comment, which is technically not wrong but can be changed if you really really want.

[aaaaaa123456789]

Keeping up to date with RGBDS is part of what I'm worried about — pokecrystal occasionally lags behind, so it's better to update those definitions along with everything else. The effort is minimal and we ensure that we can always build the repo with a single version of the toolchain. (This is not much of a concern as long as pokecrystal is part of RGBDS's CI, but we're talking about minor differences here.)

The MBC5 constants (the ones under that comment) are simply incorrect for other cart types. $3000 maps to the ROM bank, not to the "ROM bank upper byte", for instance. (And it makes no sense to speak of a "byte 0" when there's only one byte.)

What differences in constants can you find?

[mid-kid]

Keeping up to date with RGBDS is part of what I'm worried about — pokecrystal occasionally lags behind, so it's better to update those definitions along with everything else.

This is a non-issue. The file would be copied and checked into this repository. Even if the repository were to be a submodule (it won't), the version would be pinned.

Most of the benefit is in following community standards and contributing to how the hardware definitions are specified, as well as making it easier for users to use additional hardware that we haven't even considered adding to hardware_constants.inc (for example, MBCs), or future clarifications of or additions to the constants.

[aaaaaa123456789]

Most of the benefit is in following community standards and contributing to how the hardware definitions are specified...

Don't we already follow standards in everything but MBC registers (where we can't, because hardware.inc doesn't implement MBC3)?

...as well as making it easier for users to use additional hardware that we haven't even considered adding to hardware_constants.inc (for example, MBCs)...

hardware.inc doesn't support anything but MBC5, and what other additional hardware are you referring to?

...or future clarifications of or additions to the constants.

I don't see much changing when we're talking about a device that has been out of manufacturing for 20 years.

If we're going to take a snapshot of the file and check it in, what's there to gain with respect to what we already have, which is essentially a snapshot with the constants we use?

[mid-kid]

Don't we already follow standards in everything but MBC registers

We don't

what other additional hardware are you referring to?

GB hardware and bit definitions that the game doesn't use.

what's there to gain with respect to what we already have

Keeping things in sync. If we were already using all the same constants it'd be a drop-in. It isn't.

[aaaaaa123456789]

I think a step we can agree on is to rename our constants to match hardware.inc's names, then?

[mid-kid]

Yes. Although there's no reason not to use and contribute to hardware.inc at that point. Polluting the namespace with definitions relating to common gameboy hardware simply isn't a concern, and the MBC5-specific definitions can be ignored.

[ISSOtm]

And, as I've asked in my previous post, how does hardware.inc "only" support MBC5? It does not have anything MBC-specific.

[rawr51919]

Draft PR exists that has a proof-of-concept working compile and matching builds using latest RGBDS master as well as RGBDS v0.5.2 (#943)

[rawr51919]

Said hardware.inc PR will also need companions across the whole of pret, updating their builds to use the latest hardware.inc if they use it already and refactoring them to use hardware.inc if they don't