On Mon, 26 May 2014 08:56:58 +0400
"Alexander S." <alex0player_AT_gmail.com> wrote:
Hey Alexander,
thanks for your feedback!
> this function implements STR command for changing a palette. Empty
> name specifically tells to revert to system color, thus, if(!name)
> clause is there for restoring a color that may have been changed using
> the same procedure.
I found out later; see my other replies. Thanks for clearing that up
again, though.
> Here:
> > if(BETWEEN(x, 16, 6*6*6+16)) { /* 256 colour */
> you forgot -1. Don't forget BETWEEN() is inclusive on both ends.
> However, the removal of extra stack variable and some additional
> comments are very nice. I wholeheartedly approve of them.
Good catch! I'll put that into the updated version of this patch.
> Your concern about repetitiveness is valid and bothered me when I
> wrote that function, although back then xloadcols() was in much poorer
> state. By now, we may as well reimplement xloadcols() in terms of
> xsetcolorname(i, NULL). I'd be glad if you did this, as unfortunately,
> I do not have access to development environment right now.
I'd be happy to do this! This patch alone makes the binary ~0.5KB
smaller. Let's see how much this additional cut will bring!
Cheers
FRIGN
--
FRIGN <dev_AT_frign.de>
Received on Mon May 26 2014 - 15:18:48 CEST