Widget talk:Map bonus rewards

From Guild Wars 2 Wiki
Jump to navigationJump to search

TP Tax rounding errors[edit]

First, thanks for the Map bonus rewards/profit page. I use it often.

I've noticed two issues I'd like to point out, and propose solutions for. (I might do it myself, but I actually don't any wiki template experience, nor the template editing priviledge).

  1. When Shiny Baubles show up in a material list, 44 are shown to have a sell value total of 13g 19s 85c. The correct value of course is 13g 20s 0c.
  2. The sum of the individual materials in each zone does not add up to the zonetotal.

I dug into the source code for the template to discover why these errors happened. Both are due to rounding errors when computing the trading post list+sales tax (as I'm sure you know).

Problem 1[edit]

For problem number 1, there's a pretty simple, but non-optimal solution. The current code uses a simple kludge for the sell price of a 'Shiny Bauble', which is the only non-tradeable/non-taxed item on this page. It pretends the value for a single unit is something that would end up as 30s after taxing it. Unfortunately the number used isn't precise enough, and gets rounded down. So a similarly simple kludge is to replace the value 3529 with 3529.4117... in the line below (since javascript works fine with floats).

tpdataById['70093'] = { buys: { quantity: 0, unit_price: 3000 }, sells: { quantity: 0, unit_price: 3529 } }; // 3000 divided by 0.85

But, I'd prefer you don't do that, and instead implement the changes described below which takes care of problem number 1 a better way.

Problem 2[edit]

Problem number 2 is, however, not so simple to fix. In one sense, it can't be fixed. But I do think the problem can be minimized greatly. The core of the problem is that the TP tax does not scale linearly. Simplisticly its 5% to list, and 10% on a sale, for an overall tax of 15%. But practically the tax function on a sale is

Tax(sp) = Math.round(0.05 * sp) + Math.round(0.1 * sp)    // where sp = Total sale price

The rounding part makes the tax function non-linear, and prevents it from following the distributive property. What this means for the tables on this page is that the precise value of the zone total depends on how many items you sell at a time. (Selling all 44 Elaborate Totems at once yields a different amount than selling them one at a time.)

Solution Code[edit]

So I'll lay out my whole solution here. The goal is to present the correct total for a zone when all the items of each material are sold at once.

// replace the current line in the arrangeTPdata() function with this which uses the actual price of Shiny Bauble.
tpdataById['70093'] = { buys: { quantity: 0, unit_price: 3000 }, sells: { quantity: 0, unit_price: 3000 } };

// add this after the arrangeTPdata() function.
// (note: return value object could be trimmed of everything but taxed, since that's all 
// that ends up being used)
function sellPrice(id, quant) {
  // perunit - raw sell price per unit before tax, if any, is charged
  // untaxed - value of entire lot before tax, if any, is charged
  // taxed - value of entire lot after tax, if any, is charged
  // isuntaxed - is this an untaxed commodity
  var rv = {};
  rv.perunit = tpdata[id]['sells']['unit_price'];
  rv.untaxed = rv.perunit * quant;
  rv.taxed = rv.untaxed;
  rv.istaxed = false;
  if (id != '70093') {  // so far, only 'Shiny Baubles' are not tradeable/taxed
    rv.istaxed = true;
    // calculate the listing and sales tax just like the TP does
    rv.taxed = rv.untaxed - Math.round(0.05*rv.untaxed) - Math.round(0.1*rv.untaxed);
  }
  return rv;
}

// this should replace the code from "//For each reward" to and including "zonetotal = Math.ceil..."
// It computes the zone total as the sum of each of the material summary lines... so the total HAS to be right.

  // Add up the sales price (after tax) for each material rewarded in the zone.
  var zonetotal = 0;
  $.each(materialSummary(matdata, weekkey, zonekey), function(key,val){
     zonetotal += sellPrice(val[2], val[0]).taxed;
  });

// And finally, fix the two locations in the code that calculate the value after tax 
// so that they use the new function. Replace each of the 2 instances of:
  getCoin(Math.ceil(0.85 * val[0] * tpdata[val[2]]['sells']['unit_price'])).html
// with:
  getCoin(sellPrice(val[2], val[0]).taxed).html

I've personally extracted the page/template to a standalone html file and tested it, and it seems to work as I intended. IE. when displaying the material lists for a zone as material totals, the individual lines add up to the zone total. Of course when displaying the list as individual material drops at each 200 rewardpoint break, the single items won't add up to the zone total (because of the non-linear rounding errors as described earlier). But at least one version of the display looks correct, and I think that's the version that most people would be interested in.

--SomeGuy (talk) 00:32, 13 January 2021 (UTC)

Problem 1 seemed easy to implement so it's been done and works perfectly.
Problem 2 seems pretty minor and most people probably wouldn't notice the impact. I think I saw Nightsky experimenting with (what is no doubt) a non-jquery solution the other day so I'm not going to dive too deep into this js I haven't touched since 2016... but that being said I'll stick your solution up. If it breaks we can revert it later!
Thanks for commenting up your code so my completely-worn-out-and-baked-post-work mind can understand it. -Chieftain AlexUser Chieftain Alex sig.png 20:31, 1 February 2021 (UTC)
Only had a tiny bug where getCoin was being passed a non-integer value, for which I've applied a ceiling function, otherwise looks good. -Chieftain AlexUser Chieftain Alex sig.png 20:37, 1 February 2021 (UTC)
I'm actually amazed how the volatile price of the materials causes the options with Shiny Baubles to become rapidly more or less attractive depending on when this page fetches the TP prices.
Does this now mean we're assuming that we sell in batches as the materials accumulate, rather than waiting for the end of all the materials and selling in bulk? -Chieftain AlexUser Chieftain Alex sig.png 21:07, 1 February 2021 (UTC)
The intent there was to remove literal html from within the script tags; since the mobile parser still has trouble with literal closing div tags. And while that ended up with the code almost without jquery that is mostly because i didn't think of it also having methods to do the same that are safer to use than parsing the literal html too.
I cant quite make out what you're refering to with the this in "Does this now mean we're assuming that..." but if it's about the default's for the version over at Widget:Test; i've set the last two choices to default to what i assume would be the least profitable option, or know to be the least profitable in the case of buys/sells, with the idividual transactions assumed to yield less because of the more rounding from the greater amount of transactions potentially rounding up more often, though i've also noticed it sometimes being less profitable if sold in bulk by now so i guess that's not always the case but i needed to determine a default in some way. (The default's are the left most buttons in each category. (Which also has the buy/sell buttons be in the same order as the sections in the tp.))
While the current version works i figured it's likely to slow to be propperly used and since i didn't hear anything contrary didn't swap it over nor mentioned anything here as i've meant to rewrite it faster by having it precalculate everything and then only swaping out results appropriately too; but that has taken me so long already and i kindof can't see it anymore by now, so that's unlikely to happen anymore. Nightsky (talk) 18:05, 3 February 2021 (UTC)
My comment with respect to "does this now mean we're assuming that..." was referring to the amendments to the current Widget:Map bonus rewards post the changes I made based on SomeGuy's recommendation. -Chieftain AlexUser Chieftain Alex sig.png 20:15, 3 February 2021 (UTC)
Oh i see. In that case, the use of the materialSummary method in the $.each takes care of that; making it so that it correctly calculates the values for the sum of all materials at the end of all 40 tiers, then summing those sums up for the total. Whilst the operation for the summary totals already uses that very same method rendering both totals equal. (For what it's worth, if you were to load Map bonus reward/profit and Widget:Test at the same time, then select sells and in bulk at the bottom for the later; the values will match.) In short no, it's essentially assuming the items are sold in the quantities shown by the summary. Nightsky (talk) 22:12, 3 February 2021 (UTC)