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

Address unintuitive macro expansion #637

Merged

Conversation

StanHash
Copy link
Member

This will eventually be necessary because of the version of ColorzCore I am working on (see FireEmblemUniverse/ColorzCore#63), which hopefully will make it as an "official" release, will break this.

Explanation

#define MaxPlayerUnits 62+1
// ...
#define EUDebuffTable "PUDebuffTable + (MaxPlayerUnits*DebuffEntrySize)"
// ...
WORD EUDebuffTable

Because of a quirk in the current version of ColorzCore, the body of the object-like macro "MaxPlayerUnits" is evaluated early as a number, and results in the single token "0x3F" (63) at definition time.

This results eventually in the following statement when expanded:

WORD PUDebuffTable + (0x3F*DebuffEntrySize)

(keeping macro names for the parts that aren't relevant for clarity)

Now, I made a change to the way '#define' works that allows it to take any sequence of tokens as macro body, not just an expression or a string. I believe it's an improvement but is means that the body of "MaxPlayerUnits" is cannot be evaluated early anymore, and keeps being the sequence ["62", "+", "1"].

Eventually, when expanded:

WORD PUDebuffTable + (62+1*DebuffEntrySize)

This is not the same as above because of operator precedence.

Notes:

  • this would probably also be broken in legacy NL!Core.
  • this would also be broken in the C preprocessor.
  • this would also be broken if you put quotes around the definitions (#define MaxPlayerUnits "62+1" breaks)

(I might have to add some hacks in ColorzCore to at least warn users of such cases when they upgrade).

The fix

Put parenthesizes around the expression in the definition. I also added quotes for good measure.

@sme23 sme23 merged commit 820a686 into FireEmblemUniverse:master Apr 27, 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

2 participants