Cleanup of Capturable
var => let
periods, caps
JSdocs where useful
don't get the hitpoints twice
Details
Details
- Reviewers
- None
- Commits
- rP23875: Cleanup of Capturable component
Let the bots run
Diff Detail
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
| binaries/data/mods/public/simulation/components/Capturable.js | ||
|---|---|---|
| 205 ↗ | (On Diff #12403) | One too much? |
| binaries/data/mods/public/simulation/components/Capturable.js | ||
|---|---|---|
| 47 ↗ | (On Diff #12403) | with |
| 56 ↗ | (On Diff #12403) | capture points? |
| 59 ↗ | (On Diff #12403) | multiplied |
| 60 ↗ | (On Diff #12403) | s/,/. |
| 182 ↗ | (On Diff #12403) | Perhaps "best player" could be elaborated ;) |
| 230 ↗ | (On Diff #12403) | Comment on top? |
| 246 ↗ | (On Diff #12403) | Comment necessary? |
| 249 ↗ | (On Diff #12403) | Delete? |
| 337 ↗ | (On Diff #12403) | Comment on top? |
| 355–356 ↗ | (On Diff #12403) | (Seems like one sentence?) |
| binaries/data/mods/public/simulation/components/Capturable.js | ||
|---|---|---|
| 16 ↗ | (On Diff #12407) | Comment seems outdated (also recalculated onValueModification). Is it needed? |
| 57 ↗ | (On Diff #12407) | capturing? Do you "attack" an entity of your ally when taking capture points? |
| 136 ↗ | (On Diff #12407) | let? |
| 151 ↗ | (On Diff #12407) | let? |
| 214 ↗ | (On Diff #12407) | let |
| 60 ↗ | (On Diff #12403) | Ah yeah,,, |
| binaries/data/mods/public/simulation/components/Capturable.js | ||
|---|---|---|
| 183 ↗ | (On Diff #12407) | Maybe it could be this.cp.filter(cp => cp >= this.cp[bestPlayer]).length |
Comment Actions
Note that I deliberately avoided the cmpOwnership.GetOwner calls, the need a slightly more subtle care
Comment Actions
Build failure - The Moirai have given mortals hearts that can endure.
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
| 19| 19| this.cp = [];
| 20| 20| };
| 21| 21|
| 22| |-//// Interface functions ////
| | 22|+// // Interface functions ////
| 23| 23|
| 24| 24| /**
| 25| 25| * 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
| 159| 159| return sourceEnemyCp > 0;
| 160| 160| };
| 161| 161|
| 162| |-//// Private functions ////
| | 162|+// // Private functions ////
| 163| 163|
| 164| 164| /**
| 165| 165| * 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
| 301| 301| this.CheckTimer();
| 302| 302| };
| 303| 303|
| 304| |-//// Message Listeners ////
| | 304|+// // Message Listeners ////
| 305| 305|
| 306| 306| Capturable.prototype.OnValueModification = function(msg)
| 307| 307| {
Executing section cli...Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2510/display/redirect
| binaries/data/mods/public/simulation/components/Capturable.js | ||
|---|---|---|
| 46 ↗ | (On Diff #12436) | I'd prefer you explicated that this is one value per player, and if one must pass the player? Or just pass one value for all players. |
| 69 ↗ | (On Diff #12436) | I'm coming up with bonusMultiplier *= 0.1 + 0.9 * hitpoints / cmpHealth.GetMaxHitpoints() when I do the factoring? |
| 185 ↗ | (On Diff #12436) | Have you profiled this to be faster? I would call it less readable. |
| binaries/data/mods/public/simulation/components/Capturable.js | ||
|---|---|---|
| 69 ↗ | (On Diff #12436) | bonusMultiplier *= cmpHealth.GetMaxHitpoints() / (0.1 * cmpHealth.GetMaxHitpoints() + 0.9 * cmpHealth.GetHitpoints()) Where is my mistake? |
| 185 ↗ | (On Diff #12436) | I did now: the proposed code gives a 4-5x improvement |
Comment Actions
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
| 19| 19| this.cp = [];
| 20| 20| };
| 21| 21|
| 22| |-//// Interface functions ////
| | 22|+// // Interface functions ////
| 23| 23|
| 24| 24| /**
| 25| 25| * 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
| 159| 159| return sourceEnemyCp > 0;
| 160| 160| };
| 161| 161|
| 162| |-//// Private functions ////
| | 162|+// // Private functions ////
| 163| 163|
| 164| 164| /**
| 165| 165| * 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
| 301| 301| this.CheckTimer();
| 302| 302| };
| 303| 303|
| 304| |-//// Message Listeners ////
| | 304|+// // Message Listeners ////
| 305| 305|
| 306| 306| Capturable.prototype.OnValueModification = function(msg)
| 307| 307| {
Executing section cli...Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2742/display/redirect