[Merged by Bors] - Refactor ResMut/Mut/ReflectMut to remove duplicated code

[NathanSWard]

ResMut, Mut and ReflectMut all share very similar code for change detection.
This PR is a first pass at refactoring these implementation and removing a lot of the duplicated code.

Note, this introduces a new trait ChangeDetectable.

Please feel free to comment away and let me know what you think!

[NathanSWard]

Note: I can already tell the I'll have to change the get get_mut names since they clash with other function names.

[NathanSWard]

I would appreciate if anyone had ideas for better names for
underlying() underlying_mut() and underlying_mut_unchecked() 😅

[bjorn3]

UnsafeCell uses get as name for getting access to the interior value just like you used. Another option would be inner.

[cart]

I think get() is probably too common a name for this context. I would expect that to conflict with the pointer type's methods on a regular basis. Getting the underlying value using the underlying function should only really happen when the Deref lifetimes are overly strict. I think long uncommon names (like underlying()) are actually a feature here because they discourage regular use.

[NathanSWard]

I think long uncommon names (like underlying()) are actually a feature here because they discourage regular use.

Yep, that's the same thinking I had for chosing underlying.
However, I would ideally like the name to pertain to the trait ChangeDetectable (e.g. incorporate, 'change' 'detect' or something like that in the name.)
But ultimately I'm ok with underlying as well.

[alice-i-cecile]

This feels likely to conflict with @BoxyUwU's relation PR (#1627). Would you be willing to help resolve merge conflicts with that @NathanSWard? It is likely to make implementing change detection for relations nicer though, so I personally think we should try and merge this in :)

[alice-i-cecile]

I left some documentation nits, but once those are addressed this LGTM.

[NathanSWard]

Sorry to ping you directly @cart but I would appreciate if you took a look at this :)
Specifically, I might have a use case for getting mutable access to a ResMut without triggering change detection, but I'd only want to build that functionality on top of this PR.

[cart]

@BoxyUwU how disruptive is this to relations? It seems relatively scoped to me. @NathanSWard: we've been discussing putting ecs work in "slow mode" to make merging the massive relations pr easier / cut down on boxy constantly needing to merge other people's work. Would you be willing to do the merge on boxy's relations branch to help out?

Also, I'm now thinking DetectChanges is probably a more idiomatic name? Ex we call the drop trait Drop not Dropable. (sorry for the back and forth on this).

[BoxyUwU]

looks like it would be pretty simple to rebase relations on top of, shouldn't hurt to merge :)

[NathanSWard]

Would you be willing to do the merge on boxy's relations branch to help out?

Yep of course :)

Also, I'm now thinking DetectChanges is probably a more idiomatic name? Ex we call the drop trait Drop not Dropable. (sorry for the back and forth on this).

Oh, I think I like that name more as well :)
And no worries for the back and forth haha :p

[cart]

bors r+

[bors]

Pull request successfully merged into main.

Build succeeded:

• build-android
• build (nightly, ubuntu-latest)
• build (stable, macos-latest)
• build (stable, ubuntu-latest)
• build (stable, windows-latest)
• build-wasm (nightly, ubuntu-latest)
• build-wasm (stable, ubuntu-latest)
• check-doc
• check-markdown-links
• check-missing-examples-in-docs
• markdownlint
• run-examples