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

[FastPR][Core] Adding HasPrototypeEntity to EntityIdentifier #11533

Merged
merged 3 commits into from
Sep 3, 2023

Conversation

rubenzorrilla
Copy link
Member

📝 Description
On top of being handy, this is required to avoid the error thrown of the getter when using the EntityIdentifier in a context that mixes elements and conditions (see #11370 as an example).

@rubenzorrilla rubenzorrilla added Kratos Core FastPR This Pr is simple and / or has been already tested and the revision should be fast Usability labels Sep 1, 2023
@rubenzorrilla rubenzorrilla self-assigned this Sep 1, 2023
@rubenzorrilla rubenzorrilla requested a review from a team as a code owner September 1, 2023 11:10
Copy link
Member

@loumalouomega loumalouomega left a comment

Choose a reason for hiding this comment

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

I guess this is so simple we can live without test

loumalouomega
loumalouomega previously approved these changes Sep 1, 2023
Copy link
Member

@philbucher philbucher left a comment

Choose a reason for hiding this comment

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

some design comments

Also test is missing

kratos/utilities/entities_utilities.h Outdated Show resolved Hide resolved
kratos/utilities/entities_utilities.cpp Outdated Show resolved Hide resolved
@rubenzorrilla
Copy link
Member Author

some design comments

Also test is missing

I didn't found any test for this utility. I'll create a new one.

@rubenzorrilla
Copy link
Member Author

@philbucher I removed the pointer version and added a test in the last commit.

Copy link
Member

@philbucher philbucher left a comment

Choose a reason for hiding this comment

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

❤️

@rubenzorrilla rubenzorrilla merged commit 5af3577 into master Sep 3, 2023
11 checks passed
@rubenzorrilla rubenzorrilla deleted the core/entities-utilities-has-entity branch September 3, 2023 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FastPR This Pr is simple and / or has been already tested and the revision should be fast Kratos Core Usability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants