Page MenuHomeWildfire Games
Differential D2826

Cleanup of Capturable
ClosedPublic
Actions

Authored by bb on Jun 20 2020, 11:12 PM.

Details

Reviewers
None
Commits
rP23875: Cleanup of Capturable component
Summary

Cleanup of Capturable

var => let
periods, caps
JSdocs where useful
don't get the hitpoints twice

Test Plan

Let the bots run

Diff Detail

Event Timeline

bb created this revision.Jun 20 2020, 11:12 PM
Stan added a subscriber: Stan.Jun 20 2020, 11:18 PM
Stan added inline comments.
binaries/data/mods/public/simulation/components/Capturable.js
198

One too much?

Freagarach added a subscriber: Freagarach.Jun 21 2020, 11:15 AM
Freagarach added inline comments.
binaries/data/mods/public/simulation/components/Capturable.js
46

with

55

capture points?

58

multiplied

59

s/,/.

179–180

Perhaps "best player" could be elaborated ;)

223–224

Comment on top?

240

Comment necessary?

243

Delete?

331

Comment on top?

349–351

(Seems like one sentence?)

bb updated this revision to Diff 12407.Jun 21 2020, 5:19 PM
bb marked 8 inline comments as done.
bb added inline comments.
binaries/data/mods/public/simulation/components/Capturable.js
59

Not really

240

Someone might be wondering why we remove a timer.

349–351

Not really, but can make it into one more clear sentence

bb edited the summary of this revision. (Show Details)Jun 21 2020, 5:46 PM
Freagarach added inline comments.Jun 21 2020, 7:16 PM
binaries/data/mods/public/simulation/components/Capturable.js
15–16

Comment seems outdated (also recalculated onValueModification). Is it needed?

56

capturing? Do you "attack" an entity of your ally when taking capture points?

59

Ah yeah,,,

134–135

let?

150

let?

207–208

let

Stan added inline comments.Jun 21 2020, 11:00 PM
binaries/data/mods/public/simulation/components/Capturable.js
179–180
Maybe it could be this.cp.filter(cp => cp >= this.cp[bestPlayer]).length
bb updated this revision to Diff 12430.Jun 22 2020, 11:01 PM
bb marked 5 inline comments as done and an inline comment as not done.

Note that I deliberately avoided the cmpOwnership.GetOwner calls, the need a slightly more subtle care

bb added inline comments.Jun 23 2020, 2:47 PM
binaries/data/mods/public/simulation/components/Capturable.js
56

In fact you do

179–180

How? we want to find the index of cp with greatest value, maybe you mean
this.cp.reduce((bestPlayer, playerCp, player, cp) => playerCp > cp[bestPlayer] ? player : bestPlayer, 0)

bb updated this revision to Diff 12436.Jun 23 2020, 6:42 PM

Let the bots run!!

Vulcan added a comment.Jun 23 2020, 6:46 PM

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

Harbormaster failed remote builds in B12210: Diff 12436!Jun 23 2020, 6:46 PM
wraitii added a subscriber: wraitii.Jun 28 2020, 3:15 PM
wraitii added inline comments.
binaries/data/mods/public/simulation/components/Capturable.js
46

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

I'm coming up with bonusMultiplier *= 0.1 + 0.9 * hitpoints / cmpHealth.GetMaxHitpoints() when I do the factoring?

185

Have you profiled this to be faster? I would call it less readable.

bb marked an inline comment as done.Jul 21 2020, 8:17 PM
bb added inline comments.
binaries/data/mods/public/simulation/components/Capturable.js
69

bonusMultiplier *= cmpHealth.GetMaxHitpoints() / (0.1 * cmpHealth.GetMaxHitpoints() + 0.9 * cmpHealth.GetHitpoints())
cancelling cmpHealth.GetMaxHitpoints() up and downstairs gives
bonusMultiplier *= 1 / (0.1+ 0.9 * cmpHealth.GetHitpoints()/ cmpHealth.GetMaxHitpoints())
Gives:
bonusMultiplier /= 0.1+ 0.9 * cmpHealth.GetHitpoints()/ cmpHealth.GetMaxHitpoints()

Where is my mistake?

185

I did now: the proposed code gives a 4-5x improvement

bb updated this revision to Diff 12838.Jul 21 2020, 8:18 PM

comment

Vulcan added a comment.Jul 21 2020, 8:23 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
|  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

Harbormaster completed remote builds in B12548: Diff 12838.Jul 21 2020, 9:04 PM
This revision was not accepted when it landed; it landed in state Needs Review.Jul 23 2020, 3:47 PM
Closed by commit rP23875: Cleanup of Capturable component (authored by bb). · Explain Why
This revision was automatically updated to reflect the committed changes.
bb added a commit: rP23875: Cleanup of Capturable component.

Revision Contents
Changeset List

PathSize
binaries/
data/
mods/
public/
simulation/
components/
109 lines

Diff 12838

View Options

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

Loading...
Wildfire Games · Phabricator