Page MenuHomeWildfire Games

Petra: do not create trade route crossing enemy territory
ClosedPublic

Authored by mimo on Jun 3 2017, 7:25 PM.

Details

Summary

As described in #4607, petra should avoid trade routes crossing enemy territory. That's done in this patch with a simple check with a straight line between both markets.

Test Plan

test in game

Event Timeline

mimo created this revision.Jun 3 2017, 7:25 PM
mimo added a comment.Jun 3 2017, 7:34 PM

I forgot to add that, when looking for trade routes, the ai tries first a route with an ally, and if no allied market available, it tries between two of its markets. When doing that, all pairs of markets was tried twice. This is now also fixed in the patch.

mimo updated this revision to Diff 2410.Jun 3 2017, 8:12 PM

use also the check on crossing enemy territory when looking for best position for a new market

mimo updated this revision to Diff 2411.Jun 3 2017, 8:15 PM

reupdate the patch

Vulcan added a subscriber: Vulcan.Jun 3 2017, 11:40 PM

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/1461/ for more details.

Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/ai/petra/entityExtend.js
| 179| »   »   if·(bonus.Classes·&&·bonus.Classes.split(/\s+/).some(cls·=>·!target.hasClass(cls)))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '='.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/ai/petra/tradeManager.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/ai/petra/tradeManager.js
| 406| 406| 	let candidate = { "gain": 0 };
| 407| 407| 	let potential = { "gain": 0 };
| 408| 408| 	let bestIndex = { "gain": 0 };
| 409|    |-	let bestLand  = { "gain": 0 };
|    | 409|+	let bestLand = { "gain": 0 };
| 410| 410| 
| 411| 411| 	let traderTemplatesGains = gameState.getTraderTemplatesGains();
| 412| 412| 

binaries/data/mods/public/simulation/ai/petra/tradeManager.js
| 222| »   let·getBarterRate·=·function·(prices,buy,sell)·{·return·Math.round(100·*·prices.sell[sell]·/·prices.buy[buy]);·};
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'prices' is already declared in the upper scope.

binaries/data/mods/public/simulation/ai/petra/tradeManager.js
| 585| »   »   plan.onStart·=·function(gameState)·{·gameState.ai.queueManager.changePriority("economicBuilding",·gameState.ai.Config.priorities.economicBuilding);·};
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'gameState' is already declared in the upper scope.
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/119/ for more details.

Thanks, this will be a great help for this issue, but still, as alluded to the trac ticket, I believe that HQ.isDangerousLocation() should be used as well when planing trade routes, even more so as this approach is less perfect than doing an actual computePath check.

So if done with this check included, then the issue of making trade routes through enemy territories can be addressed, and also the issue of the AI repeatedly sending traders on dangerous routes that they cannot use. What do you think?

Vulcan added a comment.Jun 4 2017, 2:22 AM

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/1463/ for more details.

Vulcan added a comment.Jun 4 2017, 2:24 AM
Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/ai/petra/entityExtend.js
| 179| »   »   if·(bonus.Classes·&&·bonus.Classes.split(/\s+/).some(cls·=>·!target.hasClass(cls)))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '='.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/ai/petra/tradeManager.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/ai/petra/tradeManager.js
| 406| 406| 	let candidate = { "gain": 0 };
| 407| 407| 	let potential = { "gain": 0 };
| 408| 408| 	let bestIndex = { "gain": 0 };
| 409|    |-	let bestLand  = { "gain": 0 };
|    | 409|+	let bestLand = { "gain": 0 };
| 410| 410| 
| 411| 411| 	let traderTemplatesGains = gameState.getTraderTemplatesGains();
| 412| 412| 

binaries/data/mods/public/simulation/ai/petra/tradeManager.js
| 222| »   let·getBarterRate·=·function·(prices,buy,sell)·{·return·Math.round(100·*·prices.sell[sell]·/·prices.buy[buy]);·};
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'prices' is already declared in the upper scope.

binaries/data/mods/public/simulation/ai/petra/tradeManager.js
| 585| »   »   plan.onStart·=·function(gameState)·{·gameState.ai.queueManager.changePriority("economicBuilding",·gameState.ai.Config.priorities.economicBuilding);·};
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'gameState' is already declared in the upper scope.

binaries/data/mods/public/simulation/ai/petra/headquarters.js
|1330| »   plan.onStart·=·function(gameState)·{·gameState.ai.queueManager.changePriority("economicBuilding",·gameState.ai.Config.priorities.economicBuilding);·};
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'gameState' is already declared in the upper scope.

binaries/data/mods/public/simulation/ai/petra/headquarters.js
|1410| »   »   plan.isGo·=·function·(gameState)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'gameState' is already declared in the upper scope.

binaries/data/mods/public/simulation/ai/petra/headquarters.js
|1572| »   »   »   »   plan.onStart·=·function(gameState)·{·gameState.ai.queueManager.changePriority("defenseBuilding",·gameState.ai.Config.priorities.defenseBuilding);·};
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'gameState' is already declared in the upper scope.

binaries/data/mods/public/simulation/ai/petra/headquarters.js
|1604| »   »   plan.onStart·=·function(gameState)·{·gameState.ai.queueManager.changePriority("defenseBuilding",·gameState.ai.Config.priorities.defenseBuilding);·};
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'gameState' is already declared in the upper scope.

binaries/data/mods/public/simulation/ai/petra/headquarters.js
|1644| »   »   »   plan.onStart·=·function(gameState)·{·gameState.ai.queueManager.changePriority("militaryBuilding",·gameState.ai.Config.priorities.militaryBuilding);·};
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'gameState' is already declared in the upper scope.

binaries/data/mods/public/simulation/ai/petra/headquarters.js
| 170| »   »   »   »   builders.forEach(function·(worker)·{
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/121/ for more details.

Vulcan added a comment.Jun 4 2017, 3:15 AM

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/1464/ for more details.

Vulcan added a comment.Jun 4 2017, 3:17 AM
Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/ai/petra/entityExtend.js
| 179| »   »   if·(bonus.Classes·&&·bonus.Classes.split(/\s+/).some(cls·=>·!target.hasClass(cls)))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '='.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/ai/petra/tradeManager.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/ai/petra/tradeManager.js
| 406| 406| 	let candidate = { "gain": 0 };
| 407| 407| 	let potential = { "gain": 0 };
| 408| 408| 	let bestIndex = { "gain": 0 };
| 409|    |-	let bestLand  = { "gain": 0 };
|    | 409|+	let bestLand = { "gain": 0 };
| 410| 410| 
| 411| 411| 	let traderTemplatesGains = gameState.getTraderTemplatesGains();
| 412| 412| 

binaries/data/mods/public/simulation/ai/petra/tradeManager.js
| 222| »   let·getBarterRate·=·function·(prices,buy,sell)·{·return·Math.round(100·*·prices.sell[sell]·/·prices.buy[buy]);·};
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'prices' is already declared in the upper scope.

binaries/data/mods/public/simulation/ai/petra/tradeManager.js
| 585| »   »   plan.onStart·=·function(gameState)·{·gameState.ai.queueManager.changePriority("economicBuilding",·gameState.ai.Config.priorities.economicBuilding);·};
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'gameState' is already declared in the upper scope.

binaries/data/mods/public/simulation/ai/petra/headquarters.js
|1330| »   plan.onStart·=·function(gameState)·{·gameState.ai.queueManager.changePriority("economicBuilding",·gameState.ai.Config.priorities.economicBuilding);·};
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'gameState' is already declared in the upper scope.

binaries/data/mods/public/simulation/ai/petra/headquarters.js
|1410| »   »   plan.isGo·=·function·(gameState)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'gameState' is already declared in the upper scope.

binaries/data/mods/public/simulation/ai/petra/headquarters.js
|1572| »   »   »   »   plan.onStart·=·function(gameState)·{·gameState.ai.queueManager.changePriority("defenseBuilding",·gameState.ai.Config.priorities.defenseBuilding);·};
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'gameState' is already declared in the upper scope.

binaries/data/mods/public/simulation/ai/petra/headquarters.js
|1604| »   »   plan.onStart·=·function(gameState)·{·gameState.ai.queueManager.changePriority("defenseBuilding",·gameState.ai.Config.priorities.defenseBuilding);·};
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'gameState' is already declared in the upper scope.

binaries/data/mods/public/simulation/ai/petra/headquarters.js
|1644| »   »   »   plan.onStart·=·function(gameState)·{·gameState.ai.queueManager.changePriority("militaryBuilding",·gameState.ai.Config.priorities.militaryBuilding);·};
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'gameState' is already declared in the upper scope.

binaries/data/mods/public/simulation/ai/petra/headquarters.js
| 170| »   »   »   »   builders.forEach(function·(worker)·{
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/122/ for more details.

mimo added a comment.Jun 4 2017, 12:10 PM

Thanks, this will be a great help for this issue, but still, as alluded to the trac ticket, I believe that HQ.isDangerousLocation() should be used as well when planing trade routes, even more so as this approach is less perfect than doing an actual computePath check.

isDangerousLocation() also checks the presence of an enemy army around, which is much shorter time range than a trade route. We can nonetheless keep the other part (isUnderEnemyFire). But what's your proposition: to check it on each point of the line? that could be done in tradeManager when looking for routes but may be too expensive in the findMarketLocation where each possible location is tried, and if both does not use the same conditions, we may build useless markets (although it will still be better than what we have now with suicidal traders).

So if done with this check included, then the issue of making trade routes through enemy territories can be addressed, and also the issue of the AI repeatedly sending traders on dangerous routes that they cannot use. What do you think?

In D600#24684, @mimo wrote:

Thanks, this will be a great help for this issue, but still, as alluded to the trac ticket, I believe that HQ.isDangerousLocation() should be used as well when planing trade routes, even more so as this approach is less perfect than doing an actual computePath check.

isDangerousLocation() also checks the presence of an enemy army around, which is much shorter time range than a trade route.

Yes, and it would be preferable to include it for trade routes (eventually, Petra should check its trade routes periodically with this check imo, even if just using a straight line as is done here). But yes, isUnderEnemyFire is fine.

We can nonetheless keep the other part (isUnderEnemyFire). But what's your proposition: to check it on each point of the line? that could be done in tradeManager when looking for routes but may be too expensive in the findMarketLocation where each possible location is tried, and if both does not use the same conditions, we may build useless markets (although it will still be better than what we have now with suicidal traders).

I believe there should be checks for each point of the line, as Petra's trading functionality is often rendered useless (especially in late-game) if there are f.e. enemy defensive towers near the trade route, and it will keep sending new traders on the same unusable route over and over. This diff will help avoid this happening, and maybe in some other diff the AI could try to pick a new trade route if its traders keep losing their health.

mimo updated this revision to Diff 2447.Jun 5 2017, 4:41 PM

update patch

mimo added a comment.Jun 5 2017, 4:47 PM

I tested it with a replayprofile, and it creates huge timing spikes when looking for a market position. So the isUnderEnemyFire can't be used directly, and doing more complex things is not possible right now as we are on feature freeze. So the question is do we delay everything for A23, or do we use this simple straight line without enemy fire check for A22 which i guess would already reduce the number of suicidal traders by a huge fraction.

(In the latest patch, i've also fixed a lint warning for variable shadowing)

Vulcan added a comment.Jun 5 2017, 9:59 PM
Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/ai/petra/entityExtend.js
| 179| »   »   if·(bonus.Classes·&&·bonus.Classes.split(/\s+/).some(cls·=>·!target.hasClass(cls)))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '='.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/ai/petra/tradeManager.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/ai/petra/tradeManager.js
| 406| 406| 	let candidate = { "gain": 0 };
| 407| 407| 	let potential = { "gain": 0 };
| 408| 408| 	let bestIndex = { "gain": 0 };
| 409|    |-	let bestLand  = { "gain": 0 };
|    | 409|+	let bestLand = { "gain": 0 };
| 410| 410| 
| 411| 411| 	let traderTemplatesGains = gameState.getTraderTemplatesGains();
| 412| 412| 

binaries/data/mods/public/simulation/ai/petra/tradeManager.js
| 585| »   »   plan.onStart·=·function(gameState)·{·gameState.ai.queueManager.changePriority("economicBuilding",·gameState.ai.Config.priorities.economicBuilding);·};
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'gameState' is already declared in the upper scope.

binaries/data/mods/public/simulation/ai/petra/headquarters.js
|1322| »   plan.onStart·=·function(gameState)·{·gameState.ai.queueManager.changePriority("economicBuilding",·gameState.ai.Config.priorities.economicBuilding);·};
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'gameState' is already declared in the upper scope.

binaries/data/mods/public/simulation/ai/petra/headquarters.js
|1402| »   »   plan.isGo·=·function·(gameState)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'gameState' is already declared in the upper scope.

binaries/data/mods/public/simulation/ai/petra/headquarters.js
|1564| »   »   »   »   plan.onStart·=·function(gameState)·{·gameState.ai.queueManager.changePriority("defenseBuilding",·gameState.ai.Config.priorities.defenseBuilding);·};
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'gameState' is already declared in the upper scope.

binaries/data/mods/public/simulation/ai/petra/headquarters.js
|1596| »   »   plan.onStart·=·function(gameState)·{·gameState.ai.queueManager.changePriority("defenseBuilding",·gameState.ai.Config.priorities.defenseBuilding);·};
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'gameState' is already declared in the upper scope.

binaries/data/mods/public/simulation/ai/petra/headquarters.js
|1636| »   »   »   plan.onStart·=·function(gameState)·{·gameState.ai.queueManager.changePriority("militaryBuilding",·gameState.ai.Config.priorities.militaryBuilding);·};
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'gameState' is already declared in the upper scope.

binaries/data/mods/public/simulation/ai/petra/headquarters.js
| 170| »   »   »   »   builders.forEach(function·(worker)·{
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/138/ for more details.

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/1489/ for more details.

This revision was automatically updated to reflect the committed changes.