Skip to content

Remove final specifier from all class templates#4

Merged
skypjack merged 2 commits into
skypjack:masterfrom
w1th0utnam3:final_branch
May 28, 2017
Merged

Remove final specifier from all class templates#4
skypjack merged 2 commits into
skypjack:masterfrom
w1th0utnam3:final_branch

Conversation

@w1th0utnam3
Copy link
Copy Markdown
Contributor

All class templates from the ECS are defined as final. In my opinion this is not sensible. In my concrete application I want to be able to forward declare the ECS class without retyping all template parameters (forward declaration of typedefs is not possible). The easiest way to do this would be to inherit from the EnTT types:

class EntityComponentSystem : public entt::StandardRegistry<EntityType, ...> {/* stays empty */};

now I can forward declare the EntityComponentSystem class in other parts of the project.

In general I don't see an advantage of using final in this context and I would propose to remove the specifiers from the headers.

@skypjack
Copy link
Copy Markdown
Owner

I can see the reasons to remove it from the registry and actually I can optimize a bit the component pool without the final keyword. Therefore we can safely drop them out. Anyway, there is no reason to remove it from the View classes. They are final on purpose and those classes are not meant to be extended by the users. Please, re-apply it back and I can accept the PR and make a few other changes. Thank you.

@w1th0utnam3
Copy link
Copy Markdown
Contributor Author

I added final to the View classes.

I can optimize a bit the component pool without the final keyword

Can you elaborate on this? Do you mean e.g. making private members protected in order to properly subclass ComponentPool? Or are you talking about unrelated optimizations?

@skypjack skypjack merged commit f45ae14 into skypjack:master May 28, 2017
@w1th0utnam3 w1th0utnam3 deleted the final_branch branch May 28, 2017 13:53
GerHobbelt pushed a commit to GerHobbelt/entt that referenced this pull request Nov 1, 2024
GerHobbelt pushed a commit to GerHobbelt/entt that referenced this pull request Nov 1, 2024
Back out changes that are no longer needed.
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