Skip to content

bevy_reflect: Fix TypePath string concatenation #18609

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

Merged

Conversation

MrGVSV
Copy link
Member

@MrGVSV MrGVSV commented Mar 29, 2025

Objective

Fixes #18606

When a type implements Add for String, the compiler can get confused when attempting to add a &String to a String.

Unfortunately, this seems to be expected behavior which causes problems for generic types since the current TypePath derive generates code that appends strings in this manner.

Solution

Explicitly use the Add<&str> implementation in the TypePath derive macro.

Testing

You can test locally by running:

cargo check -p bevy_reflect --tests

@MrGVSV MrGVSV added C-Bug An unexpected or incorrect behavior D-Trivial Nice and easy! A great choice to get started with Bevy A-Reflection Runtime information about types S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Mar 29, 2025
@MrGVSV MrGVSV moved this from Open to In Progress in Reflection Mar 29, 2025
@alice-i-cecile alice-i-cecile added this to the 0.16 milestone Mar 29, 2025
@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Mar 29, 2025
@MrGVSV MrGVSV force-pushed the mrgvsv/reflect/fix-type-path-string-appending branch from 8dc720e to 2725fb2 Compare March 29, 2025 19:32
@mockersf mockersf added this pull request to the merge queue Mar 29, 2025
Merged via the queue into bevyengine:main with commit f471603 Mar 29, 2025
32 checks passed
@MrGVSV MrGVSV deleted the mrgvsv/reflect/fix-type-path-string-appending branch March 30, 2025 04:04
mockersf pushed a commit that referenced this pull request Mar 30, 2025
# Objective

Fixes #18606

When a type implements `Add` for `String`, the compiler can get confused
when attempting to add a `&String` to a `String`.

Unfortunately, this seems to be [expected
behavior](rust-lang/rust#77143 (comment))
which causes problems for generic types since the current `TypePath`
derive generates code that appends strings in this manner.

## Solution

Explicitly use the `Add<&str>` implementation in the `TypePath` derive
macro.

## Testing

You can test locally by running:

```
cargo check -p bevy_reflect --tests
```
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 D-Trivial Nice and easy! A great choice to get started with Bevy S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

Having more than two types param in a type and deriving Reflect errors
3 participants