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

Identify more bit flags #120

Merged
merged 1 commit into from
Nov 28, 2024
Merged

Identify more bit flags #120

merged 1 commit into from
Nov 28, 2024

Conversation

Rangi42
Copy link
Member

@Rangi42 Rangi42 commented Oct 9, 2024

Pretty self-explanatory; a similar effort to some of the recent pokered commits.

Some things to note:

  • I did this in pokegold, not pokecrystal, so the unidentified mobile stuff wouldn't be a distraction. This will need porting to there once it's approved.
  • There are many bit/res/set 7s left, because of a convention to set the high bit of jumptable indexes to mean "exit the jumptable". I may add a generic EXIT_JUMPTABLE_F constant in a separate PR.
  • I edited the shift_const macro to automatically define \1 (bit mask) and \1_F (0-7 bit flag) constants. This does make it harder to grep for NAME_F definitions, but otherwise we'd have a lot of redundant definition lines.
  • We don't have a perfect convention of naming flag constants with _F or not. I usually went for local consistency, i.e. imitate nearby constant names.
  • A few of these are surprising/weird, like how GetMovementPermissions in home/map.asm sets the RIGHT bit in all four directional cases. I may have made mistakes there, but AFAICT that's what the bit means.

constants/ram_constants.asm Outdated Show resolved Hide resolved
constants/ram_constants.asm Outdated Show resolved Hide resolved
constants/ram_constants.asm Outdated Show resolved Hide resolved
home/map.asm Show resolved Hide resolved
engine/overworld/player_movement.asm Outdated Show resolved Hide resolved
ram/wram.asm Show resolved Hide resolved
constants/ram_constants.asm Outdated Show resolved Hide resolved
Copy link
Collaborator

@vulcandth vulcandth left a comment

Choose a reason for hiding this comment

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

Appreciate the ton of work you did here. Sorry i've been AWOL lol. I promise I'll try to make some time this weekend at the latest to properly review the remaining items and approve this so we can get this merged. Thank you! :)

constants/ram_constants.asm Outdated Show resolved Hide resolved
Copy link
Collaborator

@vulcandth vulcandth left a comment

Choose a reason for hiding this comment

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

After these small changes I think we are good to merge! Thank you!

constants/ram_constants.asm Outdated Show resolved Hide resolved
constants/ram_constants.asm Outdated Show resolved Hide resolved
home/map.asm Show resolved Hide resolved
constants/menu_constants.asm Outdated Show resolved Hide resolved
constants/printer_constants.asm Outdated Show resolved Hide resolved
constants/ram_constants.asm Outdated Show resolved Hide resolved
constants/ram_constants.asm Outdated Show resolved Hide resolved
constants/ram_constants.asm Outdated Show resolved Hide resolved
@Rangi42 Rangi42 merged commit 7ebc432 into pret:master Nov 28, 2024
1 check passed
@Rangi42 Rangi42 deleted the bits branch November 28, 2024 19:50
github-actions bot pushed a commit that referenced this pull request Nov 28, 2024
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.

2 participants