Merge lp:~widelands-dev/widelands/tutorial-improvements into lp:widelands

[wl-zocker]

This branch makes use of the features requested in bug 1380286 and bug 1380287.

I also noticed some graphical glitch after starting a game, therefore I increased sleep() to 1000 in all campaigns/tutorials.

[TiborB (tiborb95)]

one small question in the diff. Otherwise it looks good, but I have not tested it.

[wl-zocker (wl-zocker)]

Answer in the diff.

[TiborB (tiborb95)]

see replay

[wl-zocker (wl-zocker)]

If the player destroys the quarry construction site, he is not allowed to rebuild it. I see three possibilities:

- Allow the user to rebuild the quarry. This would mean to redefinitions of immovable_is_legal() inside the while loop, which makes the code quite messy.
- Send a message to the user that destroying the quarry was not clever, and ask him to restart from savegame xy. However, this makes the impression that we were lazy.
- Leave it as is. The unexperienced user will hopefully not dismantle the quarry. If he does, the game does not crash. I can also remove the inner while loop, so the player gets automatically notified that something went wrong.

[TiborB (tiborb95)]

But now, the inner loop will "loop" forever and player will not know what is going on - do I understand it correctly?

I think player should receive an message he had messed things up and he should restart or reload the game.

[wl-zocker (wl-zocker)]

I changed it according to your suggestion. The bug that the game would not continue, has been in there before these changes, and I have heard of no complaint.

Please test it. Maybe GunChleoc should have a look on the strings before merging. I requested her review.

[GunChleoc (gunchleoc)]

I have added some suggestions for the strings, and 1 for the code. I haven't tested anything yet.

review: Needs Fixing

[TiborB (tiborb95)]

"I have heard of no complaint" - I believe this, there are probably a lot of such corner cases in tutorials, but no harm is done when you think also about this. I just left one comment in the diff.

Otherwise it LGTM (and yes, I tested it of course)

I presume GunChleoc will look at this yet and give approve if OK.

[wl-zocker (wl-zocker)]

Thank you both for you input. I am waiting for your approval now.

[TiborB (tiborb95)]

It is OK for me...

[GunChleoc (gunchleoc)]

Please go ahead and merge :)

review: Approve

[wl-zocker (wl-zocker)]

How do I do that? I've never merged to trunk before.

[TiborB (tiborb95)]

I prefer this method:

https://wl.widelands.org/wiki/BzrPrimer/#alternative_method

or somebody can do it for you :)

[GunChleoc (gunchleoc)]

Since the changes are small and current, there is no need to merge trunk into the branch first. So, this will do:

    $ cd ~/widelands-repo/trunk
    $ bzr pull (to get any recent changes to the main trunk)
    $ bzr merge ../bug-xxx
    $ bzr commit -m "Bug xxx fixed"
    $ bzr push lp:widelands/trunk

I usually grab the branch for merging from launchpad - merge requests already display the code needed on top of the page:

To merge this branch: bzr merge lp:~widelands-dev/widelands/tutorial-improvements

=== modified file 'campaigns/emp01.wmf/scripting/init.lua'
--- campaigns/emp01.wmf/scripting/init.lua	2014-10-02 17:05:38 +0000
+++ campaigns/emp01.wmf/scripting/init.lua	2015-06-01 20:08:53 +0000
@@ -11,6 +11,5 @@
 
 p1 = wl.Game().players[1]
 
-include "map:scripting/starting_conditions.lua"
 include "map:scripting/texts.lua"
 include "map:scripting/mission_thread.lua"

=== modified file 'campaigns/emp01.wmf/scripting/mission_thread.lua'
--- campaigns/emp01.wmf/scripting/mission_thread.lua	2014-10-31 11:36:29 +0000
+++ campaigns/emp01.wmf/scripting/mission_thread.lua	2015-06-01 20:08:53 +0000
@@ -1,10 +1,10 @@
 include "scripting/messages.lua"
 
 function mission_thread()
-   sleep(100) -- This is needed for yet unknown reasons
+   sleep(1000)
 
    -- Initial messages
-   local sea = wl.Game().map:get_field(47,25)
+   local sea = wl.Game().map:get_field(50,25)
    scroll_smoothly_to(sea,0)
 
    campaign_message_box(diary_page_1)
@@ -12,15 +12,17 @@
 
    -- Show the sea
    p1:reveal_fields(sea:region(6))
+   local ship = p1:place_bob("ship",sea)
    sleep(200)
    campaign_message_box(diary_page_2)
    -- Hide the sea after 5 seconds
    run(function() sleep(5000) p1:hide_fields(sea:region(6)) end)
 
    -- Back home
+   include "map:scripting/starting_conditions.lua"
    scroll_smoothly_to(wl.Game().map.player_slots[1].starting_field)
    campaign_message_box(diary_page_3)
-
+   ship:remove()
 
    sleep(400)
 

=== modified file 'campaigns/emp02.wmf/scripting/mission_thread.lua'
--- campaigns/emp02.wmf/scripting/mission_thread.lua	2014-10-31 11:36:29 +0000
+++ campaigns/emp02.wmf/scripting/mission_thread.lua	2015-06-01 20:08:53 +0000
@@ -5,7 +5,7 @@
 include "scripting/messages.lua"
 
 function building_materials()
-   sleep(200)
+   sleep(1000)
    campaign_message_box(diary_page_5)
 
    local map = wl.Game().map

=== modified file 'campaigns/tutorial01_basic_control.wmf/scripting/mission_thread.lua'
--- campaigns/tutorial01_basic_control.wmf/scripting/mission_thread.lua	2014-10-25 08:29:32 +0000
+++ campaigns/tutorial01_basic_control.wmf/scripting/mission_thread.lua	2015-06-01 20:08:53 +0000
@@ -6,7 +6,7 @@
    map:place_immovable("debris00",second_quarry_field)
    -- so that the player cannot build anything here
 
-   sleep(100)
+   sleep(1000)
 
    message_box_objective(plr, initial_message_01)
    sleep(500)
@@ -56,7 +56,7 @@
    end
    sleep(500)
 
-   click_on_field(map.player_slots[1].starting_field.brn)
+   click_on_field(sf.brn)
 
    message_box_objective(plr, lumberjack_message_04)
 
@@ -206,7 +206,7 @@
    -- Showoff direct roadbuilding
    click_on_field(first_quarry_field.brn)
    click_on_panel(wl.ui.MapView().windows.field_action.buttons.build_road, 300)
-   click_on_field(map.player_slots[1].starting_field.brn)
+   click_on_field(sf.brn)
 
    sleep(3000)
 
@@ -224,21 +224,20 @@
       else return false end
    end
 
-   -- Give the player some time to build the road
-   -- It is not possible to check for the road. See https://bugs.launchpad.net/widelands/+bug/1380286
+   -- Wait till the construction site is connected to the headquarters
    sleep(20*1000)
+   while first_quarry_field.brn.immovable.debug_economy ~= sf.brn.immovable.debug_economy do
+      message_box_objective(plr,quarry_not_connected)
+      sleep(60*1000)
+      if not first_quarry_field.immovable then message_box_objective(plr,quarry_illegally_destroyed) return end
+   end
 
    second_quarry()
 
-   -- Wait a while
-   sleep(60*1000)
-   -- When the said bug is fixed, check every 30 seconds if the second quarry is connected. Inform the player if not.
-   -- When that is finally done (and 30 seconds have passed), go on
-
    -- Interludium: talk about census and statistics
    census_and_statistics()
 
-   while #plr:get_buildings("quarry") < 1 do sleep(1400) end
+   while #plr:get_buildings("quarry") < 2 do sleep(1400) end
    o.done = true
 
    messages()
@@ -265,6 +264,13 @@
    -- Wait for the constructionsite to be placed
    while not cs do sleep(200) end
 
+   sleep(60*1000)
+   while second_quarry_field.brn.immovable.debug_economy ~= sf.brn.immovable.debug_economy do
+      message_box_objective(plr,quarry_not_connected)
+      sleep(60*1000)
+      if not second_quarry_field.immovable then message_box_objective(plr,quarry_illegally_destroyed) return end
+   end
+
    o.done = true
    register_immovable_as_allowed(cs)
 end

=== modified file 'campaigns/tutorial01_basic_control.wmf/scripting/texts.lua'
--- campaigns/tutorial01_basic_control.wmf/scripting/texts.lua	2015-01-24 11:43:42 +0000
+++ campaigns/tutorial01_basic_control.wmf/scripting/texts.lua	2015-06-01 20:08:53 +0000
@@ -338,6 +338,24 @@
    )
 }
 
+quarry_not_connected = {
+   title = _"Quarry not Connected",
+   body = rt(
+      p(_[[Your workers do not like to walk across country. You have to build a road from your headquarters to the construction site so that carriers can transport wares. The simplest way is to click on the construction site’s flag, choose ‘Build road’, and then click on the destination flag (the one in front of your headquarters), just like I’ve demonstrated.]])
+   ),
+   w = 350,
+   h = 250
+}
+
+quarry_illegally_destroyed = {
+   title = _"You Destroyed the Construction Site!",
+   body = rt(
+      p(_[[It seems you destroyed a construction site for a quarry we wanted to build. Since we need the stones, I suggest you reload the game from a previous savegame. Luckily, the are created from time to time. To do so, you have to go back to the main menu and choose ‘Single Player’ → ‘Load Game’. And please be a bit more careful next time.]])
+   ),
+   w = 350,
+   h = 250
+}
+
 build_second_quarry = {
    position = "topright",
    title = _"Build a second quarry",

=== modified file 'campaigns/tutorial02_warfare.wmf/scripting/mission_thread.lua'
--- campaigns/tutorial02_warfare.wmf/scripting/mission_thread.lua	2014-10-17 08:38:24 +0000
+++ campaigns/tutorial02_warfare.wmf/scripting/mission_thread.lua	2015-06-01 20:08:53 +0000
@@ -3,7 +3,7 @@
 -- ================
 
 function intro()
-   sleep(200)
+   sleep(1000)
    message_box_objective(plr, introduction)
 
    training()

=== modified file 'campaigns/tutorial03_seafaring.wmf/scripting/mission_thread.lua'
--- campaigns/tutorial03_seafaring.wmf/scripting/mission_thread.lua	2014-10-17 08:38:24 +0000
+++ campaigns/tutorial03_seafaring.wmf/scripting/mission_thread.lua	2015-06-01 20:08:53 +0000
@@ -36,8 +36,10 @@
 
    local o = message_box_objective(plr, tell_about_ships)
 
-   -- we cannot check for ships yet (see https://bugs.launchpad.net/widelands/+bug/1380287), so we just wait some time and hope for the best
-   sleep(25*60*1000) -- 25 minutes
+   -- we only wait for one ship and a bit longer because it takes long enough
+   while #plr:get_ships() < 1 do sleep(30*1000) end
+   sleep(5*60*1000)
+
    o.done = true
 
    expedition()
@@ -48,8 +50,14 @@
    message_box_objective(plr, expedition1)
    local o = message_box_objective(plr, expedition2)
 
-   -- again, we can only wait. Better a bit too long than too short
-   sleep(3*60*1000) -- 3 minutes
+   local function _ship_ready_for_expedition()
+      for k,ship in ipairs(plr:get_ships()) do
+         if ship.state == "exp_waiting" then return true end
+      end
+      return false
+   end
+
+   while not _ship_ready_for_expedition() do sleep(1000) end
    o.done = true
 
    local o2 = message_box_objective(plr, expedition3)

=== modified file 'campaigns/tutorial03_seafaring.wmf/scripting/texts.lua'
--- campaigns/tutorial03_seafaring.wmf/scripting/texts.lua	2014-11-11 10:31:01 +0000
+++ campaigns/tutorial03_seafaring.wmf/scripting/texts.lua	2015-06-01 20:08:53 +0000
@@ -161,8 +161,7 @@
    title = _"Off to greener pastures",
    body = rt(
       h1(_"Start your expedition") ..
-      p(_[[Your expedition should be ready about now. If not, please wait a bit – you will receive a message.]]) ..
-		p(_[[Your ship will be waiting for your orders in front of your port. It won’t be transporting wares anymore. Use its buttons to send your ship in any of the six main directions of the Widelands map. When it has reached a coastline, you can make it travel around the coast, where it will look for suitable places for landing.]] .. " " ..
+		p(_[[Your expedition ship is ready. It is waiting for your orders in front of your port. It isn’t transporting wares anymore. Use its buttons to send your ship in any of the six main directions of the Widelands map. When it has reached a coastline, you can make it travel around the coast, where it will look for suitable places for landing.]] .. " " ..
 		_[[Once a port space has been found, you can construct a new port with the button in the center of the ship’s control window.]]) ..
       p(_[[The wares will then be unloaded, and the ship will take up the task of transporting wares once again. The builder will start his work and build a port.]]) ..
       paragraphdivider() ..

=== modified file 'campaigns/tutorial04_economy.wmf/scripting/helper_functions.lua'
--- campaigns/tutorial04_economy.wmf/scripting/helper_functions.lua	2014-10-29 07:55:28 +0000
+++ campaigns/tutorial04_economy.wmf/scripting/helper_functions.lua	2015-06-01 20:08:53 +0000
@@ -15,7 +15,7 @@
          if bdescr.soldiers then
             b:set_soldiers(bdescr.soldiers)
          else
-            -- TODO(wl-zocker): this creates "1 soldier (+4)", but I want a capacity of 1 (i.e. "1 soldier")
+            -- TODO(wl-zocker): this creates "1 soldier (+4)", but I want a capacity of 1 (i.e. "1 soldier"). See https://bugs.launchpad.net/widelands/+bug/1387310
             b:set_soldiers({0,0,0,0}, 1)
          end
       elseif bdescr.soldiers then -- Must be a warehouse

=== modified file 'campaigns/tutorial04_economy.wmf/scripting/mission_thread.lua'
--- campaigns/tutorial04_economy.wmf/scripting/mission_thread.lua	2014-10-18 16:30:47 +0000
+++ campaigns/tutorial04_economy.wmf/scripting/mission_thread.lua	2015-06-01 20:08:53 +0000
@@ -3,7 +3,7 @@
 -- ===============
 
 function introduction()
-   sleep(500)
+   sleep(1000)
    message_box_objective(plr, intro1)
    message_box_objective(plr, intro2)