Skip to content

Conversation

@nicopap
Copy link
Contributor

@nicopap nicopap commented Jun 19, 2023

Objective

ParsedPath does not need to be mut to access a field of a Reflect. Be that access mutable or not. Yet element_mut requires a mutable borrow on self.

Solution

  • Make element_mut take a &self over a &mut self.

#8887 fixes this, but this is a major limitation in the API and I'd rather see it merged before 0.11.


Changelog

  • ParsedPath::element_mut and ParsedPath::reflect_element_mut now accept a non-mutable ParsedPath (only the accessed Reflect needs to be mutable)

@nicopap nicopap added C-Bug An unexpected or incorrect behavior A-Reflection Runtime information about types labels Jun 19, 2023
@nicopap nicopap added this to the 0.11 milestone Jun 19, 2023
@mockersf mockersf added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jun 19, 2023
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jun 19, 2023
Merged via the queue into bevyengine:main with commit 23863d5 Jun 19, 2023
james7132 pushed a commit to james7132/bevy that referenced this pull request Jun 19, 2023
# Objective

`ParsedPath` does not need to be mut to access a field of a `Reflect`.
Be that access mutable or not. Yet `element_mut` requires a mutable
borrow on `self`.

## Solution

- Make `element_mut` take a `&self` over a `&mut self`.

bevyengine#8887 fixes this, but this is a major limitation in the API and I'd
rather see it merged before 0.11.

---

## Changelog

- `ParsedPath::element_mut` and `ParsedPath::reflect_element_mut` now
accept a non-mutable `ParsedPath` (only the accessed `Reflect` needs to
be mutable)
@nicopap nicopap deleted the non-mut-path branch August 30, 2023 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Reflection Runtime information about types C-Bug An unexpected or incorrect behavior S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants