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

Generate enum-based battle constants at build-time #121

Merged
merged 2 commits into from
Jan 9, 2024

Conversation

lhearachel
Copy link
Collaborator

This makes use of a subproject that I wrote for generating constants files from a basic manifest schema. The goal here is to support having a single source of truth for such definitions that can be utilized by both the compiler and a direct assembler invocation for building data files (which we will need for battle scripts). The assembler does not support reading enum definitions out-of-the-box, so these definitions would need to either be:

  1. Duplicated, and thus separately maintained
  2. Moved to #define syntax, which is less-than-ideal for self-documentation of function signatures
  3. Generated from a separate source-of-truth

Comment on lines -106 to -128
enum {
OVERWORLD_WEATHER_CLEAR = 0,
OVERWORLD_WEATHER_CLOUDY,
OVERWORLD_WEATHER_RAINING,
OVERWORLD_WEATHER_HEAVY_RAIN,
OVERWORLD_WEATHER_THUNDERSTORM,
OVERWORLD_WEATHER_SNOWING,
OVERWORLD_WEATHER_HEAVY_SNOW,
OVERWORLD_WEATHER_BLIZZARD,
OVERWORLD_WEATHER_CLEAR_8,
OVERWORLD_WEATHER_SLOW_ASHFALL,
OVERWORLD_WEATHER_SANDSTORM,
OVERWORLD_WEATHER_HAILING,
OVERWORLD_WEATHER_SPIRITS,
OVERWORLD_WEATHER_CLEAR_13,
OVERWORLD_WEATHER_FOG,
OVERWORLD_WEATHER_DEEP_FOG,
OVERWORLD_WEATHER_DARK_FLASH,

// these are only for the Battle Frontier
OVERWORLD_WEATHER_HARSH_SUN = 1001,
OVERWORLD_WEATHER_TRICK_ROOM,
};
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I moved these to #define syntax, since there is a break in values outside the usual step-increments.

@@ -18,3 +18,9 @@ if not nitrogfx_exe.found()
subproject('nitrogfx', default_options: 'native=true')
nitrogfx_exe = find_program('nitrogfx', native: true)
endif

constgen_py = find_program('constgen_py', native: true, required: false)
Copy link
Member

Choose a reason for hiding this comment

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

Can't you use the fallback: parameter? Granted, that's a new meson feature I think.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This isn't really a fallback candidate, since it's not a different name. We're expecting to find it under this name.

@mid-kid
Copy link
Member

mid-kid commented Jan 7, 2024

My only complaint is that nobody is gonna be able to find the source of truth through neither their IDE nor a simple grep. This is because you took away their prefixes in the definitions files, which I think is a questionable feature.

Either:

  • Write the constant names out in full without transforming them. Yes it's more verbose, but it's also easier to deal with.
  • Output a comment at the top of the header telling the reader that it's a generated file, what program it was generated with, what file the constants originate from, and if relevant the command line arguments passed to the program.

Or both, preferably both.

@lhearachel
Copy link
Collaborator Author

Or both, preferably both.

I decided both was worthwhile, too.

Copy link
Member

@mid-kid mid-kid left a comment

Choose a reason for hiding this comment

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

Looks good!

@lhearachel lhearachel merged commit 1bacaf3 into pret:main Jan 9, 2024
1 check passed
@lhearachel lhearachel deleted the constgen branch February 21, 2024 02:14
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