Skip to content

Add OptionalSystemParam and OptionalSystemParamFetch trait#6854

Closed
JonahPlusPlus wants to merge 18 commits intobevyengine:mainfrom
JonahPlusPlus:optional_system_param
Closed

Add OptionalSystemParam and OptionalSystemParamFetch trait#6854
JonahPlusPlus wants to merge 18 commits intobevyengine:mainfrom
JonahPlusPlus:optional_system_param

Conversation

@JonahPlusPlus
Copy link
Contributor

@JonahPlusPlus JonahPlusPlus commented Dec 5, 2022

Objective

  • There is no way to implement SystemParam for Option<T>, due to orphan rules.

Solution

  • Instead of trying to implement SystemParam for Option<T>, users can implement OptionalSystemParam for T and OptionalSystemParamFetch for TState.
  • SystemParam is implemented for all Option<T: OptionalSystemParam.

Changelog

This section is optional. If this was a trivial fix, or has no externally-visible impact, you can delete this section.

  • Added OptionalSystemParam and OptionalSystemParamFetch.
  • Changed internal Option<T> system params to use OptionalSystemParam and OptionalSystemParamFetch.

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Dec 5, 2022
@alice-i-cecile alice-i-cecile self-requested a review December 5, 2022 04:25
@JonahPlusPlus
Copy link
Contributor Author

@james7132 I have a solution for your concern. You can check out the PR on my fork here.

Basically, the idea is to implement OptionalSystemParam for your system param. Then, instead of implementing SystemParamFetch for your state, implement OptionalSystemParamFetch.

Implementing OptionalSystemParam and OptionalSystemParamFetch for T and TState will automatically implement SystemParam for T and Option<T>. This works by automatically implementing SystemParamFetch for TState and Option<TState>, which means that you can create optional system params that use resources by using Option<ResState<T>>.

If this is acceptable to you, I'll merge it to this PR.

@JonahPlusPlus JonahPlusPlus changed the title Add OptionalSystemParam trait Add OptionalSystemParam and OptionalSystemParamFetch trait Dec 6, 2022
@JonahPlusPlus
Copy link
Contributor Author

@alice-i-cecile I'm closing this PR because I want to redo it once #6865 is merged (if you could review and merge it that would be appreciated). With the simplified trait structure, it will be easier to add OptionalSystemParam and ResultableSystemParam at once.

ResultableSystemParam is an idea @JoJoJet and I discussed briefly on discord. Basically, by implementing it you can use Result<T, E> as a system param. It also creates a sort of fallback structure:

  • Implementing ResultableSystemParam implements SystemParam for Result<T, E>, Option<T> and T
  • Implementing OptionalSystemParam implements SystemParam for Option<T> and T
  • Implementing SystemParam just provides T

This gives users precise control over error handling.

@JonahPlusPlus JonahPlusPlus deleted the optional_system_param branch December 16, 2022 05:29
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-Usability A targeted quality-of-life change that makes Bevy easier to use

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants