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

Remove ArrayDef macro and cleanup of Make_Global #683

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

duncanspumpkin
Copy link
Contributor

I used a regex to find/replace the ArrayDefs with the equivalent Make_Global code.
I also noticed that two users of Make_Global were from a very old version where a macro was used for the identifier name. I've brought those into normal alignment.
One item of note whilst doing this is that we have a handful of Make_Globals outside of the setupglobals file.
Unsure if they should all be moved into setupglobals.

Copy link
Contributor

@xezon xezon left a comment

Choose a reason for hiding this comment

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

Hmm. To be fair it actually looks nicer to look at with the macros. The duplication of types and array sizes is quite a strain when typing it out by hand. Maybe we can just leave it as is.

src/w3d/lib/systimer.h Show resolved Hide resolved
@duncanspumpkin
Copy link
Contributor Author

Hmm. To be fair it actually looks nicer to look at with the macros. The duplication of types and array sizes is quite a strain when typing it out by hand. Maybe we can just leave it as is.

For the make globals you can get rid of the left hand side and replace with auto& to reduce the duplication. I'm not overly fussed either way it is done this doesn't exactly represent much work

@xezon
Copy link
Contributor

xezon commented Mar 11, 2022

Yes auto could be ok here, because the type is already written on the right side.

auto &g_waterSettings = Make_Global<WaterSetting[TIME_OF_DAY_COUNT]>(PICK_ADDRESS(0x00A2F0B8, 0x00A2F0B8));

Will this compile?

@duncanspumpkin
Copy link
Contributor Author

Yes auto could be ok here, because the type is already written on the right side.

auto &g_waterSettings = Make_Global<WaterSetting[TIME_OF_DAY_COUNT]>(PICK_ADDRESS(0x00A2F0B8, 0x00A2F0B8));

Will this compile?

Yes that should compile.

@duncanspumpkin
Copy link
Contributor Author

After further testing I can confirm it would compile but only if we didn't forward declare so basically we can't do it that way.

@OmniBlade
Copy link
Contributor

@duncanspumpkin Looks like CI checks are broken, once its passing the CI I'll merge this.

@duncanspumpkin
Copy link
Contributor Author

Even with how much of a mouthful the definition ends up being?

@OmniBlade
Copy link
Contributor

We could keep definitions macros just to keep some of the duplicate type and size declarations to a minimum? At least we don't need the helper structs anymore.

@duncanspumpkin
Copy link
Contributor Author

We could keep definitions macros just to keep some of the duplicate type and size declarations to a minimum? At least we don't need the helper structs anymore.

The helper struct was removed in the other PR this was just removing the macros. Tbh I'm not sure if there is much merrit in this PR it ends up being so verbose its annoying that auto can't come to our rescue for this.

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.

None yet

3 participants