Skip to content

Make #[derive(Bundle)] work on tuple structs #2499

Closed
Veykril wants to merge 5 commits intobevyengine:mainfrom
Veykril:ecsprocmac
Closed

Make #[derive(Bundle)] work on tuple structs #2499
Veykril wants to merge 5 commits intobevyengine:mainfrom
Veykril:ecsprocmac

Conversation

@Veykril
Copy link
Contributor

@Veykril Veykril commented Jul 18, 2021 •
edited
Loading

Objective

  • This change allows one to derive the Bundle trait on all kinds of structs that is Record, Tuple and Unit(not that useful by itself, but for completioness sake) structs instead of just record structs.

@github-actions github-actions bot added S-Needs-Triage This issue needs to be labelled S-Pre-Relicense This PR was made before Bevy added the Apache license. Cannot be merged or used for other work labels Jul 18, 2021
@mockersf mockersf added A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use and removed S-Needs-Triage This issue needs to be labelled S-Pre-Relicense This PR was made before Bevy added the Apache license. Cannot be merged or used for other work labels Jul 18, 2021
alice-i-cecile
alice-i-cecile reviewed Jul 22, 2021
View reviewed changes
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

I like this! Nice bit of consistency. Could you add a quick test or doc test to statically verify that #[derive(Bundle)] works on the various struct types so we don't break it by accident in the future?

Otherwise LGTM.

@cart cart added the S-Pre-Relicense This PR was made before Bevy added the Apache license. Cannot be merged or used for other work label Jul 23, 2021
@mockersf
Copy link
Member

mockersf commented Jul 24, 2021

Do we really want to allow unit types be bundles? This would almost certainly be a user error to do that

@mockersf mockersf removed the S-Pre-Relicense This PR was made before Bevy added the Apache license. Cannot be merged or used for other work label Jul 24, 2021
@Veykril
Copy link
Contributor Author

Veykril commented Jul 24, 2021

Prior to this PR empty records are already allowed which would also be user errors to derive Bundle on.

I suppose it would make sense to emit a compile error for structs with no fields in general?

@alice-i-cecile
Copy link
Member

alice-i-cecile commented Jul 24, 2021

I suppose it would make sense to emit a compile error for structs with no fields in general?

I agree with this. If we find some obscure use case where this makes sense we can add this functionality back in :)

@Veykril Veykril closed this Jan 3, 2022
@alice-i-cecile
Copy link
Member

alice-i-cecile commented Jan 3, 2022

Sorry for leaving this in limbo for far too long :(

@alice-i-cecile alice-i-cecile mentioned this pull request Jan 3, 2022
Closed
@Veykril
Copy link
Contributor Author

Veykril commented Jan 3, 2022

No worries, I am just not interested in updating the PR any longer. And truthfully spoken, given the huge backlog of PRs on the project I wouldn't expect it to get merged until its outdated again.

If someone wants to update it be my guest though 👍

@dataphract dataphract mentioned this pull request Jan 10, 2022
Closed
@SET001 SET001 mentioned this pull request Nov 14, 2023
Merged
github-merge-queue bot pushed a commit that referenced this pull request Nov 21, 2023
@SET001
Allow #[derive(Bundle)] on tuple structs (take 3) (#10561)
eeb0c2f 
- rework of old @Veykril's work in
[2499](#2499)
- Fixes [3537](#3537)
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
@SET001
Allow #[derive(Bundle)] on tuple structs (take 3) (bevyengine#10561)
2b64d2a 
- rework of old @Veykril's work in
[2499](bevyengine#2499)
- Fixes [3537](bevyengine#3537)
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

@NathanSWard NathanSWard Awaiting requested review from NathanSWard

Assignees

No one assigned

Labels

A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Veykril @mockersf @alice-i-cecile @cart