Formspecs: Allow specifying a certain alpha value for the box[] element #7116

Thomas--S · 2018-03-09T20:58:10Z

This allows using the alpha component of the ColorString.
The default behaviour is unchanged.
Backwards compatible (only boxes with the new, fourth parameter are not drawn in old clients).

I'm looking forward to your comments! Thanks in advance for your efforts!

rubenwardy · 2018-03-09T21:02:20Z

src/gui/guiFormSpecMenu.cpp

 		video::SColor tmp_color;

+		bool use_color_alpha = false;
+		if (parts.size() == 4 && parts[3] == "true") {


there's an is_yes function somewhere

SmallJoker · 2018-03-10T09:30:48Z

Are you sure this is backwards compatible? FORMSPEC_API_VERSION was not increased, so older clients will not draw the box and throw an error instead:
https://github.com/Thomas--S/minetest/blob/49ca8474389b315ff707270e4370406cccfcba82/src/gui/guiFormSpecMenu.cpp#L1602

Also the "close on enter" feature (#4417) had to be changed to a new formspec element (#4566) due this strange version management system.

Thomas--S · 2018-03-10T09:52:35Z

Thanks for your comment, SmallJoker!

Only the boxes that use the new fourth parameter will not be drawn in old clients.

The "normal" old boxes with three parameters will work in all combinations.

Here's a little table:

	old client – new server	new client – old server	new client – new server	old client – old server
boxes with three params	✔️	✔️	✔️	✔️
boxes with four params	🚫 not drawn	✔️	✔️	🚫 not drawn

Backwards compatible (only boxes with the new, fourth parameter are not drawn in old clients).

So, the statement from my first post is correct. (I tested it.)
Backwards compatiblity is ensured for three-param boxes, four-param boxes are not drawn in old clients. (A new introduced element wouldn't be drawn in old clients either, I think.)

Thanks for your efforts!

Edit (2018-03-10 14:17 UTC):

I looked at the two PRs mentioned above again. I think this PR has technically the same problem, but a box, which is not drawn, usually doesn't cause problems, only the design might not look so good anymore.

Another good thing is that with 0.5.0, both clients and server both must have the new system, so the "problem" mentioned above only occurs with -dev versions.

SmallJoker · 2018-03-24T18:08:39Z

src/gui/guiFormSpecMenu.cpp


 		video::SColor tmp_color;

+		bool use_color_alpha = false;


Shorter: bool use_color_alpha = parts.size() >= 4 && is_yes(parts[3]);

X

rubenwardy

I'd like to see a new argument added to the color parsing function which is the alpha if not specified. This can default to 1

Thomas--S · 2018-04-08T07:00:02Z

PR updated according to @rubenwardy's comment. Tested and seems to work.

SmallJoker · 2018-04-08T12:10:06Z

src/util/string.cpp

 #endif

-static bool parseHexColorString(const std::string &value, video::SColor &color);
+static bool parseHexColorString(const std::string &value, video::SColor &color, unsigned char default_alpha = 0xff);


Line too long. Split after color,

SmallJoker · 2018-04-08T12:10:22Z

src/util/string.cpp

 }

-bool parseColorString(const std::string &value, video::SColor &color, bool quiet)
+bool parseColorString(const std::string &value, video::SColor &color, bool quiet, unsigned char default_alpha)


SmallJoker · 2018-04-08T12:10:52Z

src/util/string.h

 char *mystrtok_r(char *s, const char *sep, char **lasts);
 u64 read_seed(const char *str);
-bool parseColorString(const std::string &value, video::SColor &color, bool quiet);
+bool parseColorString(const std::string &value, video::SColor &color, bool quiet, unsigned char default_alpha = 0xff);


Done by adding a `default_alpha` parameter to the `parseColorString()` function

Thomas--S · 2018-04-22T09:06:49Z

@rubenwardy, @SmallJoker could you please review this again? The changes you requested are all done, as far as I can see.

SmallJoker

LGTM

Thomas--S · 2018-04-23T20:26:30Z

Thanks for merging!

rubenwardy reviewed Mar 9, 2018

View reviewed changes

rubenwardy added Feature ✨ PRs that add or enhance a feature @ Client / Audiovisuals labels Mar 9, 2018

Thomas--S force-pushed the box_alpha branch from 3dbe7e3 to 49ca847 Compare March 9, 2018 21:13

rubenwardy previously approved these changes Mar 9, 2018

View reviewed changes

rubenwardy added the One approval ✅ ◻️ label Mar 9, 2018

SmallJoker reviewed Mar 24, 2018

View reviewed changes

Thomas--S force-pushed the box_alpha branch from 49ca847 to cf5e5bf Compare March 24, 2018 18:16

rubenwardy removed the One approval ✅ ◻️ label Apr 7, 2018

rubenwardy requested changes Apr 7, 2018

View reviewed changes

Thomas--S force-pushed the box_alpha branch from cf5e5bf to 321330d Compare April 8, 2018 06:57

SmallJoker reviewed Apr 8, 2018

View reviewed changes

Formspecs: Allow specifying a certain alpha value for the box[] element

21f5ea7

Done by adding a `default_alpha` parameter to the `parseColorString()` function

Thomas--S force-pushed the box_alpha branch from 321330d to 21f5ea7 Compare April 8, 2018 13:43

Thomas--S changed the title ~~Formspecs: Add a <use_color_alpha> parameter to the box[] element~~ Formspecs: Allow specifying a certain alpha value for the box[] element Apr 8, 2018

SmallJoker approved these changes Apr 22, 2018

View reviewed changes

SmallJoker added the One approval ✅ ◻️ label Apr 22, 2018

rubenwardy approved these changes Apr 23, 2018

View reviewed changes

rubenwardy added >= Two approvals ✅ ✅ and removed One approval ✅ ◻️ labels Apr 23, 2018

rubenwardy merged commit 9577a43 into luanti-org:master Apr 23, 2018

Thomas--S deleted the box_alpha branch April 23, 2018 20:26

Formspecs: Allow specifying a certain alpha value for the box[] element #7116

Formspecs: Allow specifying a certain alpha value for the box[] element #7116

Uh oh!

Conversation

Thomas--S commented Mar 9, 2018

Uh oh!

rubenwardy Mar 9, 2018

Choose a reason for hiding this comment

Uh oh!

Thomas--S Mar 9, 2018

Choose a reason for hiding this comment

Uh oh!

SmallJoker commented Mar 10, 2018

Uh oh!

Thomas--S commented Mar 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SmallJoker Mar 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rubenwardy left a comment

Choose a reason for hiding this comment

Uh oh!

Thomas--S commented Apr 8, 2018

Uh oh!

SmallJoker Apr 8, 2018

Choose a reason for hiding this comment

Uh oh!

SmallJoker Apr 8, 2018

Choose a reason for hiding this comment

Uh oh!

SmallJoker Apr 8, 2018

Choose a reason for hiding this comment

Uh oh!

Thomas--S commented Apr 22, 2018

Uh oh!

SmallJoker left a comment

Choose a reason for hiding this comment

Uh oh!

Thomas--S commented Apr 23, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Thomas--S commented Mar 10, 2018 •

edited

Loading

SmallJoker Mar 24, 2018 •

edited

Loading