LotV replay length fix

[nickelsen]

I read through dsjoerg/ggpyjobs#7 and based on that I'd like to suggest the following changeset to sc2reader to address the game length issue with lotv replays.

The only way I can get it to work in my mind is to interpret game_fps as frames-per-normal-game-second. Incidentally, in HotS, that becomes frames-per-real-second, but since game speed multipliers are different in LotV, the original fps interpretation has to be changed - hence the fps change here.

I tested this by uploading HotS and LotV replays to my local ggtrackerstack and game durations show identical to what is shown in-game. I have only tested with Faster speed replays - let me know if I should test with other speeds as well.

Unittests in ggpyjobs and esdb pass locally (everything depending on vagrant branches being merged). 2 tests in sc2reader fail (besides the 2 expected), but they also fail on upstream, so I don't think they should be addressed here.

[dmpaul26]

This looks very similar to the thought process I had, but with better execution. Logically, makes sense to me. Good work!

[dsjoerg]

Wow very nice @nickelsen. Faster speeds only is fine by me.
Before I push this into production I should play with it a bit on my local box and see what else gets changed by it.

[dsjoerg]

So this is probably and good and if it doesn't break production I will release it. In order to test whether it breaks production I'm going to

1. get ggtrackerstack fully working on my dev box
2. upload a bunch of replays and confirm that everything is OK

This is certainly not the end of the LotV related time problems — for example I believe the army chart still shows old-style times (right below the VS in the middle) and perhaps the other charts do as well? And the economic benchmarks, and the income-per-minute numbers.

My point is not that everything must be fixed before I will release anything, but rather that work on the remaining time problems can proceed and are unrelated to the production-release schedule. Not sure when I'll be able to do the ggtrackerstack testing, hopefully this weekend.

Great work!

[nickelsen]

Hmm - this might not work completely as expected - it appears the stat charts are rendered from differently time-scaled series than the army chart or something. When I see the same LotV replay in my local setup with this change applied and compare to ggtracker.com, the army series appears to be preserved, but the stat charts seem cut off at the new replay length (11 minutes instead of 16 minutes for this particular replay).

My local setup (not rendering A.I. army for some unknown reason):

Same replay in ggtracker.com:

[dsjoerg]

@nickelsen once i get my ggtrackerstack up and running i'll replicate what you're seeing and diagnose what's going on.

The charts in the bottom half of the page get their data from different places, so if they are all messed up in the same way that is a hint to me that the problem is in the code that is drawing the charts, rather than something upstream from there.

[dsjoerg]

@nickelsen your changes are fine. they are necessary, but insufficient to fix the LotV time-scaling problem. I have made a commit in ggtracker that makes further progress on the time-scaling issues: f86d0d6a2a7c768ba1bab7b66f1b50b3a94a829b

my latest commit fixes some of the charts below the army chart, in particular the ones that get their information from tracker events. these tracker events used to occur every 10 game seconds during the replay; now they occur every (10 / 1.4) game seconds.

i believe some of the charts in the bottom-of-the-page section are still screwed up, including but perhaps not only the APM chart.

[dsjoerg]

I'm going to accept this PR, even though the time changes are a work in progress, I think it'll be easier for everyone to work together if the master of each codebase has our latest work.

[gravelweb]

Just pointing out that this change also caused a mismatch with the mini-map replay feature, so something else to consider for future work :)

The issue is that once we get to a certain point in the army graph, there is no longer any change being reflected in the mini-map, and the web console is whining:

TypeError: Cannot read property '34' of undefined
    at http://ggdev.com:3000/assets/gg.js:29197:39
    at Array.forEach (native)
    at Function.T.each.T.forEach (http://ggdev.com:3000/assets/gg.js:26066:625)
    at http://ggdev.com:3000/assets/gg.js:29186:17
    at Array.forEach (native)
    at Function.T.each.T.forEach (http://ggdev.com:3000/assets/gg.js:26066:625)
    at Object.updateCameras (http://ggdev.com:3000/assets/gg.js:29145:11)
    at Object.fn (http://ggdev.com:3000/assets/gg.js:26066:7285)
    at Object.Scope.$digest (http://ggdev.com:3000/assets/gg.js:15898:27)
    at Object.Scope.$apply (http://ggdev.com:3000/assets/gg.js:16092:24)

[dsjoerg]

Good point @gravelweb, created ggtracker/ggtrackerstack#35 to track that.

[dsjoerg]

Deployed in production, good work @gravelweb and @nickelsen