Review: Needs Fixing

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

Diff comments:

> === 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 16:12:33 +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 16:12:33 
> +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 16:12:33 
> +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 16:12:33 +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.brn.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.brn.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 16:12:33 +0000
> @@ -338,6 +338,24 @@
>     )
>  }
>  
> +quarry_not_connected = {
> +   title = _"Quarry not connected",
> +   body = rt(

Quarry not Connected

> +      p(_[[Your workers do not like to walk across country. You have to 
> build a road from your headquartes to the construction site for that carriers 
> can transport wares. The simplest way is to click on the flag of the 
> construction site, choose ‘Build road’, and then click on the destination 
> flag (the one in front of your headquarters), just like I’ve demonstrated.]])

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.]])

It seems you destroyed a construction site for a quarry that 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 16:12:33 +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 16:12:33 +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,v in ipairs(plr:get_ships()) do
> +         if v.state == "exp_waiting" then return k end
> +      end
> +      return nil
> +   end

How about "return true" and "return false" instead of "return k" and "return 
nil"? This would make it clearer when reading the code that this function is 
used for its boolean value.

> +
> +   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 
> 16:12:33 +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 16:12:33 +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 16:12:33 +0000
> @@ -3,7 +3,7 @@
>  -- ===============
>  
>  function introduction()
> -   sleep(500)
> +   sleep(1000)
>     message_box_objective(plr, intro1)
>     message_box_objective(plr, intro2)
>  
> 


-- 
https://code.launchpad.net/~widelands-dev/widelands/tutorial-improvements/+merge/260656
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/tutorial-improvements 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

Reply via email to