widelands

Merge lp://staging/~widelands-dev/widelands/new-tutorials into lp://staging/widelands

  1. new-tutorials
  2. Merge into trunk
Proposed by wl-zocker
Status: Merged
Merged at revision: 7236
Proposed branch: lp://staging/~widelands-dev/widelands/new-tutorials
Merge into: lp://staging/widelands
Diff against target: 155489 lines (+86758/-41659)
220 files modified
campaigns/atl01.wmf/scripting/texts.lua (+3/-3)
campaigns/bar01.wmf/elemental (+9/-8)
campaigns/bar01.wmf/extra_data (+3/-1)
campaigns/bar01.wmf/player_names (+7/-3)
campaigns/bar01.wmf/player_position (+4/-2)
campaigns/bar01.wmf/scripting/helper_functions.lua (+22/-0)
campaigns/bar01.wmf/scripting/init.lua (+13/-10)
campaigns/bar01.wmf/scripting/mission_thread.lua (+120/-220)
campaigns/bar01.wmf/scripting/secret_village.lua (+158/-0)
campaigns/bar01.wmf/scripting/starting_conditions.lua (+17/-22)
campaigns/bar01.wmf/scripting/texts.lua (+351/-263)
campaigns/bar02.wmf/elemental (+1/-1)
campaigns/bar02.wmf/scripting/init.lua (+2/-2)
campaigns/bar02.wmf/scripting/texts.lua (+40/-119)
campaigns/campaigns.conf (+6/-12)
campaigns/emp01.wmf/scripting/mission_thread.lua (+2/-2)
campaigns/emp01.wmf/scripting/starting_conditions.lua (+1/-1)
campaigns/emp01.wmf/scripting/texts.lua (+9/-10)
campaigns/emp02.wmf/scripting/mission_thread.lua (+7/-7)
campaigns/emp02.wmf/scripting/starting_conditions.lua (+5/-5)
campaigns/emp02.wmf/scripting/texts.lua (+4/-4)
campaigns/t01.wmf/elemental (+0/-10)
campaigns/t01.wmf/extra_data (+0/-2)
campaigns/t01.wmf/player_names (+0/-6)
campaigns/t01.wmf/player_position (+0/-3)
campaigns/t01.wmf/scripting/init.lua (+0/-71)
campaigns/t01.wmf/scripting/initial_messages.lua (+0/-41)
campaigns/t01.wmf/scripting/khantrukhs_talking.lua (+0/-63)
campaigns/t01.wmf/scripting/story_messages.lua (+0/-20)
campaigns/t01.wmf/scripting/texts.lua (+0/-173)
campaigns/tutorial01_basic_control.wmf/elemental (+9/-9)
campaigns/tutorial01_basic_control.wmf/extra_data (+3/-1)
campaigns/tutorial01_basic_control.wmf/player_names (+7/-9)
campaigns/tutorial01_basic_control.wmf/player_position (+4/-4)
campaigns/tutorial01_basic_control.wmf/scripting/helper_functions.lua (+32/-0)
campaigns/tutorial01_basic_control.wmf/scripting/helper_functions_demonstration.lua (+159/-0)
campaigns/tutorial01_basic_control.wmf/scripting/init.lua (+35/-0)
campaigns/tutorial01_basic_control.wmf/scripting/mission_thread.lua (+189/-598)
campaigns/tutorial01_basic_control.wmf/scripting/starting_conditions.lua (+10/-0)
campaigns/tutorial01_basic_control.wmf/scripting/texts.lua (+280/-221)
campaigns/tutorial02_warfare.wmf/elemental (+10/-0)
campaigns/tutorial02_warfare.wmf/extra_data (+4/-0)
campaigns/tutorial02_warfare.wmf/objective (+4/-0)
campaigns/tutorial02_warfare.wmf/player/1/messages (+4/-0)
campaigns/tutorial02_warfare.wmf/player/2/messages (+4/-0)
campaigns/tutorial02_warfare.wmf/player_names (+16/-0)
campaigns/tutorial02_warfare.wmf/player_position (+6/-0)
campaigns/tutorial02_warfare.wmf/port_spaces (+7/-0)
campaigns/tutorial02_warfare.wmf/scripting/init.lua (+20/-0)
campaigns/tutorial02_warfare.wmf/scripting/mission_thread.lua (+105/-0)
campaigns/tutorial02_warfare.wmf/scripting/starting_conditions.lua (+69/-0)
campaigns/tutorial02_warfare.wmf/scripting/texts.lua (+213/-0)
campaigns/tutorial02_warfare.wmf/version (+11/-0)
campaigns/tutorial03_seafaring.wmf/elemental (+10/-0)
campaigns/tutorial03_seafaring.wmf/extra_data (+4/-0)
campaigns/tutorial03_seafaring.wmf/objective (+4/-0)
campaigns/tutorial03_seafaring.wmf/player/1/messages (+4/-0)
campaigns/tutorial03_seafaring.wmf/player_names (+10/-0)
campaigns/tutorial03_seafaring.wmf/player_position (+5/-0)
campaigns/tutorial03_seafaring.wmf/port_spaces (+11/-0)
campaigns/tutorial03_seafaring.wmf/scripting/helper_functions.lua (+28/-0)
campaigns/tutorial03_seafaring.wmf/scripting/init.lua (+28/-0)
campaigns/tutorial03_seafaring.wmf/scripting/mission_thread.lua (+84/-0)
campaigns/tutorial03_seafaring.wmf/scripting/starting_conditions.lua (+164/-0)
campaigns/tutorial03_seafaring.wmf/scripting/texts.lua (+194/-0)
campaigns/tutorial03_seafaring.wmf/version (+11/-0)
campaigns/tutorial04_economy.wmf/elemental (+10/-0)
campaigns/tutorial04_economy.wmf/extra_data (+4/-0)
campaigns/tutorial04_economy.wmf/objective (+4/-0)
campaigns/tutorial04_economy.wmf/player/1/messages (+4/-0)
campaigns/tutorial04_economy.wmf/player/2/messages (+4/-0)
campaigns/tutorial04_economy.wmf/player_names (+16/-0)
campaigns/tutorial04_economy.wmf/player_position (+6/-0)
campaigns/tutorial04_economy.wmf/port_spaces (+7/-0)
campaigns/tutorial04_economy.wmf/scripting/helper_functions.lua (+29/-0)
campaigns/tutorial04_economy.wmf/scripting/init.lua (+31/-0)
campaigns/tutorial04_economy.wmf/scripting/mission_thread.lua (+140/-0)
campaigns/tutorial04_economy.wmf/scripting/starting_conditions.lua (+171/-0)
campaigns/tutorial04_economy.wmf/scripting/texts.lua (+410/-0)
campaigns/tutorial04_economy.wmf/version (+11/-0)
campaigns/tutorials.conf (+28/-0)
po/map_plateau.wmf/map_plateau.wmf.pot (+1/-1)
po/maps/maps.pot (+61/-109)
po/mp_scenario_island_hopping.wmf/mp_scenario_island_hopping.wmf.pot (+1/-1)
po/mp_scenario_smugglers.wmf/mp_scenario_smugglers.wmf.pot (+1/-1)
po/scenario_atl01.wmf/scenario_atl01.wmf.pot (+5/-5)
po/scenario_bar01.wmf/cs.po (+1669/-510)
po/scenario_bar01.wmf/da.po (+532/-270)
po/scenario_bar01.wmf/de.po (+1843/-570)
po/scenario_bar01.wmf/en_GB.po (+622/-300)
po/scenario_bar01.wmf/eo.po (+1067/-0)
po/scenario_bar01.wmf/es.po (+652/-310)
po/scenario_bar01.wmf/fi.po (+583/-373)
po/scenario_bar01.wmf/fr.po (+1861/-571)
po/scenario_bar01.wmf/gd.po (+1820/-444)
po/scenario_bar01.wmf/gl.po (+651/-310)
po/scenario_bar01.wmf/hu.po (+1641/-499)
po/scenario_bar01.wmf/it.po (+1720/-523)
po/scenario_bar01.wmf/ja.po (+1643/-276)
po/scenario_bar01.wmf/la.po (+1025/-0)
po/scenario_bar01.wmf/lt.po (+995/-0)
po/scenario_bar01.wmf/nb.po (+986/-327)
po/scenario_bar01.wmf/nl.po (+1789/-530)
po/scenario_bar01.wmf/nn.po (+1058/-0)
po/scenario_bar01.wmf/pl.po (+1770/-535)
po/scenario_bar01.wmf/pt.po (+1671/-519)
po/scenario_bar01.wmf/pt_BR.po (+1675/-507)
po/scenario_bar01.wmf/ru.po (+861/-313)
po/scenario_bar01.wmf/scenario_bar01.wmf.pot (+517/-290)
po/scenario_bar01.wmf/si.po (+511/-266)
po/scenario_bar01.wmf/sk.po (+1619/-497)
po/scenario_bar01.wmf/sv.po (+1625/-493)
po/scenario_bar01.wmf/vi.po (+1011/-0)
po/scenario_bar02.wmf/scenario_bar02.wmf.pot (+127/-127)
po/scenario_dummy.wmf/scenario_dummy.wmf.pot (+2/-2)
po/scenario_emp01.wmf/scenario_emp01.wmf.pot (+112/-117)
po/scenario_emp02.wmf/scenario_emp02.wmf.pot (+6/-6)
po/scenario_t01.wmf/cs.po (+0/-525)
po/scenario_t01.wmf/da.po (+0/-425)
po/scenario_t01.wmf/de.po (+0/-548)
po/scenario_t01.wmf/en_GB.po (+0/-426)
po/scenario_t01.wmf/eo.po (+0/-426)
po/scenario_t01.wmf/es.po (+0/-428)
po/scenario_t01.wmf/fi.po (+0/-448)
po/scenario_t01.wmf/fr.po (+0/-543)
po/scenario_t01.wmf/gd.po (+0/-550)
po/scenario_t01.wmf/gl.po (+0/-429)
po/scenario_t01.wmf/he.po (+0/-425)
po/scenario_t01.wmf/hu.po (+0/-525)
po/scenario_t01.wmf/it.po (+0/-547)
po/scenario_t01.wmf/ja.po (+0/-426)
po/scenario_t01.wmf/nb.po (+0/-468)
po/scenario_t01.wmf/nl.po (+0/-536)
po/scenario_t01.wmf/pl.po (+0/-524)
po/scenario_t01.wmf/pt.po (+0/-530)
po/scenario_t01.wmf/pt_BR.po (+0/-537)
po/scenario_t01.wmf/ru.po (+0/-534)
po/scenario_t01.wmf/scenario_t01.wmf.pot (+0/-389)
po/scenario_t01.wmf/si.po (+0/-426)
po/scenario_t01.wmf/sk.po (+0/-516)
po/scenario_t01.wmf/sv.po (+0/-529)
po/scenario_t01.wmf/vi.po (+0/-435)
po/scenario_t02.wmf/he.po (+0/-718)
po/scenario_tutorial01_basic_control.wmf/cs.po (+835/-655)
po/scenario_tutorial01_basic_control.wmf/de.po (+943/-701)
po/scenario_tutorial01_basic_control.wmf/el.po (+473/-593)
po/scenario_tutorial01_basic_control.wmf/en_GB.po (+486/-593)
po/scenario_tutorial01_basic_control.wmf/eo.po (+520/-587)
po/scenario_tutorial01_basic_control.wmf/es.po (+528/-593)
po/scenario_tutorial01_basic_control.wmf/fi.po (+468/-594)
po/scenario_tutorial01_basic_control.wmf/fr.po (+951/-709)
po/scenario_tutorial01_basic_control.wmf/gd.po (+952/-710)
po/scenario_tutorial01_basic_control.wmf/gl.po (+528/-593)
po/scenario_tutorial01_basic_control.wmf/hu.po (+886/-677)
po/scenario_tutorial01_basic_control.wmf/it.po (+871/-677)
po/scenario_tutorial01_basic_control.wmf/ja.po (+1095/-729)
po/scenario_tutorial01_basic_control.wmf/la.po (+518/-593)
po/scenario_tutorial01_basic_control.wmf/lt.po (+479/-595)
po/scenario_tutorial01_basic_control.wmf/nb.po (+594/-597)
po/scenario_tutorial01_basic_control.wmf/nl.po (+913/-689)
po/scenario_tutorial01_basic_control.wmf/nn.po (+528/-593)
po/scenario_tutorial01_basic_control.wmf/pl.po (+913/-677)
po/scenario_tutorial01_basic_control.wmf/pt.po (+849/-673)
po/scenario_tutorial01_basic_control.wmf/pt_BR.po (+854/-674)
po/scenario_tutorial01_basic_control.wmf/ru.po (+541/-597)
po/scenario_tutorial01_basic_control.wmf/scenario_tutorial01_basic_control.wmf.pot (+442/-554)
po/scenario_tutorial01_basic_control.wmf/sk.po (+828/-652)
po/scenario_tutorial01_basic_control.wmf/sv.po (+849/-664)
po/scenario_tutorial01_basic_control.wmf/tr.po (+488/-593)
po/scenario_tutorial01_basic_control.wmf/uk.po (+476/-595)
po/scenario_tutorial02_warfare.wmf/cs.po (+1561/-0)
po/scenario_tutorial02_warfare.wmf/de.po (+1695/-0)
po/scenario_tutorial02_warfare.wmf/el.po (+544/-0)
po/scenario_tutorial02_warfare.wmf/en_GB.po (+604/-0)
po/scenario_tutorial02_warfare.wmf/eo.po (+663/-0)
po/scenario_tutorial02_warfare.wmf/es.po (+653/-0)
po/scenario_tutorial02_warfare.wmf/fi.po (+553/-0)
po/scenario_tutorial02_warfare.wmf/fr.po (+1712/-0)
po/scenario_tutorial02_warfare.wmf/gd.po (+1712/-0)
po/scenario_tutorial02_warfare.wmf/gl.po (+655/-0)
po/scenario_tutorial02_warfare.wmf/hu.po (+1512/-0)
po/scenario_tutorial02_warfare.wmf/it.po (+1613/-0)
po/scenario_tutorial02_warfare.wmf/ja.po (+1589/-0)
po/scenario_tutorial02_warfare.wmf/la.po (+619/-0)
po/scenario_tutorial02_warfare.wmf/nb.po (+963/-0)
po/scenario_tutorial02_warfare.wmf/nl.po (+1648/-0)
po/scenario_tutorial02_warfare.wmf/nn.po (+648/-0)
po/scenario_tutorial02_warfare.wmf/pl.po (+1623/-0)
po/scenario_tutorial02_warfare.wmf/pt.po (+1567/-0)
po/scenario_tutorial02_warfare.wmf/pt_BR.po (+1578/-0)
po/scenario_tutorial02_warfare.wmf/ru.po (+865/-0)
po/scenario_tutorial02_warfare.wmf/scenario_tutorial02_warfare.wmf.pot (+473/-0)
po/scenario_tutorial02_warfare.wmf/sk.po (+1517/-0)
po/scenario_tutorial02_warfare.wmf/sv.po (+1522/-0)
po/scenario_tutorial02_warfare.wmf/tr.po (+577/-0)
po/scenario_tutorial02_warfare.wmf/uk.po (+548/-0)
po/scenario_tutorial03_seafaring.wmf/scenario_tutorial03_seafaring.wmf.pot (+404/-0)
po/scenario_tutorial04_economy.wmf/scenario_tutorial04_economy.wmf.pot (+712/-0)
po/texts/texts.pot (+44/-39)
po/tribe_atlanteans/tribe_atlanteans.pot (+1/-1)
po/tribe_barbarians/tribe_barbarians.pot (+1/-1)
po/tribe_empire/tribe_empire.pot (+1/-1)
po/tribes/tribes.pot (+1/-1)
po/widelands/widelands.pot (+65/-23)
po/widelands_console/widelands_console.pot (+1/-1)
po/win_conditions/win_conditions.pot (+1/-1)
po/world/world.pot (+1/-1)
scripting/coroutine.lua (+11/-11)
scripting/format_scenario.lua (+7/-6)
scripting/formatting.lua (+4/-4)
scripting/messages.lua (+178/-0)
scripting/set.lua (+18/-18)
scripting/table.lua (+2/-2)
scripting/ui.lua (+14/-1)
src/logic/campaign_visibility.cc (+3/-3)
src/ui_fsmenu/campaign_select.cc (+76/-35)
src/ui_fsmenu/campaign_select.h (+4/-2)
src/wlapplication.cc (+29/-10)
src/wlapplication.h (+1/-0)
txts/developers (+7/-0)
To merge this branch: bzr merge lp://staging/~widelands-dev/widelands/new-tutorials
Reviewer Review Type Date Requested Status
SirVer Approve
GunChleoc Approve
wl-zocker Needs Resubmitting
Review via email: mp+238682@code.staging.launchpad.net

Description of the change

This branch contains four tutorials that will replace the old one. I have removed the first Barbarian campaign because it did not introduce anything new. The story can now be found in bar01.wmf (the former t02.wmf). I have also added a new Lua function message_box_objective in message.lua.

Some links:
- There has been some discussion in our forum: https://wl.widelands.org/forum/topic/1560/
- The Lua interface misses some functionality. See bug 1380286, bug 1380287 and bug 1380288. If they could be implemented before merging, that would be cool. Otherwise, we ship the tutorials without these features and introduce them later.
- The tutorials should get their own menu and be moved to the campaigns folder (they currently are in maps/Tutorials for easy testing). This can be added when https://code.launchpad.net/~widelands-dev/widelands/i18nfixes has been merged.
- I would like to fix the paragraphdivider() (bug 1366580) differently, so please leave this bug open. This branch only fixes the symptoms. I did not fix it to avoid unnecessary conflicts with https://code.launchpad.net/~widelands-dev/widelands/bug-1298301 (and to avoid the diff growing bigger and bigger).

And there are some "TODO(codereview):" in the code. Just grep for them.

To post a comment you must log in.
Revision history for this message
GunChleoc (gunchleoc) wrote :

For now some quick answers to the code review comments:

- The campaign file version should be updated before we merge, because we have removed a scenario. packet_version entries stay as they are.

- campsect0=barbariantut tells you that the first campaign is the barbariantut campaign (computers like to start counting from 0). Scenarios are then loaded though the [barbariantut00] etc. sections. So, campsect0=barbariantut stays as it is, and the two remaining scenarios need to be [barbariantut00] and [barbariantut01].

- non-breaking spaces: AFAIK, our RT renderer does not support HTML entities. It would really be nice to have some, because then I would have non-breaking hyphens in my translations :)

I'll test and proofread.

Revision history for this message
GunChleoc (gunchleoc) wrote :

I have proofread and fixed some stuff and then playtested the tutorials. We still have the following issues to resolve:

Economy: function _try_add_objective doesn't exist. What was it supposed to do?

Basic control:

1. We should bzr move campaigns/tutorial01.wmf to Basic Control.

2. Objective "connect the quarry to the headquarters" is never achieved. Since we don't have access to roads yet, you could check for the quarry being constructed.

3. I accidentally built a lumberjack instead of the second quarry. Tutorial tore it down, like it's supposed to. Then the call to census_and_statistics(cs.fields[1]) failed

[/home/bratzbert/sources/widelands/new-tutorials/src/scripting/lua_errors.h:29] BaseImmovable no longer exists!
stack traceback:
 [string "map:scripting/mission_thread.lua"]:238: in function 'build_a_quarry'
 [string "map:scripting/mission_thread.lua"]:143: in function 'learn_to_move'
 [string "map:scripting/mission_thread.lua"]:106: in function 'build_lumberjack'
 [string "map:scripting/mission_thread.lua"]:25: in function <[string "map:scripting/mission_thread.lua"]:5>

I have yet to look at the campaign.

review: Needs Fixing
Revision history for this message
wl-zocker (wl-zocker) wrote :

Thanks for all that proofreading and the other fixes. I will have a closer look at them later.

Economy, _try_add_objective: That was the name used in the former tutorial. It should now be replaced by add_campaign_objective. I did find&replace, but did not replay the whole tutorial afterwards.

1) Can I still do that after I have bzr rm it (I am not so familiar with bazaar)

2) Based on the code, this should happen here (line 240 in r7235):
while #plr:get_buildings("quarry") < 2 do sleep(1400) end
o.done = true

3) I will investigate when I have time. But that cs should refer to the first construction site.

Had the check "if panel ~= nil then" a special reason or was it just foresightful?

Revision history for this message
GunChleoc (gunchleoc) wrote :

> Had the check "if panel ~= nil then" a special reason or was it just
> foresightful?

This fixed another crash.

Revision history for this message
wl-zocker (wl-zocker) wrote :

> This fixed another crash.

Did you check that everything still works? This has never crashed so far. Normally, the given panels should all exist.

And why did you only write about sentries in tut01? The player can build other buildings, too (at least if he waits long enough for the lumberjack to cut down some trees).

Revision history for this message
GunChleoc (gunchleoc) wrote :

> > This fixed another crash.
>
> Did you check that everything still works? This has never crashed so far.
> Normally, the given panels should all exist.

Yes, I did check - at least I didn't notice any adverse effects.

> And why did you only write about sentries in tut01? The player can build other
> buildings, too (at least if he waits long enough for the lumberjack to cut
> down some trees).

Oops, I thought that the sentry was the only building in the allowed list. I thought it would be easier for the player if we are more specific here. Please feel free to change this back.

Revision history for this message
wl-zocker (wl-zocker) wrote :

The missing panel: The following happened once while testing:
I built the first quarry. At this very moment, the check "if wl.ui.MapView().is_building_road" happened, which was false at that time. Then, the game got control and entered the road building mode (as I had set in the options). My script now attempted to click on the flag to start road building manually, but was presented with the "Cancel road" menu. In this menu, it tried to click on "Start road building", which failed.
I have added this check to deal more cleverly with the option the user can set (wasn't there some discussion about removing it?). For now, I sleep additional 100 ms and hope this is fixed.

1) bzr move: I did bzr mv the old tutorial to 01_Basic_control because most parts came from there. If bazaar can put two files together/split a file in two and keeping the history, that would be great, but I do not know such a feature.
2) As I wrote, the "Connect quarry" objective is achived after both quarries are completed. I am not really happy with this, but the only other possibility is to set this done after a certain amount of time. I have no information about the construction site (its progress).
3) Crash with cs.fields[1]: Both construction sites have their own "local cs" variable, so I have no idea how they shall interact (the cs.fields[1] refers to the first construction site). I have been able to reproduce it, but after some code changing, it did not always happen. My only idea was that the construction site was finished in meantime, but when I tried to reproduce it, I got "MapObject no longer exists!" (and not BaseImmovable). I now pass first_quarry_field to the function. Thinking about it, I could also hardcode it there and do not pass anything at all.

Revision history for this message
GunChleoc (gunchleoc) wrote :

2. How about checking for just 1 of the quarries to complete? The user needs to connect the roads for both of them, so we can assume that if he can manage one, he will manage the other.

3. I had another look at the function. Since it only needs a random empty field, hard coding the start location for _pick_empty_field shouldn't be a problem.

Revision history for this message
wl-zocker (wl-zocker) wrote :

I changed some strings, please review them.
I hope I fixed all issues that we discovered so far.

The issue with the missing panel happened again (despite my sleep(100)). I have no idea why.

Revision history for this message
GunChleoc (gunchleoc) wrote :

You have now completely replaced_pick_empty_field() with first_quarry_field.brn. This means we won't be getting an empty field now. I think the idea here was to pick an empty field on purpose, so that the menu that comes up will be smaller. I suggest we use get_field() to grab a field near the quarry that is empty.

sleep() doesn't guarantee anything about the state of variables or panels. Strange that it happened again, since we do check that it's not nil now. Do you have the full error message?

String changes LGTM, exccept for 1 tiny nit - see comment below.

Revision history for this message
wl-zocker (wl-zocker) wrote :

We can also use the bln - I think it should be empty in every case. I am not sure if the old way is 100% errorproof: What if all fields in the region are cluttered with roads? The while loop could run forever. I thought it would be illustrative to show the correct tab, too. The old way could also pick a spot where flags are buildable, and change the tab.

I got no crash, just the message on my terminal. It only looks odd that the mouse clicks on the flag although not necessary and the "Cancel road" window appears. I have no idea why the game behaves like that.

Revision history for this message
wl-zocker (wl-zocker) wrote :

As far as I can see, everything found until now has been fixed.
I still have no clue about that panel thing. I have added a TODO(codereview) and hope someone else has an idea. I have not been able to reproduce it within 20 tries, so maybe a lot of luck is involved there.
@GunChleoc: Did you also proofread the map descriptions? I have also set the textdomains. If I am not mistaken, they can be set to everything. Feel free to alter them.
Note to myself: Add the new template names to https://wl.widelands.org/wiki/TranslatingWidelands/

This branch still needs to wait for https://code.launchpad.net/~widelands-dev/widelands/i18nfixes, but in the meantime, I can do other changes to the code (if someone tells me what is not optimal yet).

review: Needs Resubmitting
Revision history for this message
GunChleoc (gunchleoc) wrote :

LTGM

I will have another look a the panel thing when I get back to my dev machine.

Don't worry about the text domains, I will take care of everything including the wiki once I make the new templates visible to translators. There will be some merging involved from my side so we won't lose old translations.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Fixed the road building mode + panel problem - you simply didn't sleep long enough.

Good point about proofreading the descriptions, I had forgotten those.

Revision history for this message
GunChleoc (gunchleoc) wrote :

I think we are done here. Could somebody please check my part of the code and do some additional playtesting?

I still need to create the gettext catalog files, but I'll do that during the merge, so we won't use any translations. I will have to run msgmerge quite a lot ;)

review: Approve
Revision history for this message
wl-zocker (wl-zocker) wrote :

Thank you for working on this. Unfortunally, I do not have much time, so I cannot test it now.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Don't worry, it is more likely anyway that any bugs will surface if somebody who hasn't worked on the code will test this.

Revision history for this message
SirVer (sirver) wrote :

I found one bug:

- secret_village.lua:17 attempt to call global send_msg, a nil value.

I fixed it in 7251.

I didn't look over all the code as Gun did that already. What I saw looked good (local variables by default mainly and consistent style). The campaigns and tutorials are a lot of fun to play, I really enjoyed this. Thanks for your work on the campaigns - should you want to do more, just ask :).

review: Approve
Revision history for this message
GunChleoc (gunchleoc) wrote :

Thanks for the fix. I'll merge this tomorrow.

Revision history for this message
wl-zocker (wl-zocker) wrote :

There are still three "TODO(codereview)":

campaigns/tutorial04_economy.wmf/scripting/helper_functions.lua
-- TODO(codereview): this creates "1 soldier (+4)", but I want a capacity of 1 (i.e. "1 soldier")
b:set_soldiers({0,0,0,0}, 1)
Probably not possible.

campaigns/tutorial04_economy.wmf/scripting/texts.lua
-- TODO(codereview): Is there a place where the player can see the needed weapons?
This would avoid that this information comes "out of nothing".

-- TODO(codereview): Is this how the "Preferably store here" option works? (change the text below, too)
listitem_arrow(_[[... All marble columns produced in the future will be brought there.]])
I do not know the code, and I do not want to state something wrong. The obj_body contains the same line.

The comments should be removed before merging.

@SirVer: In the near future, I will not have the time to do big things. Maybe in some months.

Revision history for this message
GunChleoc (gunchleoc) wrote :

1. We would need to extend the Lua interface for that. Let's keep it as TODO(wl-zocker) for now. Open a new bug?

2. No. Comment removed

3. I had a look at the code and added "if possible" to the text. It is how the option works, but the columns might be produced in a different economy (not the case here), or another warehouse also have them set as preference (also not the case here).

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to status/vote changes: