Skip to content
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

OAM data documentation; move more content into data/; move palettes into gfx/; more code+script constants; consistent map naming #456

Merged
merged 242 commits into from
Jan 22, 2018

Conversation

roukaour
Copy link
Contributor

@roukaour roukaour commented Jan 1, 2018

Also merges:

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
Copy link
Contributor

@yenatch yenatch Jan 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment was just to indicate that it wasn't a valid address. It's not actually referencing PackMenuGFX. My guess would be it's a pointer to MoveDescriptions (0b:4b52). The $4, $4b52 is $4, $4000 in Gold. The address of MoveDescriptions in Gold is also at 0x4000 (6d:4000). The bank is wrong still, but 04:4000 in Gold was GetPlayerMovement, so the bank was probably wrong to begin with.

@@ -26,7 +26,7 @@ DoBattle: ; 3c000
and a
jr z, .not_linked

ld a, [hLinkPlayerNumber]
ld a, [hSerialConnectionStatus]
cp $2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some hSerialConnectionStatus constants (like USING_INTERNAL_CLOCK) are missing in code, more specifically in engine/battle/core.asm, home/init.asm, mobile/mobile_40.asm, and a few in engine/link.asm. Pointing out in case you haven't noticed, because you may already be aware.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Even with that the link code has much left to do (e.g. the wPlayerLinkAction and wOtherPlayerLinkMode values, the many wXXXX labels, hardcoded sizes like $1b9), but I don't have the knowledge of GB serial hardware to do it.

@@ -15,6 +15,9 @@
const DEXSTATE_UPDATE_UNOWN_MODE
const DEXSTATE_EXIT

POKDEX_SCX EQU 5
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pokdex?

engine/types.asm Outdated
jp CopyBytes
; 5097b


INCLUDE "data/types/names.asm"


Unreferenced_GetGen1TrainerClassName: ; 50a28
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what? why is this here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it go in engine/routines/unreferenced_getgen1trainerclassname.asm?

wram.asm Outdated
@@ -1565,7 +1565,7 @@ wDaysSince:: db

SECTION "WRAM 1", WRAMX

wd000:: ds 1
wDecompressBuffer:: ds 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be a union?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, there are no uses of unidentified wd000 left.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pretty sure wDecompressBuffer is not 1 byte large

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I see. You could, but there's a big union right after it at d002, and every NEXTU would need a ds 2 at the beginning. How about a comment noting that wDecompressBuffer is larger than that? (Max size: GBCOnlyGFX takes $540 bytes uncompressed, and I think LZ_170d16 takes $c10 in mobile_5c.asm. Oh, mobile_5c.asm should have used wDecompressScratch, not wDecompressBuffer. So yeah, $540 bytes, and wGBCOnlyDecompressBuffer would be a more accurate name.)

charmap.asm Outdated
charmap "<ROUTE>", $35 ; "ばん どうろ"
charmap "<WATASHI>", $36 ; "わたし"
charmap "<KOKO_WA>", $37 ; "ここは"
charmap "<GA>", $4a ; "が "
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a feeling these are a 'd situation. It's fine to leave them for now but they were probably invisible.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're dummied out in the English version, but home/text.asm and a couple of control character tables still mention them, so it's either this or JP_ROUTE_CHAR EQU $35.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I mean is this:

    charmap "ばん どうろ", $35

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

? charmap "ばん どうろ", $35 doesn't exist anywhere. The "ばん どうろ" is just a comment explaining what the control character was for.

@@ -56,7 +56,7 @@ LinkTextbox2: ; 4d35b

.PlaceBorder: ; 4d37e
push hl
ld a, $76
ld a, "ぁ" ; $76
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesnt seem like a border tile

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I undid that later.

@@ -139,7 +139,7 @@ _LoadTradeScreenBorder: ; 16d696

LinkComms_LoadPleaseWaitTextboxBorderGFX: ; 16d69a
ld de, LinkCommsBorderGFX + $30 tiles
ld hl, vTiles2 tile $76
ld hl, vTiles2 tile "ぁ"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesnt seem right either

roukaour and others added 2 commits January 21, 2018 21:51
.pal -> .gbcpal has been omitted for now since there's no tool to convert it yet.
@yenatch yenatch merged commit c60f133 into pret:master Jan 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants