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

Restructure event flags in z64save.h (1/?) #2380

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

mzxrules
Copy link
Contributor

Contributions made in this pr are licensed under CC0

See #2303 for what the final changes may look like. The end goal is to ultimately create create flagIds for all flags, allowing you to extract w/e info using the flagId macros.

This pr implements GET_*_MASK macros, and converts most flags to use those instead. This macro is necessary to break up #2303 into multiple prs in a clean way.

#define SET_EVENTCHKINF(flag) (gSaveContext.save.info.eventChkInf[(flag) >> 4] |= (1 << ((flag) & 0xF)))
#define CLEAR_EVENTCHKINF(flag) (gSaveContext.save.info.eventChkInf[(flag) >> 4] &= ~(1 << ((flag) & 0xF)))

#define GET_EVENTCHKINF_MASK(flag) (1 << ((flag) & 0xF))
Copy link
Contributor

Choose a reason for hiding this comment

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

this could be used in the definitions of GET_EVENTCHKINF etc above. It might be nice to also give a name to (flag) >> 4 since this is used a couple times below too. How something like

#define EVENTCHKINF_INDEX(flag) ((flag) >> 4)
#define EVENTCHKINF_MASK(flag) (1 << ((flag) & 0xF))

#define GET_EVENTCHKINF(flag) (gSaveContext.save.info.eventChkInf[EVENTCHKINF_INDEX(flag)] & EVENTCHKINF_MASK(flag))
#define SET_EVENTCHKINF(flag) (gSaveContext.save.info.eventChkInf[EVENTCHKINF_INDEX(flag)] |= EVENTCHKINF_MASK(flag))
#define CLEAR_EVENTCHKINF(flag) (gSaveContext.save.info.eventChkInf[EVENTCHKINF_INDEX(flag)] &= ~EVENTCHKINF_MASK(flag))

I don't think GET_ is necessary, especially since it could be interpreted as getting the flag value itself.

@fig02
Copy link
Collaborator

fig02 commented Dec 19, 2024

I think splitting this into parts is a great idea, thanks for doing that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants