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

Change Color to be a struct rather than an enum #629

Merged
merged 3 commits into from
Apr 30, 2017

Conversation

SpaceManiac
Copy link
Contributor

@SpaceManiac SpaceManiac commented Apr 10, 2017

This reduces the size of the type and eliminates the need to branch to read from it, increasing its usability, and bringing it closer to the conceptual Color type used by most libraries and SDL itself.

This is a breaking change, but the RGB and RGBA constructor functions should make the impact minimal for anybody not manually matching.

I suppose I should make a note about some of the other inconsistencies; raw() is private, "because reasons", but Color implements Into<ll::SDL_Color> meaning the conversion is publicly available as safe anyways, and the ttf module seems to independently re-implement this function for no discernible reason. This could perhaps also be improved.

@Cobrand
Copy link
Member

Cobrand commented Apr 11, 2017

This is definitely a breaking change but I think it is for the greater good as well. I do match manually these enums in some of my projects, and having to rewrite this will be both a pain and a relief for later.

This is a pretty huge breaking change though; I've added a changelog file, would you mind adding a short description of your changes and maybe linking to this issue, guiding people on how to migrate to the new Color struct ? It wouldn't be fair to add such a breaking change a not notify end users IMO

This reduces the size of the type and eliminates the need to branch to
read from it, increasing its usability, and bringing it closer to the
conceptual Color type used by most libraries and SDL itself.
@SpaceManiac
Copy link
Contributor Author

Added some information to the changelog with new usage examples. Also a little bit tidied raw() and its usage in ttf.

@Cobrand
Copy link
Member

Cobrand commented Apr 30, 2017

Thanks !

@Cobrand Cobrand merged commit 1a0d7d8 into Rust-SDL2:master Apr 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.

2 participants