Skip to content

Conversation

@nickelsen
Copy link
Contributor

See ggtracker/ggtrackerstack#44 for comments.

@dsjoerg
Copy link
Owner

dsjoerg commented Oct 6, 2016

Something else needs to be adjusted, because for example look at the MINERAL INCOME chart in http://ggtracker.com/matches/6823576, to pick a random example.

The players hit 1-base saturation at 2:30 and 1:30. However looking at the MINERAL INCOME chart, it appears to be using 640, not 870, and so this commit will make the system appear broken.

@dsjoerg
Copy link
Owner

dsjoerg commented Oct 6, 2016

I'm being really lazy here... my guess is that the MINERAL INCOME chart is showing HotS-scaled income, and that as part of this change we would need to change the chart to appear as LotV-scaled.

You could confirm my guess by comparing the income you saw in-game to the income as shown in the MINERAL INCOME chart on ggtracker.

And if my guess is right, then my suggestion for how to fix would be to multiply the income numbers just before showing them in the chart.

@nickelsen
Copy link
Contributor Author

Yes, I completely agree, and I am working on that right now using the solution you suggest. You're right, these changes should go together, so I'll add it in a commit to this PR when done.

@nickelsen
Copy link
Contributor Author

@dsjoerg I've updated the PR with the commit I mentioned.

@nickelsen
Copy link
Contributor Author

I just remembered that spending quotient is calculated based on income rate, so just changing the income rate and graphs we show on the match page doesn't fix the underestimated spending quotient. I'll work on that next, but I think this PR can be merged now, as it doesn't make the system more borked and fixes a few things.

@dsjoerg
Copy link
Owner

dsjoerg commented Oct 7, 2016

I think it's better to leave spending quotient alone. The way it is, the
SQ's are in a range that people are familiar with from pre-LotV days.

On Friday, October 7, 2016, Anders Nickelsen [email protected]
wrote:

I just remembered that spending quotient is calculated based on income
rate, so just changing the income rate and graphs we show on the match page
doesn't fix the underestimated spending quotient. I'll work on that next,
but I think this PR can be merged now, as it doesn't make the system more
borked and fixes a few things.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#26 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAvzqRf1HZokp5P6rHaLSZHRHdntKex2ks5qxd4SgaJpZM4KQGMm
.

Sent from NSAMail Mobile

@dsjoerg
Copy link
Owner

dsjoerg commented Oct 8, 2016

@nickelsen how much testing have you done of these changes? I am inclined to put them into production but I'm trying to be extra lazy, have a lot going on, doing tech volunteering for the anti-Trump campaign.

If you've put a through replays through in your dev environment and everything looks OK to you, I'll push this into production.

@nickelsen
Copy link
Contributor Author

No worries. 😃

I put all the benchmark-replays I recorded through my dev version - one from each map - as well as a couple of other replays from ladder matches. I don't have real saturation data in the replays_econ_stat table, so I cannot check if saturation skills are accurate (I assume that in all my replays that stay on one base, 1-base saturation skill should be master/grandmaster according to the economic benchmarks). I can verify that locally with my benchmark replays, if you can send me the export of replays_econ_stat, but other than that I'm confident the changes are good.

Also, they are only visualization changes, so nothing that breaks already parsed replays. 😄

@dsjoerg
Copy link
Owner

dsjoerg commented Oct 11, 2016

Good point about being only visualization changes :)

@dsjoerg dsjoerg merged commit 2174566 into dsjoerg:master Oct 11, 2016
@dsjoerg
Copy link
Owner

dsjoerg commented Oct 11, 2016

I deployed this change but I think something is still borked.
For example, just picking a random replay:
http://ggtracker.com/matches/6829931

image

image

As you can see, there's no way any benchmark was hit at 3:10.

I wonder, is the time scaling off?

@dsjoerg
Copy link
Owner

dsjoerg commented Oct 11, 2016

paging @nickelsen in case my comment doesnt get your attention

@nickelsen
Copy link
Contributor Author

Ugh, ok, you're right, that doesn't look correct. I'll have a look at it and provide a fix.

@nickelsen
Copy link
Contributor Author

I verified my recorded replays + the one you mention in game and the income curves in production are accurate comparing to in-game. Somehow the mineral_saturation_1 timestamp is on HotS scale - I saw this in my dev environment and wrongly attributed this to dev/prod differences. In my dev environment my mineral_saturation_1 time was 1:30, but the income curve said I passed 870 at 1:04, which made sense (64 seconds = 90/1.4 seconds). The same goes for the replay you uploaded - both players pass the 870 at (3:10 / 1.4) seconds. I'll look more into this.

@nickelsen
Copy link
Contributor Author

Ok - I have a fix in my dev env that adjusts the displayed timings so they fit in-game and the other charts on the show match page.

However, while testing, I came across this in production - I don't get what is happening to the base timings of 'Doctor':

screen shot 2016-10-13 at 20 26 57

screen shot 2016-10-13 at 20 19 00

As far as I can see on other replays, base timings for entity=1 (the second player) are really wrong:
screen shot 2016-10-13 at 20 22 27
screen shot 2016-10-13 at 20 22 33

With my fix, the the 2nd and 3rd mining base completed times are equal to the bases chart and in-game timings for entity=0 but not entity=1.

I'll keep investigating and any pointers are greatly appreciated as to how this can happen to only one of the entities.

@dsjoerg
Copy link
Owner

dsjoerg commented Oct 13, 2016

Base-complete timing has several wrinkles that could affect one entity and not the other:

1 A "mining base" is a base that is complete and in mining position. Determining the mining position is map-specific. The code for determining mining position may be incorrect for the current map pool, but maybe only for certain spots used by one of the players.
2 There may be a bug or data out of date related to the specific names and identities of Terran building types. I noticed that in both the examples you provided it was Terran having the issue. It may be that the building was upgraded and that may have confused the code.

Anyway, this is a separate enough issue that we can pursue it in another issue.

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