Page MenuHomeWildfire Games

counter espionage
ClosedPublic

Authored by mimo on Mar 1 2017, 8:26 PM.

Details

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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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
Vulcan added a subscriber: Vulcan.Mar 1 2017, 9:13 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

mimo updated this revision to Diff 667.Mar 1 2017, 10:33 PM
mimo edited the summary of this revision. (Show Details)

Tests are not my friend today

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.

mimo updated this revision to Diff 688.Mar 2 2017, 9:18 PM

some fixes + adaptation to rP19258

Vulcan added a comment.Mar 3 2017, 1:10 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/456/ for more details.

Sandarac added inline comments.
binaries/data/mods/public/simulation/components/Player.js
185 ↗(On Diff #688)

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.

mimo added inline comments.Mar 9 2017, 7:12 PM
binaries/data/mods/public/simulation/components/Player.js
185 ↗(On Diff #688)

right, i will fix it

mimo updated this revision to Diff 742.Mar 9 2017, 7:22 PM

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?

Vulcan added a comment.Mar 9 2017, 8:06 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.

mimo updated this revision to Diff 757.Mar 11 2017, 10:57 AM

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

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.

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

elexis accepted this revision.Mar 28 2017, 7:34 AM
elexis added a subscriber: elexis.

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
523 ↗(On Diff #757)

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 ↗(On Diff #757)

Additional

126 ↗(On Diff #757)

(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
3 ↗(On Diff #757)

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 ↗(On Diff #757)

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
mimo updated this revision to Diff 985.Mar 28 2017, 8:53 PM

Fix elexis comments + new icon

mimo added a comment.Mar 28 2017, 8:58 PM
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 ↗(On Diff #757)

not understood? where?

binaries/data/mods/public/simulation/data/technologies/spy_counter.json
3 ↗(On Diff #757)

Here is my proposition, any comments :-)

elexis added inline comments.
binaries/data/mods/public/simulation/components/VisionSharing.js
126 ↗(On Diff #757)

("Cost/Resources/"+res -> "Cost/Resources/" + res, but I entirely don't insist on those)

binaries/data/mods/public/simulation/data/technologies/spy_counter.json
3 ↗(On Diff #985)

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.

Gallaecio accepted this revision.Mar 28 2017, 10:30 PM

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

binaries/data/mods/public/simulation/data/technologies/spy_counter.json
3 ↗(On Diff #757)

“Pay your units better wages to make it more expensive for your enemies to bribe them”?

3 ↗(On Diff #985)

What about “[…] watch over your population” or “[…] watch over your people”?

This revision was automatically updated to reflect the committed changes.