Skip to content
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

Fix paths in SystemData macro #232

Merged
merged 5 commits into from
Nov 27, 2023
Merged

Fix paths in SystemData macro #232

merged 5 commits into from
Nov 27, 2023

Conversation

IsseW
Copy link
Contributor

@IsseW IsseW commented Nov 20, 2023

No description provided.

@Imberflur
Copy link
Contributor

Iirc this is commonly used via a re-export from specs so many users won't have the shred crate in scope. I think it is worth giving this case some consideration.

@IsseW
Copy link
Contributor Author

IsseW commented Nov 21, 2023

Iirc this is commonly used via a re-export from specs so many users won't have the shred crate in scope. I think it is worth giving this case some consideration.

Through specs you have specs::shred to import in this case. Which I think will be a better thing to have to import than all of World, SystemData and ResourceId. Which could collide with user code.

So maybe if there aren't leading any :: this could work? I don't think there's a way to solve not having to import anything from specs without creating a seperate proc macro in specs.

@Imberflur
Copy link
Contributor

Another alternative might be to stop re-exporting the macro in specs and require the user to depend on shred to get the macro.... although specs also re-exports shred so users could still try to use the macro without depending on specs...

I think having no leading :: is a good compromise that improves the current state of things without regressions. This is the best approach for now IMO.

We could add an attribute to the macro for providing the path to shred (like serdes crate attribute). This adds some complexity to the implementation though and would be more cumbersome to use especially in the common case of using the macro through specs. At that point a separate macro for specs may be ideal.

@IsseW
Copy link
Contributor Author

IsseW commented Nov 21, 2023

Removed leading ::.

src/system.rs Outdated Show resolved Hide resolved
@Imberflur Imberflur merged commit 5450bf3 into amethyst:master Nov 27, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants