Skip to content

Conversation

nickelsen
Copy link
Contributor

Fix for combat part of ggtracker/ggtrackerstack#28.

@nickelsen
Copy link
Contributor Author

Bases and race-specific macro-chart were showing only 1/1.4 of the content - fixed also in this commit (just searched for 960). Cannot find the APM stuff easily though.

speed_multiplier = 1;
if (scope.$parent.match.expansion_tag == 'LotV') {
speed_multiplier = 1.4;
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not use Sc2.LOTV_SPEEDUP here?

@nickelsen
Copy link
Contributor Author

Good point. Didn't know if Sc2.LOTV_SPEEDUP was in scope, but I'll use it instead if possible.

@nickelsen
Copy link
Contributor Author

Fixed!

@nickelsen
Copy link
Contributor Author

Also now with fix for ggtracker/ggtrackerstack#35

@dsjoerg
Copy link
Owner

dsjoerg commented Apr 20, 2016

Here's how the APM chart works.

In ggtracker/app/views/matches/show.html.erb, there's:

      <chart id="apm" class="grid_1" data-series="match.series.apm.combined" data-condensed="condensed">
        <span class="title">actions per minute</span>
      </chart>

So the APM chart gets its data from match.series.apm.combined, which means it's the Match resource in javascript.

In ggtracker/app/assets/javascripts/angular/resources/match.js search for the word apm, it's only in a few places.

In /ggtracker/app/assets/javascripts/angular/directives/chart.js is the chart directive that does the plotting. I reminded myself how it works by uncommenting some console.log() calls and inserting a few of my own.

If I understand the code correctly: in the match page, you'll see a bunch of JSON including "entities". For each entity there is a data.apm array which contains one entry per old-style minute. The chart directive currently relies on the fact that the APM data is at minute intervals, and sets the x-axis of the chart accordingly.

To fix this, we would either refactor the code in the chart directive or in the match resource to make the chart show things correctly, or we would fix the code that creates the data.apm array. The code that creates the data.apm array involves esdb/esdb/games/sc2/match/entity.rb:chart_data(), which reads from the minute table, which is populated by /ggpyjobs/sc2parse/sc2reader_to_esdb.py:populateEntityFromReplay(), in particular minuteDB.apm = player.apm.get(minute, 0). player.apm is populated by sc2reader/engine/plugins/apm.py:handlePlayerActionEvent()

My guess is that the APMTracker has not been adjusted for LotV, and, for example, is effectively capturing 22 observations for what is now a 15-minute game. If my guess is right, then perhaps the easiest fix is to fix that.

@dsjoerg dsjoerg merged commit 29995cb into dsjoerg:master Apr 20, 2016
@nickelsen
Copy link
Contributor Author

Thanks a lot for the pointers - I managed to build something that looks like it fixed it. 😃
ggtracker/sc2reader#12
dsjoerg/ggpyjobs#10

It turns out two APMTrackers are available and it looks like sc2reader uses the one specified in factories/plugins/replay.py instead of the one specified in engine/plugins/apm.py. I couldn't figure out why two a specified. Tests pass after my changes so I don't know if I broke something somewhere else by not also changing apm.py.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants