bevyengine / bevy

A refreshingly simple data-driven game engine built in Rust
https://bevyengine.org
Apache License 2.0
36.14k stars 3.57k forks source link

`World::resource_ref` should return `Ref<T>` #11825

Closed JoJoJet closed 1 month ago

JoJoJet commented 9 months ago

What problem does this solve or what need does it fill?

World::resource_ref currently returns Res<T>, which does not make sense as Res<T> doesn't provide any functionality over Ref<T>. The only reason that Res<T> and ResMut<T> exist is that system params need to use the type to distinguish resources from other data. Otherwise, Res{Mut}<T> do not need to exist, and thus we have always treated Ref<T> and Mut<T> and as the change detection primitives. ResMut::map_unchanged and ResMut::reborrow have always returned Mut<T> to reflect this.

I disagree with returning Res<T> here, as it encourages more use of this type. We should use types like Res<T> and ResMut<T> and little as possible, so we won't need to duplicate all change detection APIs for these types.

What solution would you like?

World::resource_ref should return Ref<T>.

What alternative(s) have you considered?

None

Additional context

https://github.com/bevyengine/bevy/issues/9926 https://github.com/bevyengine/bevy/pull/11776 https://github.com/bevyengine/bevy/pull/11776 https://github.com/bevyengine/bevy/pull/9940

alice-i-cecile commented 9 months ago

I'm on board with that as a design principle.

However we should have a parallel method on Res to convert it into a Ref.

Can you close out all of the conflicting issues and PRs?

Adamkob12 commented 8 months ago

I have no problem with going in this direction. But to play devil's advocate, I'll say that maybe if the only way to get a change-detection-enabled reference to a resource in a system is through Res, it should also be the only way to do so outside of the system.

We don't have a choice on whether to use Ref or Res inside systems, but we do have a choice on which to use outside of systems. Why not keep them consistent?

Res may not have functional value over Ref, but it has semantic value.

I know when I was learning bevy I was confused on why some things were different inside of systems as opposed to outside of them (when interacting directly with the World for example)

I don't think code duplication is a major concern because we can implement Into and/or Deref etc.

additional context: https://github.com/bevyengine/bevy/issues/9926#issuecomment-1738044120

tguichaoua commented 8 months ago

The only reason that Res<T> and ResMut<T> exist is that system params need to use the type to distinguish resources from other data. Otherwise, Res{Mut}<T> do not need to exist, and thus we have always treated Ref<T> and Mut<T> and as the change detection primitives.

To reflect this idea in the code, Res and ResMut should be refactorized into thin wrappers around Ref and Mut.

struct Res<'w, T>(Ref<'w, T>);
struct ResMut<'w, T>(Mut<'w, T>);

It will also help reduce code duplication by proxying the API calls.