Details

Reviewers

bb
elexis

Commits

rP19345: Capture The Relic gamemode.
rP19326: A few icon updates
rP19316: Catafalque wagon unit
rP19327: Move catafalque actor to global (as it is not civ specific).

Summary

Originally this diff added a "Capture the Flag" victory condition that used "flag" units.

This diff adds a type of Capture the Relic victory condition. Players (or teams) have to capture and keep all of the relics that spawn on the map for a certain amount of time.

The number of "relics" that spawn corresponds to the number of players (for example,
4 players, 4 relic units spawn). Currently, the relics use a packed siege engine actor, and they will spawn randomly in neutral territory (they have an always visible selection overlay, like heroes, to try to make them look distinct). In this diff, the relics do not necessarily have to be brought back to the territory of the captured player; s0600204 does this in the "Capture the Sheep" game here https://github.com/s0600204/0ad/tree/cts, but it also requires a new component (this diff also takes inspiration from s0600204's "Capture the Sheep" game).

Also, the relics would have to be "garrisonable" for a territory requirement, and this would add the extra issue of dealing with ships that are destroyed with relics on board.

Test Plan

In the Capture the Relic victory condition, relics will spawn in neutral territory at the start of the game. They can be captured, and they cannot be deleted or damaged. Once a player/team owns all of the relics, they will win the game after a period of time. If an enemy player captures a relic from a player/team who has already captured them all, the countdown will reset.

Diff Detail

Repository

rP 0 A.D. Public Repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 869
Build 1365: Vulcan Build	Jenkins

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Rename the victory condition to "Capture The Relic".

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

Harbormaster completed remote builds in B768: Diff 804.Mar 15 2017, 11:57 PM

Sandarac retitled this revision from Capture the Flag victory condition to Capture the Relic victory condition.Mar 16 2017, 12:23 AM

Sandarac edited the summary of this revision. (Show Details)

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 (302 tests)..............................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!

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

Harbormaster completed remote builds in B769: Diff 806.Mar 16 2017, 12:41 AM

Merge the Wonder victory duration option and the Capture The Relic duration option into a new generic "Victory Duration", and give the relics a PassabilityClass and a unique Minimap colour.

elexis mentioned this in D215: Dynamic Special Templates.Mar 16 2017, 3:39 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/526/ for more details.

Harbormaster completed remote builds in B770: Diff 807.Mar 16 2017, 4:03 AM

Correctly handle translatePluralWithContext calls.
In CaptureTheRelic.js:

Spilt the checks for cmpPosition and cmpIdentity
Inline validSpawnPoint.
Remove duplicate UnitMotionFlying check.
Split the call to StartCaptureTheRelicCountdown over multiple lines.
Place cmpTrigger variables in a local scope (D195).

Fix typo.

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

Harbormaster completed remote builds in B772: Diff 809.Mar 16 2017, 6:58 AM

Update the context for the translatePluralWithContext in loadVictoryDuration in settings.js.

Harbormaster completed remote builds in B774: Diff 811.Mar 16 2017, 8:26 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/530/ for more details.

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

Harbormaster completed remote builds in B775: Diff 812.Mar 16 2017, 9:10 AM

elexis mentioned this in D230: Armour.Invincibility template for AI compatibility, use it in Survival of the Fittest.Mar 17 2017, 5:17 PM

elexis added inline comments.Mar 17 2017, 5:57 PM

binaries/data/mods/public/maps/scripts/CaptureTheRelic.js
2	All of the functions look like they could easily be extended if we intend to implement the artifact pickup variant, so naming them Relic is in line with that. g_CatafalqueTemplate though and special_relic -> special_catafalque
45	When testing I encountered a mapgen where 2 relics were spawned right beside each other near one players base. It were preferable if the distance between the avilable spawn points is maximized. Doesn't have to be the actual maximum possible (as that might require brute forcing all solutions, NP-hard issue iirc), a simple algorithm should be sufficient. Maybe picking one location, then picking the next location that has the greatest distance to all other locations?
51	D230 if you wan't to add the foundation for generic AI compatibility
binaries/data/mods/public/simulation/components/Identity.js
57	Should that become Catafalque, so as to differentiate it from the artifact that might be added in the future?
binaries/data/mods/public/simulation/templates/other/special_relic.xml
23 ↗	(On Diff #812)	catafalque

Sandarac updated this revision to Diff 837.Mar 18 2017, 1:50 AM

Sandarac marked 2 inline comments as done.Mar 18 2017, 2:24 AM

Sandarac added inline comments.

binaries/data/mods/public/maps/scripts/CaptureTheRelic.js
45	Maximizing the distance between spawn points causes the entities to be spawned in extreme corners of the map, in an extremely predictable pattern, so this is not preferable, but yes, using something other than purely random selection is a good idea. The updated diff takes into account the position of already-placed relics when spawning them, and also uses some randomness.
51	As D230 is not complete, `cmpDamageReceiver.SetInvulnerability(true);` can be done for now.
binaries/data/mods/public/simulation/components/Identity.js
57	The Relic class could likely be used in both cases.

Add some refinements to pickSpawnPoint.

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

Harbormaster completed remote builds in B795: Diff 837.Mar 18 2017, 2:35 AM

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

Harbormaster completed remote builds in B796: Diff 838.Mar 18 2017, 3:19 AM

Decrease randFloat values slightly to increase the probability that relics will spawn towards the center of the map.

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

Harbormaster completed remote builds in B797: Diff 845.Mar 18 2017, 8:04 AM

Sandarac mentioned this in D236: Petra: Initial Capture The Relic support.Mar 18 2017, 8:48 PM

Commit finger is itching, but some things should be fixed before we can finally enjoy this gamemode:

Somehow those newly added files don't appear as added but modified, do you know why that is the case? Did you svn add them?

Math.ceil(number of players / 2) relics would be a better number until we have the gamesetup option

Template:

Use the new Catafalque actor committed by Enrique
Should walk 20% slower? (6->5) thats annoyingly slow, but that is appropriate, no?
run speed way too fast, should be at 8 maybe?
walk speed, run speed, not alphabetic is the right order apparently
GarrisonRegenRate unused, so could be 0 just as well
100 capture points seems to be too few, too easy to capture. even one single cavalry unit has it relatively quick. should be quick, but not that quick, right? siege RegenRate is 10 to 20, so having 10 here would be much more appropriate. some testing reveals 10 regenrate with 250CP means it can be captured quick enough by 8 citizen soldiers
star selection ring is ugly and not needed anymore
Generic name should be Catafalque
Loot tooltip empty, should be disabled.
<ResourceGatherer/> after noticing an empty loot tooltip
<DefaultStance>standground</DefaultStance>?
<CanGuard>true</CanGuard> nope
VisionRange ok
That galloping sound is too fast for the slow walk speed. Those fs_grass ogg files (audio actor singlesteps/steps_grass) seem more appropriate, agree?
Bribable to be discussed somewhere else
Formations nope, see fishing ships f.e.
<Decay> can be kept for future potentially destructable ones
<Population>1</Population> 0?
<Armour> must be kept in order to not break everything afaik
<Looter/> can and should be kept
Perhaps someone wants to give that thing some aura, perhaps for garrisoned regen rate if that is possible. should be discussed separately

Relic location:

angle should be randomized (randFloat(0, 2 * PI))
Rename gaiaEntities to potentialSpawnPoints
Code should have a comment stating that it uses gaia entities assuming that they are placed in reachable locations
Should ignore actors, as these can be placed on impassable areas (either stronghold or frontier was such a map)
distanceToSquared -> DistanceBetweenEntities (also a bit faster as it used 2D)
No valid spawn points on tiny Continent map. Can be solved by first not doing the territory manager check, then saving the territory filtered spawn points to another variable and then using that if its not empty.
reliably placed near the map border. Perhaps maxDist/2 is sufficient? (Also for a tiny cycladic archipelago map with 8 players I get 3 spawned in corner of the middle island and 5 in the other, unexpected but not wrong)

Strings:

"has captured all of the relics" -> "has captured all relics"
markForTranslation("%(player)s has captured all of the relics. They will have won in %(time)s") singular they is ugly. just avoid it with an "and" (for both or all four sentences for consistency)

This revision now requires changes to proceed.Mar 20 2017, 6:04 PM

The Relic class should likely become a visible class, and it seems weird to have the GenericName Relic when its a Catafalque. Players might not know which thing the relic is, on the other side, they will likely find out themselves

Thanks elexis for the review. I agree with most of the comments, but I think that the relics should be slightly easier to capture than what you propose.

In D152#8930, @elexis wrote:

Somehow those newly added files don't appear as added but modified, do you know why that is the case? Did you svn add them?

I used svn add because I wasn't sure how else to give the new files the native eol-style.

Lionkanzen awarded a token.Mar 21 2017, 9:19 AM

Lionkanzen added a subscriber: Lionkanzen.

elexis added commits: rP19327: Move catafalque actor to global (as it is not civ specific)., rP19316: Catafalque wagon unit, rP19326: A few icon updates.Mar 23 2017, 4:53 AM

Sandarac updated this revision to Diff 922.Mar 24 2017, 3:29 AM

Sandarac edited edge metadata.

Sandarac updated this revision to Diff 923.Mar 24 2017, 3:59 AM

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

Harbormaster completed remote builds in B868: Diff 922.Mar 24 2017, 4:16 AM

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

Harbormaster completed remote builds in B869: Diff 923.Mar 24 2017, 5:56 AM

Apply elexis' comments.

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

Harbormaster completed remote builds in B870: Diff 924.Mar 24 2017, 7:25 AM

Use pickRandom when placing relics - placement of relics will be handled in another diff.

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

Harbormaster completed remote builds in B885: Diff 940.Mar 25 2017, 6:50 AM

Last code remarks. They were discussed in irc, fixed and reviewed by Sandarac via pastebin.
Tested thoroughly with diplomacy changes and various maps of different sizes, gaia entity counts and shapes (water.

binaries/data/mods/public/maps/scripts/CaptureTheRelic.js
2	This file leaves a clean impression, good work!
29	Not triggered by hawks as they don't have Identity. Tested with acropolis bay with gaia planes and test works.
33	Tested on cycladic archipelago. Some semi submerged berries and stone mines were detected too.
42	Even if it's only one, that one will be used N times, so it works for all maps
50	In case it actually doesn't pick the max anymore, a shuffleArray would make things independent of entity ID order
69	That , 1 not really needed, might be easier to read without, doesn't matter though
69	Hope to see someone accomplishing the impossible and triggering that case with some trick
99	Next time move that { to the next line
122	Since this function isn't called from an ownership change / diplomacy change event but called manually from this prototype, there is no point to hide the actual two arguments in the data variable. isTeam is derived from data.to, so shouldn't be passed as an additional argument. Remotely refs https://en.wikipedia.org/wiki/Single_source_of_truth (Variable shuffling performed locally, sent via pastebin and accepted by Sandarac in irc) The entity doesn't need to be passed at all, since it isn't relevant which player is shown on the screen (with this diff it shows the one that captured the lowest entity ID entity). If it were to show the last one that captured something, that would have to be saved in a prototype grade variable to also work with diplomacy changes. So we end up with only passing playersAndAllies.
124	messages var can be nuked
136	players contains all players that are not in playersAndAllies and is being used for the otherMessage, so should be called others
164	"and will have won" seems more natural. Considered but rejected periods. Also Gallaecio prefers this string over "every Relic".
179	Since there can be only one countdown and one message per player at a time, those things shouldn't be arrays / objects, just a single atomic number / undefined variable

Congratulation on this great feature Sandarac, it will be one of the highlights of Alpha 22!

Original idea years ago with pickup relics.
Because that was unhandy to implement (requiring pickup #28),
a movable and capturable siege engine was used.

wowgetoffyourcellphone has then suggested that we could make this a Catalafque unit to fit into the historic context.

Designed the details of the gamemode in a handful of sleepless nights with Sandarac.
Spawn point generation code was advised with bb.
The name of the game was discussed with Pureon, Enrique and Itms.
Enrique was kind enough to create a new model for the catafalque wagon and committed it in rP19316.
Pureon has drawn an icon for this unit and committed it in rP19326.

Future Tasks:
(1) Petra Bot support
(2) Auras for the relics, perhaps even 8 different catafalque wagons with different auras each
(3) Gamesetup option to allow players to vary the number of relics on the map
(4) Entirely random relic distribution can sometimes favor one player
(5) Gamemode variant where Artifact relics need to be picked up and garrisoned to win

Thanks to everyone who has and will contribute to this new gamemode!

Closed by commit rP19345: Capture The Relic gamemode. (authored by elexis). · Explain WhyMar 25 2017, 8:01 AM

This revision was automatically updated to reflect the committed changes.

elexis edited the summary of this revision. (Show Details)Mar 25 2017, 8:09 AM

elexis mentioned this in D269: Civ specific catafalque templates and auras.Mar 28 2017, 12:09 AM

Sandarac mentioned this in D104: Regicide game - Gamesetup Option to Disable Garrisoning of Heroes.Apr 15 2017, 11:01 PM

elexis mentioned this in rP20060: Colorize relic victory message..Aug 28 2017, 1:30 AM

elexis mentioned this in rP20385: Move catafalque relic templates from the "other" directory to the regular units….Oct 31 2017, 12:44 PM

Capture the Relic victory condition
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 923

binaries/data/mods/public/gui/common/gamedescription.js

binaries/data/mods/public/gui/common/settings.js

binaries/data/mods/public/gui/gamesetup/gamesetup.js

binaries/data/mods/public/gui/gamesetup/gamesetup.xml

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

binaries/data/mods/public/maps/scripts/CaptureTheRelic.js

binaries/data/mods/public/maps/scripts/WonderVictory.js

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

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

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

binaries/data/mods/public/simulation/data/settings/victory_conditions/capture_the_relic.json

binaries/data/mods/public/simulation/data/settings/victory_times.json

binaries/data/mods/public/simulation/data/settings/wonder_times.json

binaries/data/mods/public/simulation/helpers/Setup.js

binaries/data/mods/public/simulation/templates/other/special_catafalque.xml

source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Map/Map.cpp

Capture the Relic victory conditionClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 923

binaries/data/mods/public/gui/common/gamedescription.js

binaries/data/mods/public/gui/common/settings.js

binaries/data/mods/public/gui/gamesetup/gamesetup.js

binaries/data/mods/public/gui/gamesetup/gamesetup.xml

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

binaries/data/mods/public/maps/scripts/CaptureTheRelic.js

binaries/data/mods/public/maps/scripts/WonderVictory.js

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

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

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

binaries/data/mods/public/simulation/data/settings/victory_conditions/capture_the_relic.json

binaries/data/mods/public/simulation/data/settings/victory_times.json

binaries/data/mods/public/simulation/data/settings/wonder_times.json

binaries/data/mods/public/simulation/helpers/Setup.js

binaries/data/mods/public/simulation/templates/other/special_catafalque.xml

source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Map/Map.cpp

Capture the Relic victory condition
ClosedPublic
Actions

Revision Contents
Changeset List