This patch contains
- a transcription of the "Introductory Tutorial" which is in the demos scenarios to use the new Tutorial.js script based on trigger instead of the old tutorial-ai
- and a removal of the tutorial-ai
Details
Details
- Reviewers
- None
- Group Reviewers
Restricted Owners Package (Owns No Changed Paths) - Commits
- rP20084: Remove the tutorial-ai which is obsolete
play the tutorial without the patch (i.e. using the tutorial-ai) or with (i.e. using triggers). The missions are identical, with the same strings.
Diff Detail
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Comment Actions
Build has FAILED
Link to build: http://jw:8080/job/phabricator/1270/
See console output for more information: http://jw:8080/job/phabricator/1270/console
Comment Actions
Build is green
Executing section Default... Executing section Source... Executing section JS... Executing section Python... Executing section Perl...
http://jw:8080/job/phabricator_lint/5/ for more details.
Comment Actions
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
http://jw:8080/job/phabricator/1293/ for more details.
Comment Actions
rebase patch after latest changes in Tutorial script.
This is just a retranscription of the old tutorial to the new script, to be able to remove all the obsolete tutorial-ai. A proper rewritting of the tutorial will still be needed in a later stage.
| binaries/data/mods/public/maps/scenarios/Introductory_Tutorial.js | ||
|---|---|---|
| 35 ↗ | (On Diff #3264) | entity and txt could be inlined |
| 375 ↗ | (On Diff #3264) | (early return could be moved one call upwards.) |
| 382 ↗ | (On Diff #3264) | (same as below optionally. Could even become this.attackers = GetEntities.filter().every()) |
| 393 ↗ | (On Diff #3264) | Could be // (maybe some instead of every, maybe a negation missing)
return this.attackers.every(ent => {
let cmpHealth = Engine.QueryInterface(ent, IID_Health);
return cmpHealth && cmpHealth.GetHitpoints() > 0;
});(also exists immediately once one item matches) |
| 411 ↗ | (On Diff #3264) | (DestroyEntity most often wants some StatisticsTracker call, but not needed here as we don't track gaia (yet)) |
| 419 ↗ | (On Diff #3264) | wrap L415 to end into a scope { } and use let for cmpTrigger, so that it's impossible to use cmpTrigger in the prototype functions when this should be used. (this points to the correct thing after deserialization, whereas cmpTrigger is still the prototype instance from before the deserialization, which gave a big weird OOS in survival once, was a release blocker and savegames would be broken too and so forth ) |
| binaries/data/mods/public/maps/scenarios/Introductory_Tutorial.xml | ||
| 4306 ↗ | (On Diff #3264) | Dunno if complete (someone could do a grep), but certainly correct changes. (Ownership fix could be committed separately if you want) |
| binaries/data/mods/public/maps/scripts/Tutorial.js | ||
| 28 ↗ | (On Diff #3264) | Thought we wanted to have our source in one specific format? |
| binaries/data/mods/public/simulation/ai/tutorial-ai/_init.js | ||
| 1 ↗ | (On Diff #3264) | hell yeah delete this directory! (the item in the gamesetup will disappear, right?) |
Comment Actions
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Checking XML files...
http://jw:8080/job/phabricator/1896/ for more details.
Comment Actions
Executing section Default... Executing section Source... Executing section JS... binaries/data/mods/public/maps/scripts/TriggerHelper.js | 45| » if·(owner·==·null) | | [NORMAL] JSHintBear: | | Use '===' to compare with 'null'. binaries/data/mods/public/maps/scripts/TriggerHelper.js | 206| » » if·(cmpTerritoryManager.GetOwner(pos.x,·pos.z)·==·0) | | [NORMAL] JSHintBear: | | Use '===' to compare with '0'. Executing section XML GUI... Executing section Python... Executing section Perl...
http://jw:8080/job/phabricator_lint/427/ for more details.
| binaries/data/mods/public/maps/scripts/Tutorial.js | ||
|---|---|---|
| 28 ↗ | (On Diff #3264) | yes, but that's a bit annoying when such a tutorial, when all instructions have only one line, to have to define arrays. So I've now re-allowed string. But no strong opinion about it. |
| binaries/data/mods/public/simulation/ai/tutorial-ai/_init.js | ||
| 1 ↗ | (On Diff #3264) | yes |
Comment Actions
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Checking XML files...
http://jw:8080/job/phabricator/1903/ for more details.
Comment Actions
Executing section Default... Executing section Source... Executing section JS... binaries/data/mods/public/maps/scripts/TriggerHelper.js | 45| » if·(owner·==·null) | | [NORMAL] JSHintBear: | | Use '===' to compare with 'null'. binaries/data/mods/public/maps/scripts/TriggerHelper.js | 206| » » if·(cmpTerritoryManager.GetOwner(pos.x,·pos.z)·==·0) | | [NORMAL] JSHintBear: | | Use '===' to compare with '0'. Executing section XML GUI... Executing section Python... Executing section Perl...
http://jw:8080/job/phabricator_lint/428/ for more details.
Comment Actions
Rest appears good, though I didn't test it yet.
(Got an opinion on whether the tutorial panel should become non-tranparent as proposed in #4738?)
| binaries/data/mods/public/maps/scenarios/Introductory_Tutorial.js | ||
|---|---|---|
| 1 ↗ | (On Diff #3281) | (That file should be moved from scenarios/ to tutorials/ I guess, possibly after this patch to avoid phabricator issues) |
| 367 ↗ | (On Diff #3281) | could cache cmpRangeManager.GetEntitiesByPlayer(this.playerID) |
| 370 ↗ | (On Diff #3281) | find returns the first matching entity or undefined, ( so nuke .length) Could do let target = entities.find(..."DefenseTower") || entities.find(..."CC") |
| 379 ↗ | (On Diff #3281) | every returns true if a condition is met for every item of the array (some returns true if any of the items match a condition). x => y looks like the return value is relevant, so it should be x = > { y; } (possibly newlines). |
| 402 ↗ | (On Diff #3281) | Trigger.prototype.tutorialGoals = hugeArray at the top or cmpTrigger.tutorialGoals = tutorialGoals; inside the scope seems more readable, no? (It's de/serialized in both cases afaics) |
| binaries/data/mods/public/maps/scenarios/Introductory_Tutorial.xml | ||
| 44 ↗ | (On Diff #3281) | (not sure if we want demo maps to appear when selecting the trigger maps filter. However this thing should be moved to the Tutorial folder anyhow later.) |
| binaries/data/mods/public/maps/scripts/Tutorial.js | ||
| 3 ↗ | (On Diff #3281) | ^ |
| 28 ↗ | (On Diff #3264) | If someone adds the first array, it will become inconsistent and one will have to fix the entire file. Better do it in advance. |
| binaries/data/mods/public/maps/scenarios/Introductory_Tutorial.js | ||
|---|---|---|
| 1 ↗ | (On Diff #3281) | yes, later when we have a way to deal with several tutorials, otherwise it won't be accessible in game in the meantime. |
| 367 ↗ | (On Diff #3281) | not useful as the second case will never happen (as we've just asked the player to build a tower). It's here just to avoid a crash in case the player would destroy it. But ok |
| 370 ↗ | (On Diff #3281) | sure for length |
| 379 ↗ | (On Diff #3281) | ok |
| 402 ↗ | (On Diff #3281) | ok |
| binaries/data/mods/public/maps/scenarios/Introductory_Tutorial.xml | ||
| 44 ↗ | (On Diff #3281) | yep, trigger can be removed. |
| binaries/data/mods/public/maps/scripts/Tutorial.js | ||
| 28 ↗ | (On Diff #3264) | don't get what you mean? nothing would be inconsistent if we add an array. So either it is the Tutorial.js file which accepts both format, or this is dealt with in each tutorial. I don't mind. |
Comment Actions
Executing section Default... Executing section Source... Executing section JS... binaries/data/mods/public/maps/scripts/TriggerHelper.js | 45| » if·(owner·==·null) | | [NORMAL] JSHintBear: | | Use '===' to compare with 'null'. binaries/data/mods/public/maps/scripts/TriggerHelper.js | 206| » » if·(cmpTerritoryManager.GetOwner(pos.x,·pos.z)·==·0) | | [NORMAL] JSHintBear: | | Use '===' to compare with '0'. Executing section XML GUI... Executing section Python... Executing section Perl...
http://jw:8080/job/phabricator_lint/429/ for more details.
Comment Actions
Since the tutorials are in a separate translation resource, moving the strings from sim to maps resource and from maps to tutorials resource will add some overhead to transifex.
We could add the new files in the target directory and add a new main menu entry for this tutorial and pass down the mapname to the gamesetup.
| binaries/data/mods/public/maps/scenarios/Introductory_Tutorial.js | ||
|---|---|---|
| 370 ↗ | (On Diff #3281) | x || y returns x if it can be converted to true, otherwise it returns y |
| binaries/data/mods/public/maps/scripts/Tutorial.js | ||
| 28 ↗ | (On Diff #3264) | (Was just wondering because you prefered the to remove the type distinction in the other patch) |
Comment Actions
that's ok for me. But better do it in a following patch as i already had phabricator problems when moving and modifying a file in the same patch.
Comment Actions
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Checking XML files...
http://jw:8080/job/phabricator/1904/ for more details.
Comment Actions
We should determine the the least buggy, most cost efficient (to both author and translators) solution.
Adding a dozen of strings changes, removing them again and add them in a different place will imply surplus work for the translators who might be equally annoyed by that.
If it's only the phabricator issue, I don't mind seeing a patch uploaded as a patch that deletes the old file and adds a new one, or as a file attachment, or seeing a commit where the file is put into the tutorial directory.
Comment Actions
Do you mean that if a same sentence is in two different resources, it will be translated twice? I don't know how transifex works, but that would be a serious flaw of the system.
Anyway, i really don't care how it is done: only the end result matters. We thus have two choices:
- either leave it temporarily in the scenarios folder even if it is a bit more work for translators, so that it is still accessible by players while waiting for somebody to do an interface for playing several turorials (i say on purpose somebody because it won't be me as that's still something which will lead to endless discussions).
- or do the svn-move to the tutorial folder just after commiting that one, and then it will be inaccessible as long as nobody has written the interface.
Comment Actions
add a new main menu entry for this tutorial
This way the tutorial will be accessible to players, reside in the final directory and doesn't require any discussion from my side.
Comment Actions
ok i move them them to the tutorial folder and wait for anybody to do the needed gui (hopefully that'll come before A23).
Comment Actions
No comment.
Index: binaries/data/mods/public/gui/gamesetup/gamesetup.js =================================================================== --- binaries/data/mods/public/gui/gamesetup/gamesetup.js (revision 20087) +++ binaries/data/mods/public/gui/gamesetup/gamesetup.js (working copy) @@ -211,13 +211,13 @@ var g_IsNetworked; * Is this user in control of game settings (i.e. singleplayer or host of a multiplayergame). */ var g_IsController; /** - * Whether this is a tutorial. + * If given, launch a tutorial map. */ -var g_IsTutorial; +var g_TutorialMap; /** * To report the game to the lobby bot. */ var g_ServerName; @@ -941,11 +941,11 @@ function init(attribs) return; } g_IsNetworked = attribs.type != "offline"; g_IsController = attribs.type != "client"; - g_IsTutorial = !!attribs.tutorial; + g_TutorialMap = attribs.tutorial; g_ServerName = attribs.serverName; g_ServerPort = attribs.serverPort; g_StunEndpoint = attribs.stunEndpoint; if (!g_IsNetworked) @@ -1030,11 +1030,11 @@ function initGUIObjects() loadPersistMatchSettings(); updateGameAttributes(); sendRegisterGameStanzaImmediate(); - if (g_IsTutorial) + if (g_TutorialMap) { launchTutorial(); return; } @@ -1468,11 +1468,11 @@ function loadMapData(name) /** * Sets the gameattributes the way they were the last time the user left the gamesetup. */ function loadPersistMatchSettings() { - if (!g_IsController || Engine.ConfigDB_GetValue("user", "persistmatchsettings") != "true" || g_IsTutorial) + if (!g_IsController || Engine.ConfigDB_GetValue("user", "persistmatchsettings") != "true" || g_TutorialMap) return; let settingsFile = g_IsNetworked ? g_MatchSettings_MP : g_MatchSettings_SP; if (!Engine.FileExists(settingsFile)) return; @@ -1523,11 +1523,11 @@ function loadPersistMatchSettings() g_IsInGuiUpdate = false; } function savePersistMatchSettings() { - if (g_IsTutorial) + if (g_TutorialMap) return; let attributes = Engine.ConfigDB_GetValue("user", "persistmatchsettings") == "true" ? g_GameAttributes : {}; Engine.WriteJSONFile(g_IsNetworked ? g_MatchSettings_MP : g_MatchSettings_SP, attributes); } @@ -1880,11 +1880,11 @@ function launchGame() } function launchTutorial() { g_GameAttributes.mapType = "scenario"; - selectMap("maps/tutorials/starting_economy_walkthrough"); + selectMap("maps/tutorials/" + g_TutorialMap); launchGame(); } /** * Don't set any attributes here, just show the changes in the GUI. Index: binaries/data/mods/public/gui/pregame/mainmenu.xml =================================================================== --- binaries/data/mods/public/gui/pregame/mainmenu.xml (revision 20087) +++ binaries/data/mods/public/gui/pregame/mainmenu.xml (working copy) @@ -154,29 +154,43 @@ "url": "http://trac.wildfiregames.com/wiki/0adManual" }); </action> </object> - <!-- START TUTORIAL BUTTON --> + <!-- INTRODUCTORY TUTORIAL BUTTON --> <object type="button" style="StoneButtonFancy" size="0 32 100% 60" tooltip_style="pgToolTip" > - <translatableAttribute id="caption">Tutorial</translatableAttribute> + <translatableAttribute id="caption">Introductory Tutorial</translatableAttribute> + <translatableAttribute id="tooltip">Start the introductory tutorial.</translatableAttribute> + <action on="Press"> + Engine.SwitchGuiPage("page_gamesetup.xml", { "type": "offline", "tutorial": "Introductory_Tutorial" }); + </action> + </object> + + <!-- ECONOMIC WALKTHROUGH TUTORIAL BUTTON --> + <object + type="button" + style="StoneButtonFancy" + size="0 64 100% 92" + tooltip_style="pgToolTip" + > + <translatableAttribute id="caption">Economic Tutorial</translatableAttribute> <translatableAttribute id="tooltip">Start the economic tutorial.</translatableAttribute> <action on="Press"> - Engine.SwitchGuiPage("page_gamesetup.xml", { "type": "offline", "tutorial": true }); + Engine.SwitchGuiPage("page_gamesetup.xml", { "type": "offline", "tutorial": "starting_economy_walkthrough" }); </action> </object> <!-- STRUCTREE BUTTON --> <object style="StoneButtonFancy" type="button" - size="0 64 100% 92" + size="0 96 100% 124" tooltip_style="pgToolTip" > <translatableAttribute id="caption">Structure Tree</translatableAttribute> <translatableAttribute id="tooltip">View the structure tree of civilizations featured in 0 A.D.</translatableAttribute> <action on="Press"> @@ -187,11 +201,11 @@ <!-- HISTORY BUTTON --> <object style="StoneButtonFancy" type="button" - size="0 96 100% 124" + size="0 128 100% 156" tooltip_style="pgToolTip" > <translatableAttribute id="caption">History</translatableAttribute> <translatableAttribute id="tooltip">Learn about the many civilizations featured in 0 A.D.</translatableAttribute> <action on="Press"> @@ -449,11 +463,11 @@ > <translatableAttribute id="caption">Learn to Play</translatableAttribute> <translatableAttribute id="tooltip">Learn how to play, start the tutorial, discover the technology trees, and the history behind the civilizations</translatableAttribute> <action on="Press"> closeMenu(); - openMenu("submenuLearn", (this.parent.size.top+this.size.top), (this.size.bottom-this.size.top), 4); + openMenu("submenuLearn", (this.parent.size.top+this.size.top), (this.size.bottom-this.size.top), 5); </action> </object> <!-- SINGLEPLAYER BUTTON --> <object