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

[Chore] Regenerate asset headers with include guards #3132

Merged
merged 1 commit into from
Sep 1, 2023

Conversation

Archez
Copy link
Contributor

@Archez Archez commented Aug 20, 2023

I noticed after one of the ZAPD upstream pulls, we never re-ran the gen headers command, so I've done that, but noticed there was an error in the new include guards where some header files had the same guard name. I've tweaked the include guard to now also prefix the parent folder name to increase the uniqueness of the include guards made.

I also removed the unneeded #pragma once since only the include guard is necessary now.

Ran with non-mq and mq together (mq is used last)

I'm submitting this now, as I have plans to update our xmls/headers, so I want to get this initial noise out of the way first.

Build Artifacts

@garrettjoecox
Copy link
Contributor

I also removed the unneeded #pragma once since only the include guard is necessary now.

Forgive my ignorance, but don't these do the same thing? From my initial search it seems like #pragma once isn't as widely supported, but was that really an issue for our compilation setup? Seems like it's been fine thus far, and cleaner than the wrapped #ifdef method

@Archez
Copy link
Contributor Author

Archez commented Aug 20, 2023

I also removed the unneeded #pragma once since only the include guard is necessary now.

Forgive my ignorance, but don't these do the same thing? From my initial search it seems like #pragma once isn't as widely supported, but was that really an issue for our compilation setup? Seems like it's been fine thus far, and cleaner than the wrapped #ifdef method

Technically #pragma once is non-standard. Although the big compilers support it (gcc, msvc), include guards are "more proper".

ZAPD upstream made the switch but upstream they already removed the pragma line, but in SoH we had both.

so this PR is doing two things:

  • Finishing up the switch over to include guards
  • Fixing a collision with the include guards

I guess Ill leave it to @louist103 or @NEstelami to add further comments on "why" the switch was made in ZAPD.

@garrettjoecox garrettjoecox merged commit dd37d4f into HarbourMasters:develop Sep 1, 2023
8 checks passed
@Archez Archez deleted the refresh-asset-headers branch September 1, 2023 18:23
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