Register ordering, byte-based sizes, OAM additions, general cleanup #4
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ISSOtm
reviewed
Apr 22, 2019
There was a problem hiding this comment.
Positive:
- I really like the ordering of the registers, especially as they're naturally grouped (are there exceptions? I'm not aware of any)
- I also like the new constants. That said,
CPU_CGBcould be renamed to something more explicit?BOOTUP_CGB, maybe?
Negative:
- The
oXXchange is useless imo, because it's strictly equivalent toLOW(rXX).
Neutral:
_HW. Also I'd rather have called it_IO
Unrelated:
- What about adding byte constants for ROM and SRAM sizes? Everyone uses bytes nowadays.
- Add more memory map convenience equ's - Add rSMBK which makes more sense (to me) than rSVBK - Revert the 'o' equ's - Rename the bootup CPU values - Changed to kilobyte-based size equ's
BlitterObjectBob
changed the title
|
I have a local change (not committed yet) related to the bootup values: Thoughts on including this in the PR? Thoughts in general? |
BlitterObjectBob
changed the title
|
I've included them (plus OAM changes) in the PR for commenting there. |
|
Bump :) |
ISSOtm
reviewed
Aug 6, 2020
ISSOtm
requested changes
Aug 6, 2020
ISSOtm
reviewed
Aug 7, 2020
|
Suggestion: |
ISSOtm
requested changes
Aug 28, 2020
hardware.inc
Outdated
| @@ -739,7 +739,7 @@ BOOTUP_A_MGB EQU $FF ; Mini GameBoy (Pocket GameBoy) | |||
| ; if a=BOOTUP_A_CGB, bit 0 in b can be checked to determine if real CGB or | |||
| ; other system running in GBC mode | |||
| BOOTUP_B_CGB EQU %00000000 | |||
| BOOTUP_B_OTHER EQU %00000001 ; GBA, GBA SP, Game Boy Player, or New GBA SP | |||
| BOOTUP_B_AGB EQU %00000001 ; AGB, AGB SP, Game Boy Player, or New AGB SP | |||
There was a problem hiding this comment.
Either "GBA SP" or "AGS", but pick one :p
ISSOtm
approved these changes
Aug 28, 2020
|
Wow, this went a long way. I like the changes. Thank you for sticking with this, @BlitterObjectBob and @ISSOtm ! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm not a fan of the 'o'-named register EQUs (for offset), but couldn't think of anything else.