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

Avoid runtime indirection when initializing plCreatable class indices #1449

Merged
merged 3 commits into from
Aug 7, 2023

Conversation

dgelessus
Copy link
Contributor

The initialization went through a couple of getter/setter methods (one of them virtual), but the field can easily be made a constant initialized at compile time with no dynamic lookup.

I'm actually fixing a couple of IDE warnings here: one about virtual calls in constructors (ClassIndex()) and one about "unnecessary" semicolons after the plCreator.h REGISTER_* macros.

The plCreator implementations don't have any subclasses themselves, so
the value of ClassIndex() is a known compile-time constant and the
runtime indirection through the virtual method isn't needed.
@Hoikas Hoikas requested a review from dpogue August 6, 2023 14:50
Copy link
Member

@Hoikas Hoikas left a comment

Choose a reason for hiding this comment

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

This LGTM, but I'd like for @dpogue to glance it over. He's particularly interested in external creatables, so I think he has a better picture of how all this fits together than I do.

Copy link
Member

@dpogue dpogue left a comment

Choose a reason for hiding this comment

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

This looks to me like it should be fine

@Hoikas Hoikas merged commit 132d27f into H-uru:master Aug 7, 2023
14 checks passed
@dgelessus dgelessus deleted the plcreatable_indices_static_init branch November 29, 2023 21:28
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.

3 participants