Page MenuHomeWildfire Games
Differential D972

Support diplomacy allied victory for capture the relic and reset wonder counter on player defeat
ClosedPublic
Actions

Authored by bb on Oct 22 2017, 6:32 PM.

Details

Reviewers
Sandarac
elexis
temple
Commits
rP21441: Create winning teams for relic and wonder victory instead of letting all allies…
Trac Tickets
#4648
Summary

Currenlty the CTR countdown is reset on a diplo change between players who are allies of the player with the lowest playerID who is mutually allied to all owners of a relic. (Excuses for the too long sentence.) Also all those allies win, even if the allies are enemies of eachother.
Thus enemies of eachother can win together. imo that shouldn't happen thus all the relicOwners should be mutually ally to start a timer. And only everyone who is mutually ally to all relicOwners will win when the timer runs out. Ofc in between those there can be players enemy of eachother, but now at least the players who actually make the victory decide who win, not a lucky player who is allied to all relicOwners and has the lowest playerID.

For keeping this symmetrical when diplo changes, the timer is reset on all occations the group of winning players changes (thus all the relicOwners + all player mutually allied to all relicOwners).

To keep symmetry on player defeat reset the wonder counter

Test Plan

Agree these rules are what we want

Make random diplo changes like
A, B, C have relic and A<=>B and A <=>C, then no relic
A<=>B have relic and CD change diplo => no change
A<=>B have relic and C change to ally B but B stays war => no change, then B allies => reset
A<=>B have relic and A <=>C<=>B, C wins too, and if C changes diplo to A or B => reset
etc.

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

There are a very large number of changes, so older changes are hidden. Show Older Changes
bb mentioned this in D881: Add diplomacy changes support for wonder victory and add relevant messages.Dec 11 2017, 6:06 PM
elexis added a comment.Dec 28 2017, 4:40 PM
  • Me passes a free coupon to in a separate reviewed or unreviewed commit to
    • rename all functions to start with CaptureTheRelic
    • execute a unification of the timer + notification deletion
    • make the strings consistent with the wonder ones (team -> allies?)
  • Any idea how to rename this file to become agnostic of relics and make it more versatile to RTS in general welcome (do we dare to call it CaptureTheFlag? I guess not as long as we don't split the 0ad content from the simulation code)
binaries/data/mods/public/maps/scripts/CaptureTheRelic.js
53 ↗(On Diff #3937)

-the group

77 ↗(On Diff #3937)

earl continue or custom function?

elexis requested changes to this revision.Dec 28 2017, 8:45 PM
  • Correct to not use sets since we use array functions and need to pass an array to the GUI interface.
binaries/data/mods/public/maps/scripts/CaptureTheRelic.js
81 ↗(On Diff #3937)

this loop doesnt do anything

This revision now requires changes to proceed.Dec 28 2017, 8:45 PM
bb marked 3 inline comments as done.Jan 1 2018, 5:20 PM
  • Any idea how to rename this file to become agnostic of relics and make it more versatile to RTS in general welcome (do we dare to call it CaptureTheFlag? I guess not as long as we don't split the 0ad content from the simulation code)

Well if we want it really general it should be "OwnSpawnedEntities", Wonder will then be "OwnEntities", maybe the wonder and relic code could be merged a bit, but that requires some rewriting in both (we could have a triggerscript for just spawning the entities and another for the condition checking).

bb updated this revision to Diff 5021.Jan 1 2018, 5:21 PM
Vulcan added a comment.Jan 1 2018, 7:14 PM

Successful build - Chance fights ever on the side of the prudent.

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...
Vulcan added a comment.Jan 1 2018, 7:18 PM
Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/maps/scripts/CaptureTheRelic.js
|  81| »   »   »   playerAndAllies·=·playerAndAllies.filter(ally·=>·QueryPlayerIDInterface(owner).IsMutualAlly(ally))
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
Harbormaster completed remote builds in B4274: Diff 5021.Jan 1 2018, 7:18 PM
elexis added a comment.Jan 5 2018, 8:16 PM

Perhaps the entire logic could become shorter if we just keep track of the spawned entity IDs at startup, then ask for the owners when we need to test if the timer needs to be stopped or started and only keep a local players variable?

IMO you can commit a short fix as long as it works and doesn't increase the complexity. I agree with the proposed change of the victory condition definition (especially fixing that bug that the player with the smallest ID has an advantage).

binaries/data/mods/public/maps/scripts/CaptureTheRelic.js
51 ↗(On Diff #5021)

have
Maybe we can omit the individual player as it's one case of the second case.

52 ↗(On Diff #5021)

-Also
Don't make the reader resolve what "winning players" mean and derive what it means (the group of mutually allied relic owners, i.e. == it?).
Maybe point out that it also changes if an ally was added? (Since the original allies didn't actually change),. i.e. changes or extends?

67 ↗(On Diff #5021)

team was an incorrect word

81 ↗(On Diff #5021)

(Was wondering if it were possible to do that with a single map and possibly a some/every call, dunno)

89 ↗(On Diff #5021)

We have the same array comparison in one of Imaroks mod patches. Perhaps we should add a helper function doing only that length and some thing? (Once both copies are committed maybe)

bb marked 4 inline comments as done.Jan 7 2018, 8:21 PM
In D972#48515, @elexis wrote:

Perhaps the entire logic could become shorter if we just keep track of the spawned entity IDs at startup,

We already do that right?

then ask for the owners when we need to test if the timer needs to be stopped or started and only keep a local players variable?

don't think the logic would become shorter, ofc, onownershipchange we only need to check for the newOwner and always stop the timer, and a diplo change would only have effect when at least one of the parties has a relic, so we could omit the loop, but create two more functions.

binaries/data/mods/public/maps/scripts/CaptureTheRelic.js
51 ↗(On Diff #5021)

thought about omitting but wasn't sure

52 ↗(On Diff #5021)

not only the owners but also the players that are mutally allied to all of the owners

81 ↗(On Diff #5021)

no map, filter(every)

bb updated this revision to Diff 5168.Jan 7 2018, 8:21 PM
bb marked 3 inline comments as done.
bb added inline comments.
binaries/data/mods/public/maps/scripts/CaptureTheRelic.js
47 ↗(On Diff #5168)

maybe unneeded

67–68 ↗(On Diff #5168)

currently useless, but calling this function for a nonrelicOwner will bug, thus left the check in

Vulcan added a comment.Jan 8 2018, 2:01 AM

Successful build - Chance fights ever on the side of the prudent.

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...
Vulcan added a comment.Jan 8 2018, 2:02 AM
Executing section Default...
Executing section Source...
Executing section JS...
Harbormaster completed remote builds in B4373: Diff 5168.Jan 8 2018, 2:03 AM
bb planned changes to this revision.Jan 8 2018, 12:50 PM
bb added inline comments.
binaries/data/mods/public/maps/scripts/CaptureTheRelic.js
65 ↗(On Diff #5168)

we don't need that playerID

bb updated this revision to Diff 5198.Jan 9 2018, 9:33 PM

Nuke the player argument, but rely on the relicOwners array

bb added inline comments.Jan 9 2018, 9:36 PM
binaries/data/mods/public/maps/scripts/CaptureTheRelic.js
77–80 ↗(On Diff #5198)

ugly, a better suggestion is appreciated

77–83 ↗(On Diff #5198)

(notice that lms is supported with this too)

Vulcan added a comment.Jan 9 2018, 10:33 PM

Successful build - Chance fights ever on the side of the prudent.

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...
Vulcan added a comment.Jan 9 2018, 10:34 PM
Executing section Default...
Executing section Source...
Executing section JS...
Harbormaster completed remote builds in B4402: Diff 5198.Jan 9 2018, 10:34 PM
temple added a subscriber: temple.Feb 25 2018, 3:12 AM

I feel like the strings could be better here and in the wonder game type. That one says "x owns a wonder and x and their allies will win in time". Here we could say "x, y, z hold relics and x, y, z and their allies will win in time". (It's important to know who has the relics so we know who to attack.)
But I think it would be even better if we were explicit and listed the allies, because players might have forgotten their diplomacy stances and think they're an ally of the winning player. (I know the winning messages say "you will win", but the losing ones don't explicitly say "you will lose" so I think it's easy to imagine that a losing player could get confused.)

binaries/data/mods/public/maps/scripts/CaptureTheRelic.js
49 ↗(On Diff #5198)

data.to isn't used in that function

66 ↗(On Diff #5198)

don't inline?

75 ↗(On Diff #5198)

extra tabs

77–80 ↗(On Diff #5198)

Could create non-owners in same loop as owners,
then check that all owners are mutual allies of each other (and if not, quit),
then create allies from non-owners by filtering if they're allies of all owners.
So then we have an owners array and an allies array to use in the two-part message, and we can easily check if they've changed (if so, reset the timer). (Elements of the arrays are in order, so check that lengths are equal and the elements in each position are equal.)

98 ↗(On Diff #5198)

early return if (!this.relicsVictoryTimer)

99–100 ↗(On Diff #5198)

inline

141 ↗(On Diff #5198)

In diplomacy games, can we even tell the "team" a player is on? All we know is whether we're allied with a player or not, not how they're allied with each other.
We have the list of mutual allies, so it seems we could be explicit and list all the members (like we do in the victory message)?
Otherwise we could have a player mutually allied with the person mentioned in the message, so he thinks he's on that player's team, when actually he'll lose because he's not mutually allied with everyone on the "team".

elexis added a comment.Feb 25 2018, 12:20 PM

There are some catches about these string proposals.

  • Player 1 can hold all relics, but they could be distributed to all bases of his allies or out of territory altogether.
  • We should avoid redundancy of playernames, they might become too long easily. Especially if we go beyond 8 players sometime.
bb updated this revision to Diff 6010.Mar 3 2018, 12:02 AM
bb marked 6 inline comments as done.

Sad noone found the real bug :S

binaries/data/mods/public/maps/scripts/CaptureTheRelic.js
66 ↗(On Diff #5198)

getallplayers

Vulcan added a comment.Mar 3 2018, 2:18 AM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'allies'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
| 141| 141| 	cmpGUIInterface.PushNotification({
| 142| 142| 		"type": "won",
| 143| 143| 		"players": [winningPlayers[0]],
| 144|    |-		"allies" : winningPlayers,
|    | 144|+		"allies": winningPlayers,
| 145| 145| 		"message": victoryString(winningPlayers.length)
| 146| 146| 	});
| 147| 147| 
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'allies'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
| 149| 149| 		cmpGUIInterface.PushNotification({
| 150| 150| 			"type": "defeat",
| 151| 151| 			"players": [defeatedPlayers[0]],
| 152|    |-			"allies" : defeatedPlayers,
|    | 152|+			"allies": defeatedPlayers,
| 153| 153| 			"message": defeatString(defeatedPlayers.length)
| 154| 154| 		});
| 155| 155| };
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'allies'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
| 199| 199| 		cmpGuiInterface.PushNotification({
| 200| 200| 			"type": "won",
| 201| 201| 			"players": [allies[0]],
| 202|    |-			"allies" : allies,
|    | 202|+			"allies": allies,
| 203| 203| 			"message": markForPluralTranslation(
| 204| 204| 				"%(lastPlayer)s has won (last player alive).",
| 205| 205| 				"%(players)s and %(lastPlayer)s have won (last players alive).",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 4 tabs but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/scripts/WonderVictory.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/scripts/WonderVictory.js
|  92|  92| 					"players": [player],
|  93|  93| 					"translateMessage": true
|  94|  94| 				},
|  95|    |-			wonderDuration)
|    |  95|+				wonderDuration)
|  96|  96| 		]
|  97|  97| 	};
|  98|  98| };

Link to build: https://jenkins.wildfiregames.com/job/differential/139/display/redirect

Harbormaster completed remote builds in B5314: Diff 6010.Mar 3 2018, 2:18 AM
temple added a comment.Mar 3 2018, 2:27 AM

GetAllPlayers includes gaia, so probably want to make a GetNonGaiaPlayers function.

binaries/data/mods/public/maps/scripts/CaptureTheRelic.js
66–69 ↗(On Diff #6010)

could filter instead

binaries/data/mods/public/simulation/components/EndGameManager.js
95 ↗(On Diff #6010)

Rather than repeat this, we could

let winningPlayers = Engine.QueryInterface(SYSTEM_ENTITY, IID_PlayerManager).GetAllPlayers().filter(player => player == playerID || this.alliedVictory && QueryPlayerIDInterface(player).IsMutualAlly(playerID))

and then call the next function.

138 ↗(On Diff #6010)

Guess I'd go with win + lose or victory + defeat (winning/losingPlayers or victorious/defeatedPlayers), but whatever.

bb updated this revision to Diff 6015.Mar 3 2018, 4:12 PM
bb marked 2 inline comments as done.
bb added inline comments.
binaries/data/mods/public/simulation/components/EndGameManager.js
95 ↗(On Diff #6010)

NonGaia

138 ↗(On Diff #6010)

(function inlined)

Vulcan added a comment.Mar 3 2018, 4:18 PM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/components/PlayerManager.js
|  52| »   var·cmpPlayer·=·Engine.QueryInterface(ent,·IID_Player);
|    | [NORMAL] JSHintBear:
|    | 'cmpPlayer' is already defined.

binaries/data/mods/public/simulation/components/PlayerManager.js
|  61| »   for·(var·e·of·entities)
|    | [NORMAL] JSHintBear:
|    | 'e' is already defined.

binaries/data/mods/public/simulation/components/PlayerManager.js
|  63| »   »   var·cmpOwnership·=·Engine.QueryInterface(e,·IID_Ownership);
|    | [NORMAL] JSHintBear:
|    | 'cmpOwnership' is already defined.
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'allies'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
| 115| 115| 	cmpGUIInterface.PushNotification({
| 116| 116| 		"type": "won",
| 117| 117| 		"players": [winningPlayers[0]],
| 118|    |-		"allies" : winningPlayers,
|    | 118|+		"allies": winningPlayers,
| 119| 119| 		"message": victoryString(winningPlayers.length)
| 120| 120| 	});
| 121| 121| 
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'allies'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
| 123| 123| 		cmpGUIInterface.PushNotification({
| 124| 124| 			"type": "defeat",
| 125| 125| 			"players": [defeatedPlayers[0]],
| 126|    |-			"allies" : defeatedPlayers,
|    | 126|+			"allies": defeatedPlayers,
| 127| 127| 			"message": defeatString(defeatedPlayers.length)
| 128| 128| 		});
| 129| 129| 
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'allies'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
| 175| 175| 		cmpGuiInterface.PushNotification({
| 176| 176| 			"type": "won",
| 177| 177| 			"players": [allies[0]],
| 178|    |-			"allies" : allies,
|    | 178|+			"allies": allies,
| 179| 179| 			"message": markForPluralTranslation(
| 180| 180| 				"%(lastPlayer)s has won (last player alive).",
| 181| 181| 				"%(players)s and %(lastPlayer)s have won (last players alive).",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 4 tabs but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/scripts/WonderVictory.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/scripts/WonderVictory.js
|  92|  92| 					"players": [player],
|  93|  93| 					"translateMessage": true
|  94|  94| 				},
|  95|    |-			wonderDuration)
|    |  95|+				wonderDuration)
|  96|  96| 		]
|  97|  97| 	};
|  98|  98| };

Link to build: https://jenkins.wildfiregames.com/job/differential/143/display/redirect

Harbormaster completed remote builds in B5321: Diff 6015.Mar 3 2018, 4:18 PM
temple added a comment.Mar 3 2018, 6:49 PM

A lot of stuff in the old code can be cleaned up (inlined (but maybe I'm too obsessed with that), GetNonGaiaPlayers), but for another diff (or skip it, whatever).

binaries/data/mods/public/maps/scripts/CaptureTheRelic.js
67 ↗(On Diff #6015)

no newline?

74 ↗(On Diff #6015)

inline

84 ↗(On Diff #6015)

newline?

99 ↗(On Diff #6015)

just Engine.etc

binaries/data/mods/public/simulation/components/EndGameManager.js
112 ↗(On Diff #6015)

If a player isn't active, we don't set their state, but also we shouldn't include them in the won/defeat message. So I guess start with activePlayers then filter to get winningPlayers and defeatedPlayers from that (or however you want to do it).

bb marked 5 inline comments as done.Mar 3 2018, 7:01 PM

indeed the new function can be used in many places, but that runs out of scope...

bb updated this revision to Diff 6017.Mar 3 2018, 7:02 PM
bb added inline comments.Mar 3 2018, 7:07 PM
binaries/data/mods/public/simulation/components/EndGameManager.js
108–109 ↗(On Diff #6015)

should be removed (missing save)

Vulcan added a comment.Mar 3 2018, 7:11 PM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'allies'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
| 115| 115| 	cmpGUIInterface.PushNotification({
| 116| 116| 		"type": "won",
| 117| 117| 		"players": [winningPlayers[0]],
| 118|    |-		"allies" : winningPlayers,
|    | 118|+		"allies": winningPlayers,
| 119| 119| 		"message": victoryString(winningPlayers.length)
| 120| 120| 	});
| 121| 121| 
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'allies'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
| 123| 123| 		cmpGUIInterface.PushNotification({
| 124| 124| 			"type": "defeat",
| 125| 125| 			"players": [defeatedPlayers[0]],
| 126|    |-			"allies" : defeatedPlayers,
|    | 126|+			"allies": defeatedPlayers,
| 127| 127| 			"message": defeatString(defeatedPlayers.length)
| 128| 128| 		});
| 129| 129| 
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'allies'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
| 175| 175| 		cmpGuiInterface.PushNotification({
| 176| 176| 			"type": "won",
| 177| 177| 			"players": [allies[0]],
| 178|    |-			"allies" : allies,
|    | 178|+			"allies": allies,
| 179| 179| 			"message": markForPluralTranslation(
| 180| 180| 				"%(lastPlayer)s has won (last player alive).",
| 181| 181| 				"%(players)s and %(lastPlayer)s have won (last players alive).",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 4 tabs but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/scripts/WonderVictory.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/scripts/WonderVictory.js
|  92|  92| 					"players": [player],
|  93|  93| 					"translateMessage": true
|  94|  94| 				},
|  95|    |-			wonderDuration)
|    |  95|+				wonderDuration)
|  96|  96| 		]
|  97|  97| 	};
|  98|  98| };

binaries/data/mods/public/simulation/components/PlayerManager.js
|  52| »   var·cmpPlayer·=·Engine.QueryInterface(ent,·IID_Player);
|    | [NORMAL] JSHintBear:
|    | 'cmpPlayer' is already defined.

binaries/data/mods/public/simulation/components/PlayerManager.js
|  61| »   for·(var·e·of·entities)
|    | [NORMAL] JSHintBear:
|    | 'e' is already defined.

binaries/data/mods/public/simulation/components/PlayerManager.js
|  63| »   »   var·cmpOwnership·=·Engine.QueryInterface(e,·IID_Ownership);
|    | [NORMAL] JSHintBear:
|    | 'cmpOwnership' is already defined.

Link to build: https://jenkins.wildfiregames.com/job/differential/145/display/redirect

Harbormaster completed remote builds in B5324: Diff 6017.Mar 3 2018, 7:11 PM
bb updated this revision to Diff 6018.Mar 3 2018, 8:14 PM
Vulcan added a comment.Mar 3 2018, 9:28 PM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 4 tabs but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/scripts/WonderVictory.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/scripts/WonderVictory.js
|  92|  92| 					"players": [player],
|  93|  93| 					"translateMessage": true
|  94|  94| 				},
|  95|    |-			wonderDuration)
|    |  95|+				wonderDuration)
|  96|  96| 		]
|  97|  97| 	};
|  98|  98| };
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'allies'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
| 109| 109| 	cmpGUIInterface.PushNotification({
| 110| 110| 		"type": "won",
| 111| 111| 		"players": [winningPlayers[0]],
| 112|    |-		"allies" : winningPlayers,
|    | 112|+		"allies": winningPlayers,
| 113| 113| 		"message": victoryString(winningPlayers.length)
| 114| 114| 	});
| 115| 115| 
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'allies'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
| 117| 117| 		cmpGUIInterface.PushNotification({
| 118| 118| 			"type": "defeat",
| 119| 119| 			"players": [defeatedPlayers[0]],
| 120|    |-			"allies" : defeatedPlayers,
|    | 120|+			"allies": defeatedPlayers,
| 121| 121| 			"message": defeatString(defeatedPlayers.length)
| 122| 122| 		});
| 123| 123| 
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'allies'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
| 169| 169| 		cmpGuiInterface.PushNotification({
| 170| 170| 			"type": "won",
| 171| 171| 			"players": [allies[0]],
| 172|    |-			"allies" : allies,
|    | 172|+			"allies": allies,
| 173| 173| 			"message": markForPluralTranslation(
| 174| 174| 				"%(lastPlayer)s has won (last player alive).",
| 175| 175| 				"%(players)s and %(lastPlayer)s have won (last players alive).",

binaries/data/mods/public/simulation/components/PlayerManager.js
|  52| »   var·cmpPlayer·=·Engine.QueryInterface(ent,·IID_Player);
|    | [NORMAL] JSHintBear:
|    | 'cmpPlayer' is already defined.

binaries/data/mods/public/simulation/components/PlayerManager.js
|  61| »   for·(var·e·of·entities)
|    | [NORMAL] JSHintBear:
|    | 'e' is already defined.

binaries/data/mods/public/simulation/components/PlayerManager.js
|  63| »   »   var·cmpOwnership·=·Engine.QueryInterface(e,·IID_Ownership);
|    | [NORMAL] JSHintBear:
|    | 'cmpOwnership' is already defined.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 0.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/scripts/CaptureTheRelic.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/scripts/CaptureTheRelic.js
|  30|  30| 
|  31|  31| Trigger.prototype.CheckCaptureTheRelicVictory = function(data)
|  32|  32| {
|  33|    |-warn("P")
|    |  33|+	warn("P")
|  34|  34| 	let cmpIdentity = Engine.QueryInterface(data.entity, IID_Identity);
|  35|  35| 	if (!cmpIdentity || !cmpIdentity.HasClass("Relic") || data.from == INVALID_PLAYER)
|  36|  36| 		return;
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/scripts/CaptureTheRelic.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/scripts/CaptureTheRelic.js
|  30|  30| 
|  31|  31| Trigger.prototype.CheckCaptureTheRelicVictory = function(data)
|  32|  32| {
|  33|    |-warn("P")
|    |  33|+warn("P");
|  34|  34| 	let cmpIdentity = Engine.QueryInterface(data.entity, IID_Identity);
|  35|  35| 	if (!cmpIdentity || !cmpIdentity.HasClass("Relic") || data.from == INVALID_PLAYER)
|  36|  36| 		return;

binaries/data/mods/public/maps/scripts/CaptureTheRelic.js
|  33| warn("P")
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

Link to build: https://jenkins.wildfiregames.com/job/differential/146/display/redirect

Harbormaster completed remote builds in B5327: Diff 6018.Mar 3 2018, 9:28 PM
temple added a comment.Mar 3 2018, 10:09 PM

I'm okay with resetting the timer when allied players are defeated, i.e. to treat that the same as a diplomacy change. But if so then we should change wonder victories to also reset the timer when allied players are defeated, since we do that when the diplomacy changes. (Right now it'll throw a warning and still include them in the victory message if a player has resigned but is allied to the wonder victory player.)

Alternative is to ignore player defeated messages and then in MarkPlayerAndAlliesAsWon filter out inactive players on the winning side too. I'd be okay with this too.

binaries/data/mods/public/maps/scripts/CaptureTheRelic.js
33 ↗(On Diff #6018)

no

binaries/data/mods/public/simulation/components/EndGameManager.js
106 ↗(On Diff #6018)

QueryPlayerIDInterface(playerID)

bb updated this revision to Diff 6024.Mar 3 2018, 11:08 PM
bb marked 2 inline comments as done.
bb retitled this revision from Support diplomacy allied victory for capture the relic to Support diplomacy allied victory for capture the relic and reset wonder counter on player defeat.
bb edited the summary of this revision. (Show Details)
Vulcan added a comment.Mar 3 2018, 11:10 PM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'allies'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
| 110| 110| 	cmpGUIInterface.PushNotification({
| 111| 111| 		"type": "won",
| 112| 112| 		"players": [winningPlayers[0]],
| 113|    |-		"allies" : winningPlayers,
|    | 113|+		"allies": winningPlayers,
| 114| 114| 		"message": victoryString(winningPlayers.length)
| 115| 115| 	});
| 116| 116| 
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'allies'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
| 118| 118| 		cmpGUIInterface.PushNotification({
| 119| 119| 			"type": "defeat",
| 120| 120| 			"players": [defeatedPlayers[0]],
| 121|    |-			"allies" : defeatedPlayers,
|    | 121|+			"allies": defeatedPlayers,
| 122| 122| 			"message": defeatString(defeatedPlayers.length)
| 123| 123| 		});
| 124| 124| 
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'allies'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/EndGameManager.js
| 170| 170| 		cmpGuiInterface.PushNotification({
| 171| 171| 			"type": "won",
| 172| 172| 			"players": [allies[0]],
| 173|    |-			"allies" : allies,
|    | 173|+			"allies": allies,
| 174| 174| 			"message": markForPluralTranslation(
| 175| 175| 				"%(lastPlayer)s has won (last player alive).",
| 176| 176| 				"%(players)s and %(lastPlayer)s have won (last players alive).",

binaries/data/mods/public/simulation/components/PlayerManager.js
|  52| »   var·cmpPlayer·=·Engine.QueryInterface(ent,·IID_Player);
|    | [NORMAL] JSHintBear:
|    | 'cmpPlayer' is already defined.

binaries/data/mods/public/simulation/components/PlayerManager.js
|  61| »   for·(var·e·of·entities)
|    | [NORMAL] JSHintBear:
|    | 'e' is already defined.

binaries/data/mods/public/simulation/components/PlayerManager.js
|  63| »   »   var·cmpOwnership·=·Engine.QueryInterface(e,·IID_Ownership);
|    | [NORMAL] JSHintBear:
|    | 'cmpOwnership' is already defined.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 4 tabs but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/scripts/WonderVictory.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/scripts/WonderVictory.js
|  92|  92| 					"players": [player],
|  93|  93| 					"translateMessage": true
|  94|  94| 				},
|  95|    |-			wonderDuration)
|    |  95|+				wonderDuration)
|  96|  96| 		]
|  97|  97| 	};
|  98|  98| };

Link to build: https://jenkins.wildfiregames.com/job/differential/149/display/redirect

Harbormaster completed remote builds in B5332: Diff 6024.Mar 3 2018, 11:10 PM
temple accepted this revision.Mar 4 2018, 1:50 AM

Think it's okay after these changes.

binaries/data/mods/public/maps/scripts/CaptureTheRelic.js
48 ↗(On Diff #6024)

Remove, since it's possible relics get destroyed and we want to check in that case too. (If a timer was going we probably want to reset it in that case, so we can keep the delete messages line above.)

99 ↗(On Diff #6024)

and then this.relicsVictoryTimer = undefined;

bb added inline comments.Mar 5 2018, 6:42 PM
binaries/data/mods/public/maps/scripts/CaptureTheRelic.js
48 ↗(On Diff #6024)

its already a bug if relics get destroyed, so doing whatever is correct, thus going with the shortest-code-way

This revision was not accepted when it landed; it landed in state Needs Review.Mar 5 2018, 7:02 PM
Closed by commit rP21441: Create winning teams for relic and wonder victory instead of letting all allies… (authored by bb). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
ps/
trunk/
binaries/
data/
mods/
public/
maps/
scripts/
85 lines
2 lines
14 lines
simulation/
components/
51 lines
11 lines

Diff 6041

View Options

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

Loading...
View Options

ps/trunk/binaries/data/mods/public/maps/scripts/TriggerHelper.js

Loading...
View Options

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

Loading...
View Options

ps/trunk/binaries/data/mods/public/simulation/components/EndGameManager.js

Loading...
View Options

ps/trunk/binaries/data/mods/public/simulation/components/PlayerManager.js

Loading...
Wildfire Games · Phabricator