dysath / seat-billing

A billing system for mining/PvE costs for corps/alliances
GNU General Public License v2.0
6 stars 6 forks source link

Inconsistent Tax Rate Formatting Leads to Inconsistent/Incorrect Tax Owed #10

Open Eingang opened 5 years ago

Eingang commented 5 years ago

Summary

Floating point mining tax rates are not displayed or applied consistently or correctly across Current Billing (Individual/Corp) and Past Billing (Individual and Corp).

Discussion

The problem's actually a little broader than summarized because it not only affects the display of Mining Tax and calculations based on it, but the display and calculation of other numeric values too. However, the Mining Tax one is the most egregious.

Let's start by looking at the Settings: IRS Settings

Note that I did not change any of the settings after the Past Billing Invoices for the previous month were calculated. In the IRS Settings screenshot above, the Incentivized Settings-> Ore TAX Rate of 2.5% is correctly displayed and stored. It's also correctly displayed and used to calculate the Tax Owed on the Current Billing Summary (Corporation Mining) page, and the Mined Amount and Adjusted Value calculations are also correctly displayed and calculated, as seen below.

Current Billing (Corp)

After that, things get messy. On the Current Billing Summary (Individual Mining), the Mining Amount is displayed with 3 decimal places instead of 2. The Mining Tax value and resultant calculations and displays seem to be correct (see screenshow below).

Current Billing (Individual)

On the Past Billing Invoices (Corporation Mining) page, the Mined Amount, Adjusted Value, and Tax Owed are all correctly displayed with two decimal places. However, the Tax Rate of .2.5% has been rounded up to 3% on the display and 3% is what's been used to calculate the Tax Owed (see screenshot below).

Past Billing Invoices (Corp)

And just to be sure we've covered all the bases, on the Past Billing Invoices (Individual Mining) page, the Mining Amount has has 0 decimal places instead of 2. The Tax Rate has been rounded down to 0 places and then used to calculate the Tax Due. So instead of 2.5% being used, 2% was used as illustrated below.

Past Billing Invoices (Individual)

Problem Summary

Here's a handy summary of the issues on a page-by-page, variable-by-variable basis:

Other Comments

It's possible the Market Modifier for mining-related calculations is also off, but I used an integer for that and not a floating point value, so I can't tell. Likewise, I can't tell if the Tax Rate for PvE tax calculations is displayed incorrectly and then being used in the Tax Owed (PvE) calculation to provide an incorrect amount. The Total Bounties and Tax Owedfields on the Current and Past Billing pages for Corporation PvE at least have the correct number of decimal places displayed.

I'd suggest internally using floating point for all the calculations and using number-format() or printf() (or something similar) with relevant rounding functions then on numbers to be displayed. On the plus side, at least this stuff should be relatively easy to fix.

And finally, yes, I realize my Incentivized Settings -> Bounty Tax Rate is a whacko value. Fixed. (-:

dysath commented 5 years ago

That may be the best bug report evar!! I'll take a gander at fixing that stuff. Honestly, I don't think I ever took floats into consideration when I wrote it.

On Fri, Feb 1, 2019 at 2:04 PM Eingang notifications@github.com wrote:

Summary

Floating point mining tax rates are not displayed or applied consistently or correctly across Current Billing (Individual/Corp) and Past Billing (Individual and Corp). Discussion

The problem's actually a little broader than summarized because it not only affects the display of Mining Tax and calculations based on it, but the display and calculation of other numeric values too. However, the Mining Tax one is the most egregious.

Let's start by looking at the Settings: [image: IRS Settings] https://camo.githubusercontent.com/c2bd3235d87de793dcda67b5ca2945145139abf0/68747470733a2f2f692e696d6775722e636f6d2f7a625a697a496b2e706e67

Note that I did not change any of the settings after the Past Billing Invoices for the previous month were calculated. In the IRS Settings screenshot above, the Incentivized Settings-> Ore TAX Rate of 2.5% is correctly displayed and stored. It's also correctly displayed and used to calculate the Tax Owed on the Current Billing Summary (Corporation Mining) page, and the Mined Amount and Adjusted Value calculations are also correctly displayed and calculated, as seen below.

[image: Current Billing (Corp)] https://camo.githubusercontent.com/1939380367f08765ac9ae566565329828079bb1c/68747470733a2f2f692e696d6775722e636f6d2f7a326e466856322e706e67

After that, things get messy. On the Current Billing Summary (Individual Mining), the Mining Amount is displayed with 3 decimal places instead of

  1. The Mining Tax value and resultant calculations and displays seem to be correct (see screenshow below).

[image: Current Billing (Individual)] https://camo.githubusercontent.com/fb417c18cb2209cc1f9ce203fc735b8070e8b46d/68747470733a2f2f692e696d6775722e636f6d2f796b73543352712e706e67

On the Past Billing Invoices (Corporation Mining) page, the Mined Amount, Adjusted Value, and Tax Owed are all correctly displayed with two decimal places. However, the Tax Rate of .2.5% has been rounded up to 3% on the display and 3% is what's been used to calculate the Tax Owed (see screenshot below).

[image: Past Billing Invoices (Corp)] https://camo.githubusercontent.com/e1c6d80c58a386cdb4500ca6b83720478491194b/68747470733a2f2f692e696d6775722e636f6d2f566f4b383534442e706e67

And just to be sure we've covered all the bases, on the Past Billing Invoices (Individual Mining) page, the Mining Amount has has 0 decimal places instead of 2. The Tax Rate has been rounded down to 0 places and then used to calculate the Tax Due. So instead of 2.5% being used, 2% was used as illustrated below.

[image: Past Billing Invoices (Individual)] https://camo.githubusercontent.com/cd464f5abe83e03395dafcba0a7daf76f3662e24/68747470733a2f2f692e696d6775722e636f6d2f734b4e487a6c6c2e706e67 Problem Summary

Here's a handy summary of the issues on a page-by-page, variable-by-variable basis:

  • Current Billing Summary (Individual Mining) -> Mining Amount: Displayed with 3 decimal places instead of 2.
  • Past Billing Invoices (Corporation Mining) -> Tax Rate: Rounded up to 0 decimal places.
  • Past Billing Invoices (Corporation Mining) -> Tax Due: Incorrectly calculated using rounded up (to 0 decimal places) Tax Rate.
  • Past Billing Invoices (Individual Mining) -> Tax Rate: Incorrect displayed with 0 decimal places.
  • Past Billing Invoices (Individual Mining) -> Tax Due: Incorrectly calculated using rounded down (to 0 decimal places) Tax Rate.
  • Past Billing Invoices (Individual Mining) -> Mining Amount: Rounded to 0 decimal places instead of 2.

Other Comments

It's possible the Market Modifier for mining-related calculations is also off, but I used an integer for that and not a floating point value, so I can't tell. Likewise, I can't tell if the Tax Rate for PvE tax calculations is displayed incorrectly and then being used in the Tax Owed (PvE) calculation to provide an incorrect amount. The Total Bounties and Tax Owedfields on the Current and Past Billing pages for Corporation PvE at least have the correct number of decimal places displayed.

I'd suggest internally using floating point for all the calculations and using number-format() or printf() (or something similar) with relevant rounding functions then on numbers to be displayed. On the plus side, at least this stuff should be relatively easy to fix.

And finally, yes, I realize my Incentivized Settings -> Bounty Tax Rate is a whacko value. Fixed. (-:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/dysath/seat-billing/issues/10, or mute the thread https://github.com/notifications/unsubscribe-auth/AQF4jPcjW0gqD3FbsLEpz5w7vRNn_Lgzks5vJJ20gaJpZM4ae_kb .

-- Ed Stafford