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

add wesnoth.colors #4797

Merged
merged 2 commits into from
Mar 15, 2020
Merged

add wesnoth.colors #4797

merged 2 commits into from
Mar 15, 2020

Conversation

gfgtdf
Copy link
Contributor

@gfgtdf gfgtdf commented Mar 10, 2020

see #3706

the main usecase to be able to print messages and dialog labels in the color of a specific team.
Im still not 100% sure about the exact interface thougfh

@gfgtdf
Copy link
Contributor Author

gfgtdf commented Mar 10, 2020

@CelticMinstrel im also unsure won whether to make the function that gives a pango compatible string a poperty of a function color.pango_string vs color:to_pango_str() any opinions on that?

@gfgtdf gfgtdf force-pushed the lua_color branch 2 times, most recently from 0c1c950 to 12e572a Compare March 11, 2020 19:52
@CelticMinstrel
Copy link
Member

  • I don't see any reason to have the lua_color class - it looks like you could just embed the color_range directly into the userdata. Am I missing something?
  • The names rep and pango_string don't seem great. I don't even know what rep is, but for pango_string, why not just call it what it is - hex?

@gfgtdf
Copy link
Contributor Author

gfgtdf commented Mar 12, 2020

it looks like you could just embed the color_range directly into the userdata

Hmm yes could do that, when i started writing it i wasn't sure what exactly i wanted to put in that lua_color class so i just started with a new class.

I don't even know what rep is

the doc says "High-contrast shade, intended for the minimap markers." i just coped the name rep from the c++ code name. so we could call it contrast or minimap_color

, why not just call it what it is - hex?

I thought the name pango_string is more self explaining as it imples that its in the pango format, in particular that it contains the leading #

@CelticMinstrel
Copy link
Member

Yeah, calling it some variation of "map color" seems better than the cryptic rep; "contrast" doesn't seem too great but has the advantage of being less specific, as there might be other places besides the map where you'd want to use that colour.

As for the other, maybe pango_hex then?

@CelticMinstrel
Copy link
Member

Whoops, somehow accidentally closed it. By the way, is the pango hex string also the contrast colour?

@gfgtdf
Copy link
Contributor Author

gfgtdf commented Mar 12, 2020

By the way, is the pango hex string also the contrast colour?

No, its the "mid" color. afaik that's also what other dialogs with side colors (mp create) use.

@CelticMinstrel
Copy link
Member

My only other thought is wondering if we should avoid single-letter members (red, green, blue, alpha instead of r, g, b, a)…

@Wedge009 Wedge009 added the Lua API Issues with the Lua engine and API. label Mar 13, 2020
@gfgtdf
Copy link
Contributor Author

gfgtdf commented Mar 13, 2020

My only other thought is wondering if we should avoid single-letter members (red, green, blue, alpha instead of r, g, b, a)…

hmm no strong opinion on this one.

see wesnoth#3706

the main usecase to be able to show messages and
dialog labels in the color of a specific team.
@gfgtdf gfgtdf merged commit 9fdf618 into wesnoth:master Mar 15, 2020
hrubymar10 added a commit that referenced this pull request Mar 16, 2020
Pentarctagon pushed a commit that referenced this pull request Mar 16, 2020
* fix cb projecfile after https://github.com/wesnoth/wesnoth/pull/4797/files

* Update tests.cbp

* Update wesnoth.cbp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Lua API Issues with the Lua engine and API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants