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

Palette improvements #433

Open
wants to merge 5 commits into
base: master
from
Open

Palette improvements #433

wants to merge 5 commits into from

Conversation

@Lignum
Copy link
Contributor

@Lignum Lignum commented Sep 4, 2017

After heavy debate, as seen in #343 and #197, I feel it's time for a new PR to fix some of the things that people were complaining about.

This includes:

  • Functions like term.(g|s)etPaletteColou?r now deal with colours encoded as RGB8 instead of 0-1 range values.
  • There is now a term.nativePaletteColo(u)r( colour ), as suggested by @SquidDev, which returns the default RGB8 values for a certain colour. @BombBloke I'd love to put this in colours, but writing this function in Java is a lot cleaner, since all the colours are stored in the Colours class.
  • colours.rgb8 is now neatly divided into colours.packRGB and colours.unpackRGB.

This is going to break a lot of stuff, but that's why we haven't released yet, after all.

Lignum added 4 commits Sep 4, 2017
@viluon viluon mentioned this pull request Sep 4, 2017
@viluon
Copy link

@viluon viluon commented Sep 4, 2017

Thank you @Lignum.

Since the API is being changed again, I'd like to suggest adding a means of changing multiple palette entries at once. A couple of term.(s|g)Palette functions accepting/returning a table would enable for effortless palette swaps.

@Lignum
Copy link
Contributor Author

@Lignum Lignum commented Sep 5, 2017

@viluon No problem! Passing tables was a feature in the original PR for some time, but we decided we didn't need it. As such, I won't bother putting any effort into adding it back unless @dan200 has changed his mind.

@BombBloke
Copy link
Contributor

@BombBloke BombBloke commented Sep 5, 2017

@BombBloke I'd love to put this in colours, but writing this function in Java is a lot cleaner, since all the colours are stored in the Colours class.

Fair 'nuff. At the end of the day the location is less important than the availability. Thanks for your efforts! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.