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

Overkill damage #48

Merged
merged 5 commits into from Sep 26, 2015

Conversation

Projects
None yet
3 participants
@forivall
Contributor

forivall commented Sep 24, 2015

fixes #47

Note the terminology change. A FAQ page has been added to explain the difference.

Some other minor changes were made, mainly in comments.

A full test, with a server running an up-to-date stats plugin, will be needed. ngrok works well for this; we should make a note of that. @dy-dx , did you use anything like it?

@forivall

This comment has been minimized.

Show comment
Hide comment
@forivall

forivall Sep 24, 2015

Contributor

currently publicly visible at http://0350a0a5.ngrok.io/stats/4

Contributor

forivall commented Sep 24, 2015

currently publicly visible at http://0350a0a5.ngrok.io/stats/4

@dy-dx

This comment has been minimized.

Show comment
Hide comment
@dy-dx

dy-dx Sep 24, 2015

Member

Thanks. I will test this out tonight.

Member

dy-dx commented Sep 24, 2015

Thanks. I will test this out tonight.

@SizzlingCalamari

This comment has been minimized.

Show comment
Hide comment
@SizzlingCalamari

SizzlingCalamari Sep 25, 2015

Member

The plan for the terminology now is
"RD" - "Real Damage"
"TD" - "Total Damage"
"RDPM" - "Real Damage Per Minute"
"TDPM" - "Total Damage Per Minute"

I'm fine with keeping the faq page to explain those stats just in case. (I get ss vs logs damage questions very frequently)

Member

SizzlingCalamari commented Sep 25, 2015

The plan for the terminology now is
"RD" - "Real Damage"
"TD" - "Total Damage"
"RDPM" - "Real Damage Per Minute"
"TDPM" - "Total Damage Per Minute"

I'm fine with keeping the faq page to explain those stats just in case. (I get ss vs logs damage questions very frequently)

@SizzlingCalamari

This comment has been minimized.

Show comment
Hide comment
@SizzlingCalamari
Member

SizzlingCalamari commented Sep 26, 2015

LGTM

SizzlingCalamari added a commit that referenced this pull request Sep 26, 2015

@SizzlingCalamari SizzlingCalamari merged commit 29590cf into SizzlingStats:master Sep 26, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@dy-dx

This comment has been minimized.

Show comment
Hide comment
@dy-dx
Member

dy-dx commented Sep 26, 2015

@forivall

This comment has been minimized.

Show comment
Hide comment
@forivall

forivall Sep 27, 2015

Contributor

Hmm, maybe the FAQ could be updated (in the short term) that server admins will need to update the sizzling stats plugin to see that they are different

or, i could submit a pr to check the version and only display the tdpm / td columns if the plugin version is high enough. @SizzlingCalamari is the old version v0.1 and the new version v0.2?

Contributor

forivall commented Sep 27, 2015

Hmm, maybe the FAQ could be updated (in the short term) that server admins will need to update the sizzling stats plugin to see that they are different

or, i could submit a pr to check the version and only display the tdpm / td columns if the plugin version is high enough. @SizzlingCalamari is the old version v0.1 and the new version v0.2?

@SizzlingCalamari

This comment has been minimized.

Show comment
Hide comment
@SizzlingCalamari

SizzlingCalamari Sep 27, 2015

Member

@forivall
We were just planning on doing a rollback until the plugin is ready, but adding the version number check sounds good to keep past stats looking the same. The current header version before this change is v0.2. I forgot to change the version when updating the json structure in the plugin, so v0.3 will be the version containing the overkill damage stat.

Member

SizzlingCalamari commented Sep 27, 2015

@forivall
We were just planning on doing a rollback until the plugin is ready, but adding the version number check sounds good to keep past stats looking the same. The current header version before this change is v0.2. I forgot to change the version when updating the json structure in the plugin, so v0.3 will be the version containing the overkill damage stat.

@forivall

This comment has been minimized.

Show comment
Hide comment
@forivall

forivall Sep 27, 2015

Contributor

👍

Contributor

forivall commented Sep 27, 2015

👍

@forivall forivall deleted the forivall:overkill-damage branch Oct 5, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment