Skip to content

Cleaner macros utilizing syn::Member v2 #21678

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 17 commits into from
Oct 30, 2025
Merged

Cleaner macros utilizing syn::Member v2 #21678

merged 17 commits into from
Oct 30, 2025

Conversation

@Bleachfuel
Copy link
Contributor

Objective

Some macros were handling Idents and indexes as seperate things, we can use syn::Member to make this more readable and nicer.

revive of #18199

@github-actions
Copy link
Contributor

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-21678

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

1 similar comment
@github-actions
Copy link
Contributor

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-21678

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

@github-actions
Copy link
Contributor

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-21678

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

@github-actions
Copy link
Contributor

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-21678

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

@Bleachfuel
Copy link
Contributor Author

no idea whats causing the changes in graphical output

Trashtalk217
Trashtalk217 approved these changes Oct 29, 2025
View reviewed changes
Copy link
Contributor

@Trashtalk217 Trashtalk217 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good improvement. I do wonder if shape.rs (in macro_utils) could maybe be renamed / be rehomed to a different file, because there's very little reason for it to exist.

@alice-i-cecile alice-i-cecile added C-Code-Quality A section of code that is hard to understand or change A-Cross-Cutting Impacts the entire engine S-Needs-Review Needs reviewer attention (from anyone!) to move forward D-Macros Code that generates Rust code 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 Oct 29, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Oct 29, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Oct 29, 2025
@alice-i-cecile
Copy link
Member

@Bleachfuel ping me when CI is green and I'll get this in (finally 😅). Sorry for the headaches!

@Bleachfuel
Copy link
Contributor Author

@alice-i-cecile I think i fixed everything 😅 As a follow up i want to find a better alternative to the members and fields

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Oct 30, 2025
Merged via the queue into bevyengine:main with commit 5a6a06a Oct 30, 2025
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

+1 more reviewer

@Trashtalk217 Trashtalk217 Trashtalk217 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A-Cross-Cutting Impacts the entire engine C-Code-Quality A section of code that is hard to understand or change D-Macros Code that generates Rust code 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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Bleachfuel @alice-i-cecile @Trashtalk217