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

Add MustFirstEntity #88

Merged
merged 1 commit into from
Dec 7, 2022
Merged

Add MustFirstEntity #88

merged 1 commit into from
Dec 7, 2022

Conversation

m110
Copy link
Contributor

@m110 m110 commented Dec 3, 2022

Note: shouldn't these methods use Entry instead of Entity? Like FirstEntry? Since it returns an entry. 🤔

@yohamta yohamta merged commit 748f2d5 into yohamta:main Dec 7, 2022
@yohamta
Copy link
Owner

yohamta commented Dec 7, 2022

@m110 Thank you very much for adding the helpful function 😄 Sorry for the delay in responding to your pull request. I've been overwhelmed with notifications on GitHub and I inadvertently missed your request for a few days.

@yohamta
Copy link
Owner

yohamta commented Dec 7, 2022

Yeah, I agree. I think the name should be using Entry instead of Entity. I think we can add new methods with proper names and deprecate the old method name 🙂

On second thought, one concern is that since ECS stands for Entity, users might expect it to have a FirstEntity() method as well, so it might be a bit confusing. I think naming it just First() might be good 🤔

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