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

Remove Debug bound for Resource trait. #20

Merged
merged 1 commit into from
May 30, 2017
Merged

Remove Debug bound for Resource trait. #20

merged 1 commit into from
May 30, 2017

Conversation

Object905
Copy link
Contributor

When using type from other crate not every type implements Debug, and Debug is not use everywhere meaningfully anyway.

@torkleyy
Copy link
Member

torkleyy commented May 30, 2017

I added the trait bound intentionally. According to the API guidelines, every type should implement Debug. Without forcing the bound, we cannot debug Resources.

@torkleyy
Copy link
Member

cc @kvark @Aceeri

@kvark
Copy link
Member

kvark commented May 30, 2017

Why don't we lift the restriction from Resource but still have derive(Debug) working only when Resource: Debug?

@torkleyy
Copy link
Member

That's not possible; there is no way to find out if all the types are Debug (nor access their vtables) because we have trait objects in Resources.

@kvark
Copy link
Member

kvark commented May 30, 2017

To be honest, I find this requirement (to have everything public as Debug) to be a little ridiculous. Is there a debugging use case where Resource: Debug would really help?

@torkleyy
Copy link
Member

To be honest, I find this requirement (to have everything public as Debug) to be a little ridiculous.

It is.

Is there a debugging use case where Resource: Debug would really help?

Well, just if you want to print all components...which brings me to the idea that we could use @Aceeri's component groups for this. I guess that's a great compromise. Thanks everybody!

@torkleyy torkleyy merged commit c9c5b74 into amethyst:master May 30, 2017
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.

None yet

3 participants