Page MenuHomeWildfire Games

Cleanup Timer.js and update documentation
Needs ReviewPublic

Authored by Stan on Feb 8 2019, 11:31 AM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Summary

For the record once we move to SM45 might be interesting https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WeakMap

Add and fix documentation. The current one was incorrect.

Test Plan

Ought to be unproblematic. Check for completeness

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 7190
Build 11718: Vulcan BuildJenkins

Event Timeline

Stan created this revision.Feb 8 2019, 11:31 AM
Vulcan added a subscriber: Vulcan.Feb 8 2019, 11:33 AM

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

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

binaries/data/mods/public/simulation/components/Timer.js
|  59| »   if·(typeof·repeattime·!=·"number"·||·!(repeattime·>=·0))
|    | [NORMAL] JSHintBear:
|    | Confusing use of '!'.
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/differential/1053/

Stan updated this revision to Diff 7471.EditedFeb 8 2019, 11:55 AM
  • Fix Vulkan warning
  • @returns -> @return
  • A little example in Vscode

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

Link to build: https://jenkins.wildfiregames.com/job/differential/1054/

Imarok added a subscriber: Imarok.Feb 8 2019, 2:32 PM
Imarok added inline comments.
binaries/data/mods/public/simulation/components/Timer.js
59

Sure, you're not meaning repeattime <= 0?

Stan marked 2 inline comments as done.Feb 8 2019, 2:37 PM
Stan added inline comments.
binaries/data/mods/public/simulation/components/Timer.js
59

Nope because else the simplification above would error out :D

Imarok added inline comments.Feb 8 2019, 2:44 PM
binaries/data/mods/public/simulation/components/Timer.js
59

So the change in behaviour is intended.
Then it's ok.

Stan marked 3 inline comments as done.Feb 8 2019, 2:46 PM
Stan added inline comments.
binaries/data/mods/public/simulation/components/Timer.js
59

Yep was wondering if I should have added a comment :) I should have :)

elexis added a subscriber: elexis.Feb 8 2019, 7:29 PM
elexis added inline comments.
binaries/data/mods/public/simulation/components/Timer.js
59

When is a timer with a repeat-time of 0 useful?
Sounds like it would either do an infinite loop or update each time the simulation is updated, which in the worst case of client-dependent progressive updates would trigger an OOS? (I didn't actually check whether they are client dependent, but I suspect they might depend on the renderer framerate, or the rejoining client not running progressive updates, dunno)

Stan marked an inline comment as done.Feb 8 2019, 7:57 PM
Stan marked an inline comment as done.Mar 6 2019, 2:47 PM
Stan added inline comments.
binaries/data/mods/public/simulation/components/Timer.js
59

A timer with a 0 repeat time is a timer that does not repeat itself. So SetInterval with repeattime = 0 = SetTimeout.

Stan marked an inline comment as done.Mar 6 2019, 2:47 PM
Stan updated this revision to Diff 7521.Mar 6 2019, 5:29 PM

Fix comment.

Vulcan added a comment.Mar 6 2019, 5:31 PM

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

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

Stan updated this revision to Diff 7635.Sun, Mar 31, 11:27 PM

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

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

bb added a subscriber: bb.Sun, Apr 14, 10:00 PM
bb added inline comments.
binaries/data/mods/public/simulation/components/Timer.js
34

I though we used hyphens and caps, however code convention page made in doubt...

35

period

52

period

55

non-zero says my math teacher and a comma If non-zero, the ...

Stan updated this revision to Diff 7752.Mon, Apr 15, 7:56 AM
Stan marked 4 inline comments as done.

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

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