Recompute values affected by modifications properly
ClosedPublic
Actions

Authored by Silier on Apr 22 2020, 10:08 PM.

Details

Reviewers

None

Group Reviewers

Restricted Owners Package

(Owns No Changed Paths)

Commits

rP23633: Fix Capture points not initialised properly.

Trac Tickets

#5712

Summary

Reported by Freagarach that modifications are not correctly applied to capture points following rP22767.
Regeneration rate change affected by aura or researched technology would not trigger timer if not decaying to territory and number of capture points stayed unaffected.
Number of capture points would not be affected after player would capture entity but would be if player researches technology after that.

Test Plan

Test that capture points do have correct values when they are created or captured.

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

Silier created this revision.Apr 22 2020, 10:08 PM

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (spaced-comment):
|    | Expected space or tab after '//' in comment.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Capturable.js
|    |++++| /zpool0/trunk/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.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Capturable.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Capturable.js
| 145| 145| 	return sourceEnemyCp > 0;
| 146| 146| };
| 147| 147| 
| 148|    |-//// Private functions ////
|    | 148|+// // Private functions ////
| 149| 149| 
| 150| 150| /**
| 151| 151|  * This has to be called whenever the capture points are changed.
|    | [NORMAL] ESLintBear (spaced-comment):
|    | Expected space or tab after '//' in comment.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Capturable.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Capturable.js
| 260| 260| 	Engine.PostMessage(this.entity, MT_CaptureRegenStateChanged, { "regenerating": true, "regenRate": regenRate, "territoryDecay": decay });
| 261| 261| };
| 262| 262| 
| 263|    |-//// Message Listeners ////
|    | 263|+// // Message Listeners ////
| 264| 264| 
| 265| 265| Capturable.prototype.RecalculateValues = function()
| 266| 266| {
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Capturable.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Capturable.js
| 273| 273| 	for (let i in this.cp)
| 274| 274| 		this.cp[i] *= scale;
| 275| 275| 	return true;
| 276|    |-}
|    | 276|+};
| 277| 277| 
| 278| 278| Capturable.prototype.OnValueModification = function(msg)
| 279| 279| {

binaries/data/mods/public/simulation/components/Capturable.js
| 310| »   »   if·(!this.cp[msg.from])·{
|    | [NORMAL] ESLintBear (brace-rules/brace-on-same-line):
|    | Opening curly brace appears on the same line as controlling statement.

binaries/data/mods/public/simulation/components/Capturable.js
| 192| »   »   var·garrisonRegenRate·=·0;
|    | [NORMAL] JSHintBear:
|    | 'garrisonRegenRate' is already defined.

binaries/data/mods/public/simulation/components/Capturable.js
| 194| »   return·regenRate·+·garrisonRegenRate;
|    | [NORMAL] JSHintBear:
|    | 'garrisonRegenRate' used out of scope.

binaries/data/mods/public/simulation/components/Capturable.js
| 276| }
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2025/display/redirect

Harbormaster completed remote builds in B11487: Diff 11707.Apr 22 2020, 10:24 PM

elexis edited the summary of this revision. (Show Details)Apr 23 2020, 12:08 AM

Freagarach added a subscriber: Freagarach.Apr 23 2020, 7:46 AM

Freagarach added inline comments.

binaries/data/mods/public/simulation/components/Capturable.js
284 ↗	(On Diff #11707)	Why would we need to check the timer when only the max has changed? Perhaps because of the scaling, but that means one should also check `RegisterCapturePointsChanged`, which could in turn incorporate the timer check? What I'm saying: Perhaps we can unify some functions ^^

Stan added a subscriber: Stan.Apr 23 2020, 8:57 AM

Stan added inline comments.

binaries/data/mods/public/simulation/components/Capturable.js
310 ↗	(On Diff #11707)	Newline. Perhaps this block could be refactored, as there are two calls to recalculate values?

Silier planned changes to this revision.Apr 24 2020, 10:15 AM

Silier added inline comments.

binaries/data/mods/public/simulation/components/Capturable.js
284 ↗	(On Diff #11707)	because regen rate could change and seems like there is another bug about that if capture points where not affected but regen rate was

Silier updated this revision to Diff 11713.Apr 25 2020, 7:41 PM

Silier retitled this revision from Initialise capture points properly and scale them properly when changing owner to Recompute values affected by modifications properly.

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

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (spaced-comment):
|    | Expected space or tab after '//' in comment.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Capturable.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Capturable.js
|  20|  20| 	this.cp = [];
|  21|  21| };
|  22|  22| 
|  23|    |-//// Interface functions ////
|    |  23|+// // Interface functions ////
|  24|  24| 
|  25|  25| /**
|  26|  26|  * Returns the current capture points array
|    | [NORMAL] ESLintBear (spaced-comment):
|    | Expected space or tab after '//' in comment.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Capturable.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Capturable.js
| 147| 147| 	return sourceEnemyCp > 0;
| 148| 148| };
| 149| 149| 
| 150|    |-//// Private functions ////
|    | 150|+// // Private functions ////
| 151| 151| 
| 152| 152| /**
| 153| 153|  * This has to be called whenever the capture points are changed.
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Capturable.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Capturable.js
| 266| 266| 	this.garrisonRegenRate = ApplyValueModificationsToEntity("Capturable/GarrisonRegenRate", +this.template.GarrisonRegenRate, this.entity);
| 267| 267| 	this.regenRate = ApplyValueModificationsToEntity("Capturable/RegenRate", +this.template.RegenRate, this.entity);
| 268| 268| 	this.maxCp = ApplyValueModificationsToEntity("Capturable/CapturePoints", +this.template.CapturePoints, this.entity);
| 269|    |-}
|    | 269|+};
| 270| 270| 
| 271| 271| /**
| 272| 272|  * Update all chached values that could be affected by modifications.
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Capturable.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Capturable.js
| 292| 292| 
| 293| 293| 	if (oldGarrosionRegenRate != this.garrisonRegenRate || oldRegenRate != this.regenRate)
| 294| 294| 		this.CheckTimer();
| 295|    |-}
|    | 295|+};
| 296| 296| 
| 297| 297| //// Message Listeners ////
| 298| 298| 
|    | [NORMAL] ESLintBear (spaced-comment):
|    | Expected space or tab after '//' in comment.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Capturable.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Capturable.js
| 294| 294| 		this.CheckTimer();
| 295| 295| }
| 296| 296| 
| 297|    |-//// Message Listeners ////
|    | 297|+// // Message Listeners ////
| 298| 298| 
| 299| 299| Capturable.prototype.OnValueModification = function(msg)
| 300| 300| {

binaries/data/mods/public/simulation/components/Capturable.js
| 269| }
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

binaries/data/mods/public/simulation/components/Capturable.js
| 295| }
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2026/display/redirect

Harbormaster completed remote builds in B11494: Diff 11713.Apr 25 2020, 7:51 PM

Silier added a reviewer: Restricted Owners Package.Apr 26 2020, 3:34 PM

This revision was not accepted when it landed; it landed in state Needs Review.May 7 2020, 6:46 PM

Closed by commit rP23633: Fix Capture points not initialised properly. (authored by Silier). · Explain Why

This revision was automatically updated to reflect the committed changes.

Silier added a commit: rP23633: Fix Capture points not initialised properly..