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 various flag labels and bit constants #454

Merged
merged 1 commit into from
Jul 16, 2024
Merged

Conversation

Rangi42
Copy link
Member

@Rangi42 Rangi42 commented Jun 22, 2024

After this, only four labels will be unidentified:

  • wcd6d (110 uses)
  • wcf91 (171 uses)
  • wd0b5 (56 uses)
  • wd11e (163 uses)

Of course, there are still many non-label constants left to be identified. In particular, flag bits and meaningful RAM values that should be going in the new ram_constants.asm file.

@Rangi42 Rangi42 requested review from dannye and vulcandth June 22, 2024 21:30
@Rangi42
Copy link
Member Author

Rangi42 commented Jun 22, 2024

Some notes on naming choices:

  • wMiscFlags is distant from all the wStatusFlags* locations, so it should have its own name.
  • wStatusFlags5 corresponds to pokecrystal's wStateFlags, but after discussion with @dannye , I'm leaving it as one of the wStatusFlags* since there are so many more in gen 1 than gen 2.
  • Arguably some of the wStatusFlags* labels could be more specific, e.g. wRodFlags for wStatusFlags since three of its bits are for getting the Rods, or wWarpFlags for wStatusFlags5 since four of its bits are warp-related, but overall they're just too generic. (I also considered using more generic names like wMiscFlags, wEngineFlags, wGameFlags, wModeFlags, etc, but assigning them to particular bytes felt really arbitrary.)
  • On the other hand, I called it wMovementFlags instead of wStatusFlags8 because engine/battle/wild_encounters.asm checks the whole value at once with and a, implying that all its flags should be related somehow.
  • wUnusedFlag is a generic name; unlike wUnusedLinkMenuByte, wUnusedCreditsByte, wUnusedAlreadyOwnedFlag, etc, there's no connection between its two usages (clearing it on the title screen and when trading Pokemon).

@Rangi42 Rangi42 mentioned this pull request Jun 24, 2024
29 tasks
@Rangi42 Rangi42 force-pushed the flags branch 3 times, most recently from f7fa11f to c62b399 Compare June 24, 2024 14:05
@Rangi42
Copy link
Member Author

Rangi42 commented Jun 24, 2024

@dannye @vulcandth I think all that really needs reviewing are the constants and wram files; the rest are just many uses of those new constants+labels.

@Rangi42 Rangi42 marked this pull request as draft June 26, 2024 02:33
@Rangi42 Rangi42 marked this pull request as ready for review June 26, 2024 12:15
@Rangi42
Copy link
Member Author

Rangi42 commented Jun 26, 2024

@dannye Okay, this is ready. I'm open to changing some of the wStatusFlags* names but don't really see better non-generic options.

Copy link
Member

@dannye dannye left a comment

Choose a reason for hiding this comment

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

Awesome.

engine/overworld/trainer_sight.asm Outdated Show resolved Hide resolved
ram/wram.asm Outdated Show resolved Hide resolved
ram/wram.asm Outdated Show resolved Hide resolved
ram/wram.asm Show resolved Hide resolved
@Rangi42
Copy link
Member Author

Rangi42 commented Jul 15, 2024

@dannye Thank you for your review comments! They should all now be addressed.

Copy link
Member

@dannye dannye left a comment

Choose a reason for hiding this comment

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

LGTM. This is pretty great, thanks for doing it.

@Rangi42 Rangi42 merged commit 8fafca7 into pret:master Jul 16, 2024
1 check passed
@Rangi42 Rangi42 deleted the flags branch July 16, 2024 17:02
SwimmingLink added a commit to SwimmingLink/SwimmingLinksPokeRed that referenced this pull request Jul 16, 2024
Identify various flag labels and bit constants (pret#454)
kqesar pushed a commit to kqesar/pokered that referenced this pull request Jul 17, 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.

None yet

4 participants