Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/fh1-barbarians1 into lp:widelands
Found only some obvious reformattings, LGTM.
Two questions inline.
Will do a testplay today in German an check for missing
translations and formatting issues.
./wideland --scenario=/data/campaigns/bar01.wmf/
Aahh. thron is the name of one of the characters :-)
Diff comments:
> === modified file 'data/campaigns/bar01.wmf/scripting/texts.lua'
> --- data/campaigns/bar01.wmf/scripting/texts.lua 2017-06-05 11:33:26
> +
> +++ data/campaigns/bar01.wmf/scripting/texts.lua 2018-01-26 17:04:24
> +
> @@ -42,21 +41,19 @@
> title = _"Start building mines on the mountain",
> number = 2,
> body = objective_text(_"Build coal and iron mines",
> - listitem_bullet(_"Build a coal mine and an iron mine.") ..
> - listitem_arrow(_"To do so, place a flag up on the mountain’s flank to
> the east (on mountain terrain though, not mountain meadow). When you click on
> the new flag, you can send geologists there. Because the flag is on a
> mountain, the geologists will search for ores; otherwise, they would search
> for water. Then build a mine for both kinds of resources that they will find,
> choosing the appropriate mine to be built:") ..
> - "" ..
> - rt("image=tribes/immovables/resi_coal1/idle_00.png", p(_"a bit of
> coal")) ..
> - rt("image=tribes/immovables/resi_coal2/idle_00.png", p(_"a lot of
> coal")) ..
> - rt("image=tribes/immovables/resi_iron1/idle_00.png", p(_"a bit of
> iron")) ..
> - rt("image=tribes/immovables/resi_iron2/idle_00.png", p(_"a lot of
> iron")) ..
> - rt("image=tribes/immovables/resi_gold1/idle_00.png", p(_"a bit of
> gold")) ..
> - rt("image=tribes/immovables/resi_gold2/idle_00.png", p(_"a lot of
> gold")) ..
> - rt("image=tribes/immovables/resi_stones1/idle_00.png", p(_"a bit of
> granite")) ..
> - rt("image=tribes/immovables/resi_stones2/idle_00.png", p(_"a lot of
> granite")) ..
> - rt("image=tribes/immovables/resi_water1/idle_00.png", p(_"water")) ..
> - rt("image=tribes/immovables/resi_none/idle_00.png", p(_"nothing was
> found here")) ..
> - "" ..
What kind of Markup is rt?
This does not look lkie this explanation:
https://www.w3schools.com/tags/tag_rt.asp
> - p(_[[Mines can only be built on mountain terrain. Suitable places
> for mines are displayed as orange mine symbols.]]))
> + li(_"Build a coal mine and an iron mine.") ..
> + li_arrow(_"To do so, place a flag up on the mountain’s flank to the
> east (on mountain terrain though, not mountain meadow). When you click on the
> new flag, you can send geologists there. Because the flag is on a mountain,
> the geologists will search for ores; otherwise, they would search for water.
> Then build a mine for both kinds of resources that they will find, choosing
> the appropriate mine to be built:") ..
> + li_image("tribes/immovables/resi_coal1/idle_00.png", _"a bit of coal")
> ..
> + li_image("tribes/immovables/resi_coal2/idle_00.png", _"a lot of coal")
> ..
> + li_image("tribes/immovables/resi_iron1/idle_00.png", _"a bit of iron")
> ..
> + li_image("tribes/immovables/resi_iron2/idle_00.png", _"a lot of iron")
> ..
> + li_image("tribes/immovables/resi_gold1/idle_00.png", _"a bit of gold")
> ..
> + li_image("tribes/immovables/resi_gold2/idle_00.png", _"a lot of gold")
> ..
> + li_image("tribes/immovables/resi_stones1/idle_00.png", _"a bit of
> granite") ..
> + li_image("tribes/immovables/resi_stones2/idle_00.png", _"a lot of
> granite") ..
> + li_image("tribes/immovables/resi_water1/idle_00.png", _"water") ..
> + li_image("tribes/immovables/resi_none/idle_00.png", _"nothing was
> found here") ..
> + p(_[[Mines can only be built on mountain terrain. Suitable places for
> mines are displayed as orange mine symbols.]]))
> }
>
> obj_basic_food = {
> @@ -166,7 +153,7 @@
>
> briefing_msg_02 = {
> title = _"The Story Begins",
> - body = thron(
> + body = thron("",
Uhm, what kind of markup/formatting is a thron?
>-- TRANSLATORS: Thron
>_[[We can see the raging flames that swallow Al’thunran from here,
> miles away.]]
>.. paragraphdivider() ..
--
https://code.launchpad.net/~widelands-dev/widelands/fh1-barbarians1/+merge/336706
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/fh1-barbarians1 into lp:widelands.
___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands
you are right, we should probably codify our style somewhere - since we are only using parts of the google c++ style. we have a long history of boost usage in Widelands, so we continue using it. We also have used exceptions a lot in the past, so we also continue using them. Were we a greenfield project, I personally would follow the style guide closer. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1574379-forester-wit. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands
I will take note for future. Thanks. I cannot resist asking: The Google C++ style guide disallows boost, with exceptions; lexical_cast is not mentioned in the list of allowed uses. How strictly should one follow the style guide? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1574379-forester-wit. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands
One more comment for the future: You could use boost::lexical_cast instead of "int ival = strtol(value.c_str(), nullptr, 10);" -- https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1574379-forester-wit. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bugfix-empire-campaign into lp:widelands
I completely misunderstood his comment, I thought he meant the repetition of "we". "sparingly" is wrong grammar. You can change it to "frugal" if you want. -- https://code.launchpad.net/~widelands-dev/widelands/bugfix-empire-campaign/+merge/336074 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bugfix-empire-campaign. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1574379-forester-wit. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands
Forgot the commit message. I guess bunnybot is not offended by repeated requests. @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1574379-forester-wit. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands has been updated. Commit Message changed to: Bug 1574379: Forester prefers good soil, and is thus more efficient. For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1574379-forester-wit. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands has been updated. Commit Message changed to: Bug 1574379: Forester prefers good soil. For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1574379-forester-wit. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands
I understood the Jan14th comments so that it is okay to merge this. @bunnybot merge Curious to see what happens. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1574379-forester-wit. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands
Continuous integration builds have changed state: Travis build 3102. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/334024010. Appveyor build 2909. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1574379_forester_wit-2909. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1574379-forester-wit. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
