Details

Reviewers

Gallaecio
elexis

Commits

rP19357: counter espionage

Summary

As was proposed in the forums, here is the addition of a tech to counter espionage: when researched, it increases the cost that other players have to pay to bribe your units. I've then reduced the initial cost of spies to 600 metal (as proposed by elexis), and it is increased by 50% when this tech (reaching the current 900 metal).

Pending questions in the patch

i've made it researchable in the market (as espionage, superseding it), but could be better to have this new one elsewhere (in the civil center) with a requirement on Espionage?
this new tech (counter espionage) needs a clever description sentence
there is a conflict with D173, so if this approach is kept, we should change D173 to a more versatile version which allow for such multiplier

Test Plan

Check that the tech indeed increases cost of spies.

Diff Detail

Repository

rP 0 A.D. Public Repository

Branch

/ps/trunk

Lint

No Lint Coverage

Unit

No Test Coverage

Build Status

Buildable 666
Build 1055: Vulcan Build	Jenkins
Build 1054: arc lint + arc unit

Event Timeline

mimo created this revision.Mar 1 2017, 8:26 PM

Owners added a subscriber: Restricted Owners Package.Mar 1 2017, 8:26 PM

Build has FAILED

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (302 tests).ERROR: JavaScript error: simulation/components/GuiInterface.js line 111
TypeError: cmpPlayer.GetSpyCostMultiplier is not a function
  GuiInterface.prototype.GetSimulationState@simulation/components/GuiInterface.js:111:25
  @simulation/components/tests/test_GuiInterface.js:260:25

In TestComponentScripts::test_scripts:
/mnt/data/jenkins/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:44: Error: Test failed: L"Running script simulation/components/tests/test_GuiInterface.js"
/mnt/data/jenkins/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:44: Error: Assertion failed: scriptInterface.LoadScript(pathname, content)
ERROR: JavaScript error: simulation/components/Player.js line 39
TypeError: this.template is undefined
  Player.prototype.Init@simulation/components/Player.js:39:27
  global.ConstructComponent@simulation/components/tests/setup.js:91:2
  @simulation/components/tests/test_Player.js:11:17
/mnt/data/jenkins/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:44: Error: Test failed: L"Running script simulation/components/tests/test_Player.js"
/mnt/data/jenkins/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:44: Error: Assertion failed: scriptInterface.LoadScript(pathname, content)
............................................................................................................................................................................................................................................................................................................
Failed 1 and Skipped 0 of 302 tests
Success rate: 99%

Link to build: http://jw:8080/job/phabricator/441/
See console output for more information: http://jw:8080/job/phabricator/441/console

Harbormaster failed remote builds in B643: Diff 663!Mar 1 2017, 9:13 PM

Tests are not my friend today

Harbormaster completed remote builds in B647: Diff 667.Mar 1 2017, 10:33 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/444/ for more details.

Harbormaster completed remote builds in B647: Diff 667.Mar 2 2017, 12:28 AM

some fixes + adaptation to rP19258

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/456/ for more details.

Harbormaster completed remote builds in B666: Diff 688.Mar 3 2017, 1:10 AM

mimo mentioned this in rP19276: create maps/tutorials folder and move the starting_economy_walkthrough map….Mar 6 2017, 10:29 PM

Sandarac added a subscriber: Sandarac.Mar 9 2017, 3:40 AM

Sandarac added inline comments.

binaries/data/mods/public/simulation/components/Player.js
185	Is there really that much of a benefit to having this check and assignment here instead of doing `this.spyCostMultiplier = +this.template.SpyCostMultiplier;` on Init? This current function does not follow the style like the other "GetXMultiplier" functions in cmpPlayer.

elexis awarded a token.Mar 9 2017, 11:11 AM

mimo added inline comments.Mar 9 2017, 7:12 PM

binaries/data/mods/public/simulation/components/Player.js
185	right, i will fix it

Updated version following Sandarac comment.
To answer some remarks on the forum, the price of spy has still decreased a bit (500 now, with still +50% when the counter-espionnage)
The two techs (spy and counter-spy are now moved to the cc rather than the market) which looks more appropriate, but i'm ok to change it back if other preferences?

Harbormaster completed remote builds in B713: Diff 742.Mar 9 2017, 7:22 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/487/ for more details.

Harbormaster completed remote builds in B713: Diff 742.Mar 9 2017, 8:06 PM

update patch to include D173 (cost in tooltip) because of conflict between both

mimo mentioned this in D173: Show the bribe cost in the bribing tooltip.Mar 11 2017, 10:58 AM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/495/ for more details.

Harbormaster completed remote builds in B727: Diff 757.Mar 11 2017, 11:41 AM

elexis retitled this revision from counter espionnage to counter espionage.Mar 11 2017, 1:14 PM

The feature works nicely, a player is allowed to bribe allied units which is extremly tactic in diplo games.
Moving the Espionage tech from the market to the Civic Center doesn't lock the bribing to traders only.
The price seems ok (imo the price could even be increased to 500 food and 500 iron, since the enemy offers an higher amount of iron).

Lionkanzen added a subscriber: Lionkanzen.Mar 27 2017, 7:30 PM

The cost tooltip is already included, so D173 can be abolished, correct?

i've made it researchable in the market (as espionage, superseding it), but could be better to have this new one elsewhere (in the civil center) with a requirement on Espionage?

No real opinion on it, both works for me.

Tested, works. Thanks for the patch. Not only makes the feature more affordable but also more interesting. Gives the players more room to interact indirectly, requires them to estimate whether their opponent is likely going to use the feature and weigh the pros and cons of researching the tech. I like it!

binaries/data/mods/public/gui/session/menu.js
500	Yep (this is needed and we already had bugs due to missing clones. We need #4257 so we get syntax errors instead of unexpected changes to these vars)
binaries/data/mods/public/simulation/components/VisionSharing.js
122	Additional
126–128	(2 spaces maybe)
binaries/data/mods/public/simulation/components/tests/test_Player.js
12 ↗	(On Diff #757)	(could be inline with an \n after { to allow future properties, can also stay as is)
binaries/data/mods/public/simulation/data/technologies/spy_counter.json
4	Something along the lines of: Pay your units a fair share of resources and motivate them to stay loyal. Going to ask Hannibal, he is better at making up descriptions than me.
binaries/data/mods/public/simulation/templates/special/spy.xml
11	As ranted about before, this value seems much more realistic to me

This revision is now accepted and ready to land.Mar 28 2017, 7:34 AM

Fix elexis comments + new icon

In D179#10297, @elexis wrote:

The cost tooltip is already included, so D173 can be abolished, correct?

yes

i've made it researchable in the market (as espionage, superseding it), but could be better to have this new one elsewhere (in the civil center) with a requirement on Espionage?

No real opinion on it, both works for me.

Tested, works. Thanks for the patch. Not only makes the feature more affordable but also more interesting. Gives the players more room to interact indirectly, requires them to estimate whether their opponent is likely going to use the feature and weigh the pros and cons of researching the tech. I like it!

binaries/data/mods/public/simulation/components/VisionSharing.js
126–128	not understood? where?
binaries/data/mods/public/simulation/data/technologies/spy_counter.json
4	Here is my proposition, any comments :-)

elexis added subscribers: Gallaecio, leper.Mar 28 2017, 9:20 PM

elexis added inline comments.

binaries/data/mods/public/simulation/components/VisionSharing.js
126–128	("Cost/Resources/"+res -> "Cost/Resources/" + res, but I entirely don't insist on those)
binaries/data/mods/public/simulation/data/technologies/spy_counter.json
4	Hannibal said he thinks the description is nice, but he has no strong opinion apparently. Maybe "to watch over the population". With the new icon, it should be obvious what that means :P (Though I'd expect the bribed units to die with that icon maybe). Works for me, but maybe someone more dedicated on strings like @Gallaecio or @leper have a better proposal.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/627/ for more details.

Harbormaster completed remote builds in B927: Diff 985.Mar 28 2017, 10:23 PM

I’ve added some text suggestions, but the current proposal seems good nonetheless.

binaries/data/mods/public/simulation/data/technologies/spy_counter.json
4	What about “[…] watch over your population” or “[…] watch over your people”?
4	“Pay your units better wages to make it more expensive for your enemies to bribe them”?

Closed by commit rP19357: counter espionage (authored by mimo). · Explain WhyMar 28 2017, 11:37 PM

This revision was automatically updated to reflect the committed changes.

counter espionage
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 688

binaries/data/mods/public/gui/session/menu.js

binaries/data/mods/public/simulation/components/GuiInterface.js

binaries/data/mods/public/simulation/components/Player.js

binaries/data/mods/public/simulation/components/VisionSharing.js

binaries/data/mods/public/simulation/components/tests/test_GuiInterface.js

binaries/data/mods/public/simulation/components/tests/test_VisionSharing.js

binaries/data/mods/public/simulation/data/technologies/spy_counter.json

binaries/data/mods/public/simulation/templates/special/player.xml

binaries/data/mods/public/simulation/templates/special/player_gaia.xml

binaries/data/mods/public/simulation/templates/special/spy.xml

binaries/data/mods/public/simulation/templates/template_structure_economic_market.xml

counter espionageClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 688

binaries/data/mods/public/gui/session/menu.js

binaries/data/mods/public/simulation/components/GuiInterface.js

binaries/data/mods/public/simulation/components/Player.js

binaries/data/mods/public/simulation/components/VisionSharing.js

binaries/data/mods/public/simulation/components/tests/test_GuiInterface.js

binaries/data/mods/public/simulation/components/tests/test_VisionSharing.js

binaries/data/mods/public/simulation/data/technologies/spy_counter.json

binaries/data/mods/public/simulation/templates/special/player.xml

binaries/data/mods/public/simulation/templates/special/player_gaia.xml

binaries/data/mods/public/simulation/templates/special/spy.xml

binaries/data/mods/public/simulation/templates/template_structure_economic_market.xml

counter espionage
ClosedPublic
Actions

Revision Contents
Changeset List