amethyst / shred

Shared resource dispatcher
Apache License 2.0
233 stars 66 forks source link

Fix paths in SystemData macro #232

Closed IsseW closed 9 months ago

Imberflur commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

Removed leading ::.