Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PlayerStatistics should stop ticking after lost/win #16858

Merged

Conversation

@teinarss
Copy link
Contributor

commented Jul 28, 2019

No description provided.

@pchote

This comment has been minimized.

Copy link
Member

commented Aug 6, 2019

The two Queues don't store time info, so the times will get out of sync after 100 minutes when some players start dequeuing elements and others don't.

The simplest fix here is to remove the dequeuing - doing this after 100 samples is really arbitrary, and will never be reached in the majority of games. Those that do pass 100 probably won't last much longer, so we may as well preserve the entire history.

@teinarss teinarss force-pushed the teinarss:playerstats_should_not_keep_ticking branch from de0e1cd to 5c9a7b4 Aug 26, 2019
@teinarss

This comment has been minimized.

Copy link
Contributor Author

commented Aug 26, 2019

Updated

Copy link
Member

left a comment

This works how it is supposed to be (stopping the graphs, which I like), but does not fix #3451. Since the $/min stat (not graph) still decreases for dead players.

@teinarss

This comment has been minimized.

Copy link
Contributor Author

commented Aug 30, 2019

This works how it is supposed to be (stopping the graphs, which I like), but does not fix #3451. Since the $/min stat (not graph) still decreases for dead players.

Hah, i should learn to read!

@abcdefg30

This comment has been minimized.

Copy link
Member

commented Sep 6, 2019

Needs a rebase.

@teinarss teinarss force-pushed the teinarss:playerstats_should_not_keep_ticking branch from 5c9a7b4 to e234180 Sep 7, 2019
@teinarss

This comment has been minimized.

Copy link
Contributor Author

commented Sep 7, 2019

Rebased

@RoosterDragon

This comment has been minimized.

Copy link
Member

commented Sep 13, 2019

I think it would be nice if it fixed the $/min stat and maybe also zeroed the "Earned this min" for the minutes after those in which a player has lost.

If not, edit the desc as this doesn't close the linked issue.

@teinarss

This comment has been minimized.

Copy link
Contributor Author

commented Sep 13, 2019

Done, updated the desc. Been some talk about changing the $/min stat so no point in chaining it here.

@RoosterDragon

This comment has been minimized.

Copy link
Member

commented Sep 13, 2019

Let's take this as-is then.

@RoosterDragon RoosterDragon merged commit c15f66a into OpenRA:bleed Sep 13, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@RoosterDragon

This comment has been minimized.

Copy link
Member

commented Sep 13, 2019

@pchote pchote added this to the Next Release milestone Oct 6, 2019
@pchote

This comment has been minimized.

Copy link
Member

commented Oct 6, 2019

Picked to prep to avoid rebase conflicts with @abcdefg30's more recent spectator PRs.

088288f and updated both changelogs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.