Merge lp:~widelands-dev/widelands/fh1-scenario-groundwork-emp1 into lp:widelands

[GunChleoc]

Started converting scenarios to new font renderer

- Story Message Box and Objectives now try to use the new font renderer before falling back to the old one.
- Added version of scenario formatting for the new renderer.
- Converted Empire Scenario 1 to new font renderer
- Atlantean campaign now uses the campaign_message_box function.

Finished the needed C++ programming for converting the scenarios to the new font renderer. Using Tutorial 1 and Empire 1 as test cases.

When testing, all story messages and objectives need to be looked at, to make sure that there's no broken markup.

https://code.launchpad.net/~widelands-dev/widelands/fh1-editorhelp should be reviewed first - those changes were merged into this branch to make it work.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 2974. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/317654212.
Appveyor build 2783. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_fh1_scenario_groundwork_emp1-2783.

[kaputtnik (franku)]

I guessed the issue i found also in current trunk will be fixed when converting to the new font renderer, but when opening the first tutorial with this branch i get the same messages in terminal for each message box:

Story Message Box: falling back to OLD font renderer:
[..]
Syntax error at 1:20: expected an allowed attribute, got 'font-size'. String continues with: '=18 font-weight=bold font-color=D1D1D1>Welcome to Widelands!<br></p><p font-size=8> <br></p><p line-'

or:

Syntax error at 1:23: expected an allowed attribute, got 'line-spacing'. String continues with: '=3 font-size=12>First, I’ll left-click on the symbol where I want the lumberjack’s hut to be bui'

There is also one with 'image'

Is this a new bug?

[GunChleoc (gunchleoc)]

Actually, that should not happen with the first tutorial - looks like I missed something while converting the scenario.

For all other scenarios, that message is expected.

[GunChleoc (gunchleoc)]

Seems like my changes for Tutorial1 didn't land in the branch, so they're lost now. Will have to redo those in another branch.

Is the empire scenario clean?

[kaputtnik (franku)]

The text in the first message does not use the available width below the image. The text appears all on the right side. In trunk the text uses the whole width of the message box below the image if the text block is taller than the height of the image.

All other looks good, imho.

Sorry for posting in the wrong merge request :-S

[kaputtnik (franku)]

Some nitpicking in the comments

[GunChleoc (gunchleoc)]

Thanks for the review :)

Needs a quick retest before merging.

[kaputtnik (franku)]

Looks like there was a merge conflict wich is not solved in scenario/richtext.lua. See line number 350 in the div below.

The scenario does not even load:

Trying to run: map:scripting/init.lua: Fatal exception: lua: [/home/kaputtnik/Quellcode/widelands-repo/fh1-scenario-groundwork-emp1/src/scripting/lua_errors.cc:22] [/home/kaputtnik/Quellcode/widelands-repo/fh1-scenario-groundwork-emp1/src/scripting/lua_errors.cc:22] [/home/kaputtnik/Quellcode/widelands-repo/fh1-scenario-groundwork-emp1/src/scripting/lua_errors.cc:22] [/home/kaputtnik/Quellcode/widelands-repo/fh1-scenario-groundwork-emp1/src/scripting/lua_errors.cc:22] [string "scripting/richtext.lua"]:311: 'end' expected (to close 'function' at line 309) near '
FATAL ERROR - game crashed. Attempting emergency save.

review: Needs Fixing

[GunChleoc (gunchleoc)]

Hopefully fixed now.

[kaputtnik (franku)]

No, see diff comment.

review: Needs Fixing

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 3027. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/325436141.
Appveyor build 2835. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_fh1_scenario_groundwork_emp1-2835.

[GunChleoc (gunchleoc)]

Good thing Travis errored - I hate not being able to compile. Should be fixed now.

[GunChleoc (gunchleoc)]

@bunnybot merge

[kaputtnik (franku)]

The text of the first message does not yet slip below the image which i have tried to explain in https://code.launchpad.net/~widelands-dev/widelands/fh1-scenario-groundwork-emp1/+merge/335294/comments/878675. I thought that has been fixed?

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 3033. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/325760854.
Appveyor build 2841. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_fh1_scenario_groundwork_emp1-2841.

[bunnybot (widelandsofficial)]

Refusing to merge, since Travis is not green. Use @bunnybot merge force for merging anyways.

Travis build 3033. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/325760854.

[GunChleoc (gunchleoc)]

No, it hasn't, since that's already known - it will take come non-trivial coding to add the feature to the new font renderer too. There is some stuff the new renderer can do that the old one can't - e.g. proper bullet-point lists - and vice versa.

I still think that converting everything now is important though, since our Korean translator can see only blank squares.

Is everything else OK in this branch now?

[kaputtnik (franku)]

It's not a show stopper, and everything else looks good, so get this in now :-)

@bunnybot merge force

[GunChleoc (gunchleoc)]

Thanks for the review :)

=== modified file 'data/campaigns/emp01.wmf/scripting/texts.lua'
--- data/campaigns/emp01.wmf/scripting/texts.lua	2016-05-17 16:40:22 +0000
+++ data/campaigns/emp01.wmf/scripting/texts.lua	2018-01-06 11:27:52 +0000
@@ -2,8 +2,7 @@
 -- Some formating functions
 -- =========================
 
-include "scripting/formatting.lua"
-include "scripting/format_scenario.lua"
+include "scripting/richtext_scenarios.lua"
 
 function lutius(title, text)
    return speech("map:Lutius.png", "2F9131", title, text)
@@ -28,7 +27,7 @@
    title=_"Build a blockhouse",
    number = 1,
    body = objective_text(_"Blockhouse",
-      listitem_bullet(_[[Build a blockhouse at the red house symbol on the east side of the forests, to the right of your provisional headquarters.]])
+      li(_[[Build a blockhouse at the red house symbol on the east side of the forests, to the right of your provisional headquarters.]])
    ),
 }
 
@@ -37,7 +36,7 @@
    title=_"Build a lumberjack’s house",
    number = 1,
    body = objective_text(_"Lumberjack’s House",
-      listitem_bullet(_[[Build a lumberjack’s house at the red house symbol, south of your provisional headquarters.]])
+      li(_[[Build a lumberjack’s house at the red house symbol, south of your provisional headquarters.]])
    ),
 }
 
@@ -46,7 +45,7 @@
    title=_"Build 2 lumberjack’s houses and a sawmill",
    number = 3,
    body = objective_text(_"Two Lumberjack’s Houses and a Sawmill",
-      listitem_bullet(_[[Build two more lumberjack’s houses and a sawmill as soon as there is enough space for them.]])
+      li(_[[Build two more lumberjack’s houses and a sawmill as soon as there is enough space for them.]])
    ),
 }
 
@@ -55,7 +54,7 @@
    title=_"Build a forester’s house",
    number = 1,
    body = objective_text(_"Forester’s House",
-      listitem_bullet(_[[Build a forester’s house to preserve the wood resources of this island.]])
+      li(_[[Build a forester’s house to preserve the wood resources of this island.]])
    ),
 }
 
@@ -64,8 +63,8 @@
    title=_"Build a quarry",
    number = 5,
    body = objective_text(_"Quarry",
-      listitem_bullet(_[[Build a quarry in the south to cut some granite and marble out of the rocks.]]) ..
-      listitem_arrow(_[[These might be used for future buildings.]])
+      li(_[[Build a quarry in the south to cut some granite and marble out of the rocks.]]) ..
+      li_arrow(_[[These might be used for future buildings.]])
    ),
 }
 

=== added file 'data/scripting/format_scenario.lua'
--- data/scripting/format_scenario.lua	1970-01-01 00:00:00 +0000
+++ data/scripting/format_scenario.lua	2018-01-06 11:27:52 +0000
@@ -0,0 +1,85 @@
+-- RST
+-- format_scenario.lua
+-- -------------------
+--
+-- Function to simplify and unique text formatting in scenarios.  Most of these
+-- functions are simple wrapper functions that make working with widelands rich
+-- text formatting system more bearable.
+
+
+-- RST
+-- .. function:: speech(img, clr, title, text)
+--
+--    Formats a text as spoken by one person in a scenario. Use it as follows:
+--
+--    .. code-block:: lua
+--
+--        function jundlina(title, text)
+--           return speech("map:princess.png", "2F9131", title, text)
+--        end
+--
+--    :arg img: name of the image to use for this speaker
+--    :arg clr: a valid 6 char hex color to use for the name of this speaker
+--    :arg title: Title of this text.
+--    :arg text: The text itself. If this is nil, :const:`title` is used as text
+--       instead and there will not be any title.
+--    :returns: the formatted text.
+
+function speech(img, clr, g_title, g_text)
+   local title, text = g_title, g_text
+   if not text then
+      title = nil
+      text = g_title
+   end
+
+   -- Surround the text with translatable ","
+   text = (_'“%s”'):format(text)
+
+   local s = ""
+   if title then
+      s = rt("<p font-size=20 font-weight=bold font-face=serif " ..
+         ("font-color=%s>"):format(clr) .. title ..
+         "</p><p font-size=8> <br></p>"
+      )
+   end
+
+   return s .. rt(("image=%s"):format(img), p(text))
+end
+
+
+-- RST
+-- .. function:: objective_text(heading, body)
+--
+--    Provides nice formatting for objective texts.
+--
+--    :returns: a rich text object that contains the formatted
+--       objective text.
+--
+function objective_text(heading, body)
+   return rt(h2(heading) .. p(body))
+end
+
+
+-- RST
+-- Append an objective text with a header to a dialog box in a nice fashion.
+-- For displaying objectives with an extra title when an advisor is talking
+--
+--    Provides nice formatting for objective texts.
+--    the following arguments will be parsed:
+--    number: the number of objectives described in the body
+--    body: the objective text, e.g. created with function objective_text(heading, body)
+--
+--    :returns: a rich text object that contains the formatted
+--       objective text & title.
+--
+function new_objectives(...)
+   local sum = 0
+   local s = ""
+   for idx,obj in ipairs{...} do
+      s = s .. obj.body
+      sum = sum + obj.number
+   end
+   return rt("<p font-size=10> <br></p>" ..
+      "<p font=serif font-size=18 font-weight=bold font-color=D1D1D1>"
+      .. ngettext("New Objective", "New Objectives", sum) .. "</p>") .. s
+end

=== modified file 'data/scripting/messages.lua'
--- data/scripting/messages.lua	2017-05-12 09:43:57 +0000
+++ data/scripting/messages.lua	2018-01-06 11:27:52 +0000
@@ -5,6 +5,7 @@
 -- Functions to send messages to the player and to add objectives to campaigns.
 
 include "scripting/coroutine.lua"
+include "scripting/richtext.lua"
 include "scripting/table.lua"
 include "scripting/ui.lua"
 
@@ -51,7 +52,7 @@
    -- While the message box is shown, the user cannot do anything else anyway.
    local user_input = wl.ui.get_user_input_allowed()
    wl.ui.set_user_input_allowed(true)
-   player:message_box(title, body, parameters)
+   player:message_box(title, rt(body), parameters)
    wl.ui.set_user_input_allowed(user_input)
 end
 
@@ -83,9 +84,9 @@
 --
 function add_campaign_objective(objective)
    if objective.obj_name then
-      return wl.Game().players[1]:add_objective(objective.obj_name, objective.obj_title, objective.obj_body)
+      return wl.Game().players[1]:add_objective(objective.obj_name, objective.obj_title, rt(objective.obj_body))
    else
-      return wl.Game().players[1]:add_objective(objective.name, objective.title, objective.body)
+      return wl.Game().players[1]:add_objective(objective.name, objective.title, rt(objective.body))
    end
 end
 
@@ -160,7 +161,7 @@
       -- message_box takes care of this, but player:message_box does not
       local user_input = wl.ui.get_user_input_allowed()
       wl.ui.set_user_input_allowed(true)
-      player:message_box(message.title, message.body, message)
+      player:message_box(message.title, rt(message.body), message)
       wl.ui.set_user_input_allowed(user_input)
    else
       message_box(plr, message.title, message.body, message)

=== modified file 'data/scripting/richtext.lua'
--- data/scripting/richtext.lua	2018-01-04 14:29:21 +0000
+++ data/scripting/richtext.lua	2018-01-06 11:27:52 +0000
@@ -73,7 +73,7 @@
 --    :arg text: the text to be enclosed in rich text tags.
 --    :type text: :class:`string`
 --    :returns: the wrapped rich text.
---
+
 function rt(text_or_attributes, text)
    if text then
       return "<rt " .. text_or_attributes .. ">" .. text .. "</rt>"
@@ -99,7 +99,7 @@
 --       ref:       TODO(GunChleoc): I have no idea what it does.
 --
 --    :returns: the img tag.
---
+
 function img(src, attributes)
    if attributes then
       return "<img src=" .. src .." " .. attributes .. ">"
@@ -148,7 +148,7 @@
 --    Like :func:`h2` but smaller.
 --
 --    :returns: A paragraph with text formatted as heading.
---
+
 function h3(text)
    return p_font("", "size=13 color=D1D1D1", vspace(6) .. text .. vspace(1))
 end
@@ -160,7 +160,7 @@
 --    Like :func:`h3` but smaller.
 --
 --    :returns: A paragraph with text formatted as heading.
---
+
 function h4(text)
    return p_font("", "size=12 italic=1 color=D1D1D1", text)
 end
@@ -233,7 +233,7 @@
 --
 --    :returns: The closing tags for a paragraph
 function close_p(t)
-   return vspace(6) .. "</font></p>"
+   return vspace(6) .. "</font>" .. vspace(6)  .. "</p>"
 end
 
 -- RST
@@ -252,7 +252,7 @@
 --       ref:       TODO(GunChleoc): I don't know what this does.
 --
 --    :returns: The text wrapped in font tags with the given attributes
---
+
 function font(attributes, text)
    return ("<font %s>"):format(attributes) .. text .. "</font>"
 end
@@ -264,7 +264,7 @@
 --    :arg gap: the size of the space as pixels.
 --
 --    :returns: a space tag
---
+
 function space(gap)
    return "<space gap="..gap..">"
 end
@@ -276,7 +276,7 @@
 --    :arg gap: the size of the space as pixels.
 --
 --    :returns: a vspace tag
---
+
 function vspace(gap)
    return "<vspace gap="..gap..">"
 end
@@ -290,7 +290,7 @@
 --    :arg dd: "description data", will be rendered normally.
 --
 --    :returns: a p tag containing the formatted text
---
+
 function dl(dt, dd)
    return p(b(dt) .. " " .. dd)
 end
@@ -305,6 +305,7 @@
 --    :arg text: the text of the list item
 --
 --    :returns: a p tag containing the formatted text
+
 function li(text_or_symbol, text)
    if text then
       return div(p(text_or_symbol)) .. div(p(space(6))) .. div("width=*", p(text .. vspace(6)))
@@ -336,6 +337,7 @@
 --    :arg text: the text to be placed next to the image
 --
 --    :returns: the text wrapped in a paragraph and placed next to the image, the outer tag is a div.
+
 function li_image(imagepath, text)
    return
       div("width=100%",
@@ -353,7 +355,7 @@
 --    :arg link: the text to format
 --
 --    :returns: a font tag containing the underlined text
---
+
 function a(link)
    return font("underline=1", link)
 end
@@ -366,7 +368,7 @@
 --    :arg link: the text to format
 --
 --    :returns: a font tag containing the bold text
---
+
 function b(text)
    return font("bold=1", text)
 end
@@ -379,7 +381,7 @@
 --    :arg link: the text to format
 --
 --    :returns: a font tag containing the italic text
---
+
 function i(text)
    return font("italic=1", text)
 end
@@ -391,7 +393,7 @@
 --    :arg link: the text to format
 --
 --    :returns: a font tag containing the underlined text
---
+
 function u(text)
    return font("underline=1", text)
 end
@@ -416,7 +418,7 @@
 --    :arg text: the text to be enclosed in div tags.
 --    :type text: :class:`string`
 --    :returns: the text wrapped in a div tag.
---
+
 function div(text_or_attributes, text)
    if text then
       return ("<div %s>"):format(text_or_attributes) .. text .. "</div>"
@@ -433,7 +435,7 @@
 --    :arg t1: text in h3 format.
 --    :arg t2: text in p format.
 --    :returns: header text followed by normal text.
---
+
 function inline_header(header, text)
    return
       div("width=100%", vspace(8)) ..

=== renamed file 'data/scripting/format_scenario.lua' => 'data/scripting/richtext_scenarios.lua'
--- data/scripting/format_scenario.lua	2016-10-19 09:00:29 +0000
+++ data/scripting/richtext_scenarios.lua	2018-01-06 11:27:52 +0000
@@ -1,12 +1,13 @@
+include "scripting/richtext.lua"
+
 -- RST
--- format_scenario.lua
--- -------------------
+-- richtext_scenarios.lua
+-- ----------------------
 --
 -- Function to simplify and unique text formatting in scenarios.  Most of these
 -- functions are simple wrapper functions that make working with widelands rich
 -- text formatting system more bearable.
 
-
 -- RST
 -- .. function:: speech(img, clr, title, text)
 --
@@ -20,31 +21,33 @@
 --
 --    :arg img: name of the image to use for this speaker
 --    :arg clr: a valid 6 char hex color to use for the name of this speaker
---    :arg title: Title of this text.
---    :arg text: The text itself. If this is nil, :const:`title` is used as text
---       instead and there will not be any title.
+--    :arg title: Title of this text. Use empty string if you don't want any.
+--    :arg text: The text itself.
 --    :returns: the formatted text.
---
-function speech(img, clr, g_title, g_text)
-   local title, text = g_title, g_text
-   if not text then
-      title = nil
-      text = g_title
+
+function speech(img, clr, title, text)
+   if title ~= "" then
+      title = h1(clr, title)
    end
 
    -- Surround the text with translatable ","
    text = (_'“%s”'):format(text)
 
-   local s = ""
-   if title then
-      s = rt("<p font-size=20 font-weight=bold font-face=serif " ..
-         ("font-color=%s>"):format(clr) .. title ..
-         "</p><p font-size=8> <br></p>"
-      )
-   end
-
-   return s .. rt(("image=%s"):format(img), p(text))
-end
+   return title .. li_image(img, p(text))
+end
+
+-- RST
+-- .. function:: paragraphdivider()
+--
+--    Closes a paragraph and opens a new paragraph. Use this when you format a string with the speech function
+--    and need to divide the speech into multiple paragraphs.
+--
+--    :returns: close_p() .. open_p()
+
+function paragraphdivider()
+   return close_p() .. open_p()
+end
+
 
 
 -- RST
@@ -54,9 +57,9 @@
 --
 --    :returns: a rich text object that contains the formatted
 --       objective text.
---
+
 function objective_text(heading, body)
-   return rt(h2(heading) .. p(body))
+   return h2(heading) .. p(body)
 end
 
 
@@ -71,15 +74,13 @@
 --
 --    :returns: a rich text object that contains the formatted
 --       objective text & title.
---
+
 function new_objectives(...)
    local sum = 0
-   local s = ""
+   local text = ""
    for idx,obj in ipairs{...} do
-      s = s .. obj.body
+      text = text .. obj.body
       sum = sum + obj.number
    end
-   return rt("<p font-size=10> <br></p>" ..
-      "<p font=serif font-size=18 font-weight=bold font-color=D1D1D1>"
-      .. ngettext("New Objective", "New Objectives", sum) .. "</p>") .. s
+   return h1(ngettext("New Objective", "New Objectives", sum)) .. text
 end

=== modified file 'src/graphic/text/rt_parse.cc'
--- src/graphic/text/rt_parse.cc	2017-12-06 06:47:34 +0000
+++ src/graphic/text/rt_parse.cc	2018-01-06 11:27:52 +0000
@@ -256,6 +256,7 @@
 
 		tc.allowed_children.insert("font");
 		tc.allowed_children.insert("space");
+		tc.allowed_children.insert("vspace");
 		tc.allowed_children.insert("br");
 		tc.allowed_children.insert("img");
 		tc.allowed_children.insert("div");

=== modified file 'src/graphic/text/rt_render.cc'
--- src/graphic/text/rt_render.cc	2017-12-28 14:57:00 +0000
+++ src/graphic/text/rt_render.cc	2018-01-06 11:27:52 +0000
@@ -828,6 +828,7 @@
 		for (std::shared_ptr<RenderNode> n : nodes_to_render_) {
 			// TODO(GunChleoc): With div width=*, we are getting newline nodes here, which should have
 			// been consumed
+			// https://bugs.launchpad.net/widelands/+bug/1738760
 			const auto& renderme = n->render(texture_cache);
 			for (auto& rendered_rect : renderme->rects) {
 				if (rendered_rect->was_visited()) {

=== modified file 'src/wui/game_objectives_menu.cc'
--- src/wui/game_objectives_menu.cc	2017-02-23 17:58:25 +0000
+++ src/wui/game_objectives_menu.cc	2018-01-06 11:27:52 +0000
@@ -86,5 +86,15 @@
  * An entry in the objectives menu has been selected
  */
 void GameObjectivesMenu::selected(uint32_t const t) {
-	objectivetext.set_text(t == ListType::no_selection_index() ? std::string() : list[t].descr());
+	const std::string text = t == ListType::no_selection_index() ? "" : list[t].descr();
+	// TODO(GunChleoc): When all campaigns, scenarios and win conditions have been converted, simply add the text above.
+	try {
+		objectivetext.force_new_renderer();
+		objectivetext.set_text(text);
+		log("Objectives: using NEW font renderer.\n");
+	} catch (const std::exception& e) {
+		log("Objectives: falling back to OLD font renderer:\n%s\n%s\n", text.c_str(), e.what());
+		objectivetext.force_new_renderer(false);
+		objectivetext.set_text(text);
+	}
 }