Details
- Reviewers
bb - Commits
- rP21685: Give capturePoints of defeated Players to gaia
Test on the autostart from #5115 (either visually, or by looking at the printed statistics at the end: without the patch, the defeated player 2 still controls some 20% of the map, while it does not controls anything with the patch).
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- /ps/trunk
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 5801 Build 9724: Vulcan Build Jenkins Build 9723: arc lint + arc unit
Event Timeline
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 (comma-spacing): | | A space is required after ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | 14| 14| "structure": 20, | 15| 15| "playerID": 1, | 16| 16| "regenRate": 2, | 17| |- "garrisonedEntities": [30,31,32,33], | | 17|+ "garrisonedEntities": [30, 31,32,33], | 18| 18| "garrisonRegenRate": 5, | 19| 19| "decay": false, | 20| 20| "decayRate": 30, | | [NORMAL] ESLintBear (comma-spacing): | | A space is required after ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | 14| 14| "structure": 20, | 15| 15| "playerID": 1, | 16| 16| "regenRate": 2, | 17| |- "garrisonedEntities": [30,31,32,33], | | 17|+ "garrisonedEntities": [30,31, 32,33], | 18| 18| "garrisonRegenRate": 5, | 19| 19| "decay": false, | 20| 20| "decayRate": 30, | | [NORMAL] ESLintBear (comma-spacing): | | A space is required after ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | 14| 14| "structure": 20, | 15| 15| "playerID": 1, | 16| 16| "regenRate": 2, | 17| |- "garrisonedEntities": [30,31,32,33], | | 17|+ "garrisonedEntities": [30,31,32, 33], | 18| 18| "garrisonRegenRate": 5, | 19| 19| "decay": false, | 20| 20| "decayRate": 30, | | [NORMAL] ESLintBear (key-spacing): | | Extra space after key 'LostEntity'. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | 66| 66| }); | 67| 67| | 68| 68| AddMock(testData.structure, IID_StatisticsTracker, { | 69| |- "LostEntity" : () => {}, | | 69|+ "LostEntity": () => {}, | 70| 70| "CapturedBuilding": () => {} | 71| 71| }); | 72| 72| | | [NORMAL] ESLintBear (key-spacing): | | Extra space after key 'CapturePoints'. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | 71| 71| }); | 72| 72| | 73| 73| let cmpCapturable = ConstructComponent(testData.structure, "Capturable", { | 74| |- "CapturePoints" : testData.maxCp, | | 74|+ "CapturePoints": testData.maxCp, | 75| 75| "RegenRate" : testData.regenRate, | 76| 76| "GarrisonRegenRate" : testData.garrisonRegenRate | 77| 77| }); | | [NORMAL] ESLintBear (key-spacing): | | Extra space after key 'RegenRate'. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | 72| 72| | 73| 73| let cmpCapturable = ConstructComponent(testData.structure, "Capturable", { | 74| 74| "CapturePoints" : testData.maxCp, | 75| |- "RegenRate" : testData.regenRate, | | 75|+ "RegenRate": testData.regenRate, | 76| 76| "GarrisonRegenRate" : testData.garrisonRegenRate | 77| 77| }); | 78| 78| | | [NORMAL] ESLintBear (key-spacing): | | Extra space after key 'GarrisonRegenRate'. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | 73| 73| let cmpCapturable = ConstructComponent(testData.structure, "Capturable", { | 74| 74| "CapturePoints" : testData.maxCp, | 75| 75| "RegenRate" : testData.regenRate, | 76| |- "GarrisonRegenRate" : testData.garrisonRegenRate | | 76|+ "GarrisonRegenRate": testData.garrisonRegenRate | 77| 77| }); | 78| 78| | 79| 79| AddMock(testData.structure, IID_TerritoryDecay, { | | [NORMAL] ESLintBear (comma-spacing): | | There should be no space before ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | 90| 90| // Tests initialisation of the capture points when the entity is created | 91| 91| testCapturable(testData, cmpCapturable => { | 92| 92| Engine.PostMessage = function(ent, iid, message) { | 93| |- TS_ASSERT_UNEVAL_EQUALS(message, { "regenerating": true, "regenRate": cmpCapturable.GetRegenRate() , "territoryDecay": 0 }); | | 93|+ TS_ASSERT_UNEVAL_EQUALS(message, { "regenerating": true, "regenRate": cmpCapturable.GetRegenRate(), "territoryDecay": 0 }); | 94| 94| }; | 95| 95| cmpCapturable.OnOwnershipChanged({ "from": INVALID_PLAYER, "to": testData.playerID }); | 96| 96| TS_ASSERT_UNEVAL_EQUALS(cmpCapturable.GetCapturePoints(), [0, 3000, 0, 0]); | | [NORMAL] ESLintBear (comma-spacing): | | There should be no space before ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | 98| 98| | 99| 99| // Tests if the message is sent when capture points change | 100| 100| testCapturable(testData, cmpCapturable => { | 101| |- cmpCapturable.SetCapturePoints([0, 2000, 0 , 1000]); | | 101|+ cmpCapturable.SetCapturePoints([0, 2000, 0, 1000]); | 102| 102| TS_ASSERT_UNEVAL_EQUALS(cmpCapturable.GetCapturePoints(), [0, 2000, 0 , 1000]); | 103| 103| Engine.PostMessage = function(ent, iid, message) | 104| 104| { | | [NORMAL] ESLintBear (comma-spacing): | | There should be no space before ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | 99| 99| // Tests if the message is sent when capture points change | 100| 100| testCapturable(testData, cmpCapturable => { | 101| 101| cmpCapturable.SetCapturePoints([0, 2000, 0 , 1000]); | 102| |- TS_ASSERT_UNEVAL_EQUALS(cmpCapturable.GetCapturePoints(), [0, 2000, 0 , 1000]); | | 102|+ TS_ASSERT_UNEVAL_EQUALS(cmpCapturable.GetCapturePoints(), [0, 2000, 0, 1000]); | 103| 103| Engine.PostMessage = function(ent, iid, message) | 104| 104| { | 105| 105| TS_ASSERT_UNEVAL_EQUALS(message, { "capturePoints": [0, 2000, 0 , 1000] }); | | [NORMAL] ESLintBear (comma-spacing): | | There should be no space before ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | 102| 102| TS_ASSERT_UNEVAL_EQUALS(cmpCapturable.GetCapturePoints(), [0, 2000, 0 , 1000]); | 103| 103| Engine.PostMessage = function(ent, iid, message) | 104| 104| { | 105| |- TS_ASSERT_UNEVAL_EQUALS(message, { "capturePoints": [0, 2000, 0 , 1000] }); | | 105|+ TS_ASSERT_UNEVAL_EQUALS(message, { "capturePoints": [0, 2000, 0, 1000] }); | 106| 106| }; | 107| 107| cmpCapturable.RegisterCapturePointsChanged(); | 108| 108| }); | | [NORMAL] ESLintBear (comma-spacing): | | There should be no space before ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | 109| 109| | 110| 110| // Tests reducing capture points (after a capture attack or a decay) | 111| 111| testCapturable(testData, cmpCapturable => { | 112| |- cmpCapturable.SetCapturePoints([0, 2000, 0 , 1000]); | | 112|+ cmpCapturable.SetCapturePoints([0, 2000, 0, 1000]); | 113| 113| cmpCapturable.CheckTimer(); | 114| 114| Engine.PostMessage = function(ent, iid, message) { | 115| 115| if (iid == MT_CapturePointsChanged) | | [NORMAL] ESLintBear (comma-spacing): | | There should be no space before ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | 123| 123| | 124| 124| // Tests reducing capture points (after a capture attack or a decay) | 125| 125| testCapturable(testData, cmpCapturable => { | 126| |- cmpCapturable.SetCapturePoints([0, 2000, 0 , 1000]); | | 126|+ cmpCapturable.SetCapturePoints([0, 2000, 0, 1000]); | 127| 127| cmpCapturable.CheckTimer(); | 128| 128| TS_ASSERT_EQUALS(cmpCapturable.Reduce(2500, 3),2000); | 129| 129| TS_ASSERT_UNEVAL_EQUALS(cmpCapturable.GetCapturePoints(), [0, 0, 0, 3000]); | | [NORMAL] ESLintBear (comma-spacing): | | A space is required after ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | 125| 125| testCapturable(testData, cmpCapturable => { | 126| 126| cmpCapturable.SetCapturePoints([0, 2000, 0 , 1000]); | 127| 127| cmpCapturable.CheckTimer(); | 128| |- TS_ASSERT_EQUALS(cmpCapturable.Reduce(2500, 3),2000); | | 128|+ TS_ASSERT_EQUALS(cmpCapturable.Reduce(2500, 3), 2000); | 129| 129| TS_ASSERT_UNEVAL_EQUALS(cmpCapturable.GetCapturePoints(), [0, 0, 0, 3000]); | 130| 130| }); | 131| 131| | | [NORMAL] ESLintBear (comma-spacing): | | There should be no space before ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | 144| 144| } | 145| 145| | 146| 146| // With our testData, the total regen rate is 22. That should be taken from the ennemies | 147| |-testRegen(testData, [12, 2950, 2 , 36], [1, 2972, 2, 25], true); | | 147|+testRegen(testData, [12, 2950, 2, 36], [1, 2972, 2, 25], true); | 148| 148| testRegen(testData, [0, 2994, 2, 4], [0, 2998, 2, 0], true); | 149| 149| testRegen(testData, [0, 2998, 2, 0], [0, 2998, 2, 0], false); | 150| 150| | | [NORMAL] ESLintBear (comma-spacing): | | There should be no space before ','. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/tests/test_Capturable.js | 176| 176| function testReduce(testData, amount, player, taken) | 177| 177| { | 178| 178| testCapturable(testData, cmpCapturable => { | 179| |- cmpCapturable.SetCapturePoints([0, 2000, 0 , 1000]); | | 179|+ cmpCapturable.SetCapturePoints([0, 2000, 0, 1000]); | 180| 180| cmpCapturable.CheckTimer(); | 181| 181| TS_ASSERT_UNEVAL_EQUALS(cmpCapturable.Reduce(amount, player), taken); | 182| 182| }); binaries/data/mods/public/simulation/components/tests/test_Capturable.js | 25| function·testCapturable(testData,·test_function) | | [NORMAL] ESLintBear (no-shadow): | | 'testData' is already declared in the upper scope. binaries/data/mods/public/simulation/components/tests/test_Capturable.js | 132| function·testRegen(testData,·cpIn,·cpOut,·regenerating) | | [NORMAL] ESLintBear (no-shadow): | | 'testData' is already declared in the upper scope. binaries/data/mods/public/simulation/components/tests/test_Capturable.js | 157| function·testDecay(testData,·cpIn,·cpOut) | | [NORMAL] ESLintBear (no-shadow): | | 'testData' is already declared in the upper scope. binaries/data/mods/public/simulation/components/tests/test_Capturable.js | 176| function·testReduce(testData,·amount,·player,·taken) | | [NORMAL] ESLintBear (no-shadow): | | 'testData' is already declared in the upper scope. | | [NORMAL] ESLintBear (spaced-comment): | | Expected space or tab after '//' in comment. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Capturable.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Capturable.js | 18| 18| this.cp = []; | 19| 19| }; | 20| 20| | 21| |-//// Interface functions //// | | 21|+// // Interface functions //// | 22| 22| | 23| 23| /** | 24| 24| * Returns the current capture points array | | [NORMAL] ESLintBear (spaced-comment): | | Expected space or tab after '//' in comment. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Capturable.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Capturable.js | 127| 127| return sourceEnemyCp > 0; | 128| 128| }; | 129| 129| | 130| |-//// Private functions //// | | 130|+// // Private functions //// | 131| 131| | 132| 132| /** | 133| 133| * This has to be called whenever the capture points are changed. | | [NORMAL] ESLintBear (spaced-comment): | | Expected space or tab after '//' in comment. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Capturable.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Capturable.js | 242| 242| Engine.PostMessage(this.entity, MT_CaptureRegenStateChanged, { "regenerating": true, "regenRate": regenRate, "territoryDecay": decay }); | 243| 243| }; | 244| 244| | 245| |-//// Message Listeners //// | | 245|+// // Message Listeners //// | 246| 246| | 247| 247| Capturable.prototype.OnValueModification = function(msg) | 248| 248| { binaries/data/mods/public/simulation/components/Capturable.js | 174| » » var·garrisonRegenRate·=·0; | | [NORMAL] JSHintBear: | | 'garrisonRegenRate' is already defined. binaries/data/mods/public/simulation/components/Capturable.js | 176| » return·regenRate·+·garrisonRegenRate; | | [NORMAL] JSHintBear: | | 'garrisonRegenRate' used out of scope.
Link to build: https://jenkins.wildfiregames.com/job/differential/366/display/redirect
works as expected
Listening to the playerDefeat messages seems to be the way to go
regarding not doing the own entities here but in ownershipchange: the other way around would be possible too (not doing the ownershipchange for capturable entities), but that doesn't seem any better, so ok with this approach.
binaries/data/mods/public/simulation/components/Capturable.js | ||
---|---|---|
134 | period while at it | |
311 | period |
Thanks for the review.
Yes, i've hesitated, but
- onOwnershipChanged already dealt with the case owned
- i was not sure of possible side-effect depending on the order the messages were received: switching the ownership while the cp of the new owner were 0, or putting the cp of the owner to 0 without changing the owner! Doing both at the same time looked safer to me.