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 support for type checked term_at<T> and term_at<T>(i) #1397

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

deathbeam
Copy link
Contributor

@deathbeam deathbeam commented Oct 13, 2024

First method is simple loop over terms to find term for T. Its slower than using index directly, but sometimes type safety is more preferred and when the cost is only on initialization.

Second method is same as term_at(i) but with type checked assert. For middle ground to still keep the safety but also performance.

This is mostly to mirror the C# binding PR where this is slightly more useful but maybe cpp would benefit too:

BeanCheeseBurrito/Flecs.NET#52

Also disclaimer, I dont rly work with cpp often so I might've easily missed something, but I made best effort to write some tests and at least run them and see if they pass :d

Example use:

Current method

world.System<Position, Velocity>()
  .term_at(1)
  .singleton();

Type checked (throws assert if term_at(1) isnt Velocity type)

world.System<Position, Velocity>()
  .term_at<Velocity>(1)
  .singleton();

Type checked + resolved (throws assert if there is no Velocity component):

world.System<Position, Velocity>()
  .term_at<Velocity>()
  .singleton();

@deathbeam deathbeam force-pushed the type-checked-term-at branch 2 times, most recently from 5a80df7 to ac73c71 Compare October 13, 2024 05:44
@copygirl
Copy link
Contributor

Would it make sense to have the parameter-less term_at named differently?

@deathbeam
Copy link
Contributor Author

Maybe just .term<>() and then also add .term(flecs::entity_t component) to match other apis?

@deathbeam deathbeam changed the title Add support for type checked term_at<T> and term_at<T>(i) Add support for type checked term<T> and term_at<T>(i) Oct 14, 2024
@deathbeam
Copy link
Contributor Author

Renamed term_at to term. Also updated the logic to account for pairs properly

@deathbeam deathbeam force-pushed the type-checked-term-at branch 3 times, most recently from 72e7ea1 to fe2c902 Compare October 14, 2024 11:58
@deathbeam
Copy link
Contributor Author

Looks like I cant use term as it fails on build here so changing back to term_at I guess:

https://github.com/SanderMertens/flecs/actions/runs/11327040474/job/31497292704?pr=1397

@deathbeam deathbeam changed the title Add support for type checked term<T> and term_at<T>(i) Add support for type checked term_at<T> and term_at<T>(i) Oct 14, 2024
First method is simple loop over terms to find term for T. Its slower
than using index directly, but sometimes type safety is more preferred
and when the cost is only on initialization.

Second method is same as term_at(i) but with type checked assert. For
middle ground to still keep the safety but also performance.

This is mostly to mirror the C# binding PR where this is slightly more
useful but maybe cpp would benefit too:

BeanCheeseBurrito/Flecs.NET#52

Signed-off-by: Tomas Slusny <[email protected]>
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