Petra: Look for aquatic treasure on naval maps
ClosedPublic
Actions

Authored by Sandarac on Aug 2 2017, 12:08 AM.

Details

Reviewers

mimo

Commits

rP20108: Petra: Look for aquatic treasure on naval maps

Summary

Found on f.e. the "Cycladic Archipalego" maps, "Sporades Islands", and the random map "Island Stronghold". Petra already checks on land, so it makes sense to check on water as well.

gatherTreasure() is moved to entityExtend.js so it can be used on non-worker entities.

In a future step, the naval manager could assign a specific merchant ship for each sea accessIndex, and send it to search for treasure; in this diff, naval traders will simply check on occasion if there is any nearby treasure.

Test Plan

A good map to check is the "Cycladic Archipalego" random map. Naval traders will collect any treasure nearby, then continue on their trade route.

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

Sandarac created this revision.Aug 2 2017, 12:08 AM

Owners added a subscriber: Restricted Owners Package.Aug 2 2017, 12:08 AM

Sandarac edited the summary of this revision. (Show Details)Aug 2 2017, 12:13 AM

Sandarac edited the test plan for this revision. (Show 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/1794/ for more details.

Harbormaster completed remote builds in B2689: Diff 2999.Aug 2 2017, 12:54 AM

mimo added a subscriber: mimo.Aug 6 2017, 6:06 PM

mimo added inline comments.

binaries/data/mods/public/simulation/ai/petra/entityExtend.js
321 ↗	(On Diff #2999)	most functions like getAccessValue have "water = false" as argument instead of this "land = true". That's may be better to keep consistency here, avoiding the negations on lines 330 and 333 in addition, an early return if (!gameState.ai.HQ.treasures.hasEntities()) may help to avoid useless steps in maps with no treasures?
333 ↗	(On Diff #2999)	isn't line 347 enough and do we really need lines 333-338

I'm going to commit that patch with the changes i proposed above.
Futhermore, while testing, we could end up in a endless loop if a second treasure was found before the first one is gathered. So i've added some protection (do not look for other treasures if already gathering one). I've also added an absolute distance limit (350*350) in gatherTreasure and idle ship now also look for treasures around.
Feel free to raise a concern when you are back if you find something wrong.

mimo accepted this revision.Sep 4 2017, 8:04 PM

This revision is now accepted and ready to land.Sep 4 2017, 8:04 PM

Closed by commit rP20108: Petra: Look for aquatic treasure on naval maps (authored by mimo). · Explain WhySep 4 2017, 8:06 PM

This revision was automatically updated to reflect the committed changes.