-
Notifications
You must be signed in to change notification settings - Fork 817
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
Replace hardcoded banks & addresses in non-mobile code #460
Conversation
home/names.asm
Outdated
@@ -7,7 +7,7 @@ NamesPointers:: ; 33ab | |||
dbw 0, PartyMonOT | |||
dbw 0, OTPartyMonOT | |||
dba TrainerClassNames | |||
dbw $4, $4b52 ; within PackMenuGFX | |||
dbw $4, PackMenuGFX + 60 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make that dba PackMenuGFX + 60
since $4
is BANK(PackMenuGFX + 60)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BANK()
can't evaluate arithmetic (at least I think that's the problem). dbw BANK(PackMenuGFX), PackMenuGFX + 60
is the best that can be done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh well, dbw BANK(PackMenuGFX), PackMenuGFX + 60
is good enough.
mobile/mobile_5f.asm
Outdated
ld a, [rSVBK] | ||
push af | ||
ld a, $1 | ||
ld [rSVBK], a | ||
farcall SaveGameData_ | ||
REPT _NARG |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rept
, shift
, and endr
should be lowercase! (shift
should be tabulated too).
engine/gbc_only.asm
Outdated
@@ -13,7 +13,7 @@ GBCOnlyScreen: ; 4ea82 | |||
ld de, wd000 | |||
ld a, [rSVBK] | |||
push af | |||
ld a, 0 | |||
ld a, 0 ; this is tantamount to selecting Bank 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please clarify that this isn't a typo; writing 0 to rSVBK has the same effect as 1. (SVBK)
While at it, there may be more hardcoded WRAM banks in calls to |
engine/menu.asm
Outdated
@@ -730,15 +730,15 @@ Function24423: ; 24423 | |||
ld a, [VramState] | |||
bit 0, a | |||
ret z | |||
xor a | |||
xor a ; effectively ld a, BANK(sScratch) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other parts of the code just comment the implied zero value: xor a ; PARTYMON
, xor a ; OAKS_POKEMON_TALK
, xor a ; LOW(vBGMap0)
, etc.
I'm currently tracing backward from |
(resolve PR pret#460) # Conflicts: # engine/battle/core.asm # engine/save.asm
Addresses #413.
"Replace" is defined here as replacing constant numbers with
BANK
functions (in the case of banks) or labels (in the case of addresses), except where the use of a bank or address appears unnecessary and therefore can't be clearly labeled (such cases should be explicitly commented, though)."Non-mobile code" is defined here as all code outside
mobile/*.asm
,lib/mobile/main.asm
, andhome/mobile.asm
. (There are still thousands of hardcoded banks & addresses in mobile code.)Replace hardcoded ROM banks.(Possibly unneeded?)Replace hardcoded VRAM banks & addresses.(This isn't advisable while VRAM is still vaguely and incompletely labeled).(fb825a8 is irrelevant and a relic I committed before I knew the scope of this PR. But I left it in since it just makes one minor improvement to mobile code.)