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

War for the Planet of the Miscellaneous Fixes #700

Closed
entrpntr opened this issue Mar 24, 2020 · 12 comments
Closed

War for the Planet of the Miscellaneous Fixes #700

entrpntr opened this issue Mar 24, 2020 · 12 comments

Comments

@entrpntr
Copy link
Contributor

(presumably the correct sequel to #694)

There is some variability in whether DAYCARE or DAY_CARE is used in various constant definitions. (Personally, I think DAYCARE looks better, and it is the more used of the two.)

$ grep -nr "const.*DAYCARE\|DAYCARE.*EQU" . | wc -l
      28
$ grep -nr "const.*DAY_CARE\|DAY_CARE.*EQU" . | wc -l
      15
@Rangi42
Copy link
Member

Rangi42 commented Mar 24, 2020

There is a hyphen in "DAY-CARE", but either is OK.

@Lhivorde
Copy link
Contributor

The macro menu_coords is currently defined in /macros/data.asm, but it would make more sense for it to be defined in /macros/coords.asm.

@Lhivorde
Copy link
Contributor

In the function InitScrollingMenuCursor (in /engine/menus/scrolling_menu.asm) theres a subfunction:

.asm_24763
	ret

Is there any reason not to rename it to .ret or something similar?

@mid-kid
Copy link
Member

mid-kid commented Mar 29, 2020

There's many like these. Feel free to point them out and suggest names and/or make a pull request.

@Rangi42
Copy link
Member

Rangi42 commented Mar 29, 2020

There's never a reason for .asm_XXXX labels, we just haven't fixed them all yet. python3 utils/unnamed.py pokecrystal.sym will list them all.

@Rangi42
Copy link
Member

Rangi42 commented Apr 1, 2020

In PokegearPhoneContactSubmenu, ld bc, hBGMapAddress + 1 should be bccoord -1, -2, 0.

@Rangi42
Copy link
Member

Rangi42 commented Apr 1, 2020

The add_tm macros can be simplified, and the define macro deleted:

add_tm: MACRO
if !DEF(TM01)
TM01 EQU const_value
    enum_start 1
endc
    const TM_\1
    enum \1_TMNUM
ENDM

@Rangi42
Copy link
Member

Rangi42 commented Apr 1, 2020

hSecondsBackup is written three times but never read: twice [hSecondsBackup] = [rLY] in _ResetWRAM, once [hSecondsBackup] = [hSeconds] at the end of VBlank0. Maybe rename it hUnusedBackup.

@Rangi42
Copy link
Member

Rangi42 commented Apr 4, 2020

Most coordinate-related macros take X and then Y, but dsprite is the exception:

dsprite: MACRO
; y tile, y pxl, x tile, x pxl, vtile offset, attributes
	db (\1 * 8) % $100 + \2, (\3 * 8) % $100 + \4, \5, \6
ENDM

To avoid confusion, I think it can be renamed to dbsprite and take the argument order "X tile, Y tile, X pixel, Y pixel", just like dbpixel, ldpixel, anim_obj, etc.

@mid-kid
Copy link
Member

mid-kid commented Apr 4, 2020

I think X tile/Y tile+X/Y offset is rather dumb when you're working with object positions, so if anything I'd change it to be absolute positions + 8 for X and +16 for Y.

@Rangi42
Copy link
Member

Rangi42 commented Apr 4, 2020

That would be fine too; but let's make it a separate PR from this one, with X+Y-pixel-only-arg support for dbpixel, ldpixel, dsprite, and any other such macros all at once. (anim_obj already works like that, with legacy support for the tile+pixel argument format.)

@Rangi42
Copy link
Member

Rangi42 commented Apr 4, 2020

(Eventually these would all be good candidates for a generic pxcoord function macro.)

@Rangi42 Rangi42 closed this as completed Apr 6, 2020
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

No branches or pull requests

4 participants