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

Manual SystemState + SystemMeta ref methods #12745

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ecoskey
Copy link

@ecoskey ecoskey commented Mar 27, 2024

  • expose API for manually initializing system param states (dynamic queries)
  • no existing code should be affected
  • SystemMeta methods for exposing reference fields (custom System impls)

Objective

Currently SystemMeta's private fields and the extensive unsafe code for dealing with SystemParams make custom system implementations unwieldy. One particular area this pops up is when trying to use dynamic queries with normal systems.

Solution

Allow users to specify parameters in SystemState that require explicit state initialization, and expose some readonly SystemMeta fields.


Changelog

  • Add additional type parameter Extra to SystemState

  • add new_with_extra(), get_with_extra(), get_mut_with_extra(), get_unchecked_with_extra(), get_unchecked_mut_with_extra(), and fetch_with_extra() to SystemState (names verbose to not conflict with existing code)

  • add component_access(&self), archetype_component_access(&self), and raw_name(&self) to SystemMeta

- expose API for manually initializing system param states (dynamic queries)
- no existing code should be affected
- SystemMeta methods for exposing reference fields (custom System impls)
@ecoskey
Copy link
Author

ecoskey commented Mar 27, 2024

Also this issue is blocking my renderer abstraction stuff so it would be cool to merge this :)

@ItsDoot
Copy link
Contributor

ItsDoot commented Mar 27, 2024

Interested in an example usage.

@ecoskey
Copy link
Author

ecoskey commented Mar 27, 2024

For some motivation:

Dynamic queries (and possibly other future {Param}Builders) don't make sense for normal function systems (possibly without some new API), since you have to manually initialize the system state, which intuitively requires a struct. One example of this usecase might be a new system wrapper, which could give access to a single dynamic query param:

struct WithDynamicQuery<I, O, Q, P, F: FnMut(I, Query<Q>, SystemParamItem<P>) -> O> {
  query_state: QueryState<Q>,
  f: F,
}

struct IsDynamicQuerySystem;

struct DynamicQuerySystemI, O, Q, P, F: FnMut(I, Query<Q>, SystemParamItem<P>) -> O> {
  state: SystemState<P, Query<Q>>,
  f: F
}

impl IntoSystem for WithDynamicQuery<...> { ... }
impl System for DynamicQuerySystem<...> { ... }

Though this is the only example I can think of where this makes sense (besides the renderer stuff I'm working on), I think exposing this functionality in the most low-level way possible while maintaining safety is worth it.

@mnmaita mnmaita added C-Enhancement A new feature A-ECS Entities, components, systems, and events labels Mar 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Enhancement A new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants