Page MenuHomeWildfire Games

Rewrite Profiler2's GUI using highcharts
Needs ReviewPublic

Authored by wraitii on Sun, Jan 27, 1:55 PM.

Details

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

Don't remember why I started this yesterday, but I've rewritten Profiler2's UI to use Highcharts instead of custom-drawing everything using low-level canvas methods.

Cons:

  • quite possibly a tad slower
  • dependency on highcharts and running a basic http server locally (I've used http-server, available on NPM, but any will do).
  • Basically guaranteed to only work in Chrome but I think the former version did that already.

Pros:

  • much less code which makes this actually upgradable. I've also cleaned up things.
  • more interactivity: clicking on stuff does things and zooming is more convenient.

I need to add back support for "Live" profiling, but tbh that's not the most useful feature and I might just kill it.

Test Plan

Open a profile. Play around with stuff.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
aaa_profiler2_rewrite
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 6849
Build 11255: Vulcan BuildJenkins
Build 11254: arc lint + arc unit

Event Timeline

wraitii created this revision.Sun, Jan 27, 1:55 PM
Vulcan added a subscriber: Vulcan.Sun, Jan 27, 1:59 PM

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

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

Stan added a subscriber: Stan.Sun, Jan 27, 2:22 PM
Stan added inline comments.
source/tools/profiler2/ReportColor.js
23

Should be g_something, with maybe a comment :)

28

I believe the convention is

!(color_by_name.name)

source/tools/profiler2/index.html
10

Can you make a css file, this is bad :D

Stan added a subscriber: elexis.Sun, Jan 27, 2:25 PM
Stan added inline comments.
source/tools/profiler2/Profiler2Report.js
35

@elexis Do we use this anywhere else ? It looks nicer than prototypes. If not maybe we should.

Stan added inline comments.Sun, Jan 27, 2:32 PM
source/tools/profiler2/Profiler2Report.js
59

Enum ?

nani added a subscriber: nani.Sun, Jan 27, 2:33 PM
nani added inline comments.
source/tools/profiler2/Profiler2Report.js
35

The current spider-monkey in 0ad doesn't have support for classes afaik

Stan added inline comments.Sun, Jan 27, 2:35 PM
source/tools/profiler2/Profiler2Report.js
24

Missing 'use strict'; (It's by default in SpiderMonkey code but not here I guess"

35

That's what I thought.

smiley added a subscriber: smiley.Sun, Jan 27, 2:39 PM
smiley added inline comments.
source/tools/profiler2/ReportColor.js
26

export {get_color, default} at the bottom would be nicer no?

28

Should be using [].

smiley added inline comments.Sun, Jan 27, 2:41 PM
source/tools/profiler2/Profiler2Report.js
35

This came with es6 which means the current 0AD SM does have it. But it's just syntactic sugar over the prototype system.

smiley removed a subscriber: smiley.Sun, Jan 27, 2:43 PM