widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

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.