Bug - Fixed historical_price() and updateprices.php

Ezandora

Member
Tested in r17701.

Dynamic price data is available at http://kolmafia.us/scripts/updateprices.php?action=getmap. At login, if sharePriceData is true, mafia will download a copy of this and use its pricing data.

However, values in the map do not always update historical_price():
Code:
322 prices updated from http://kolmafia.us/scripts/updateprices.php?action=getmap

> get sharePriceData

true

> ash $item[9119].historical_price()

Returned: 25000
Value in updateprices.php, at the time of login, just before running that command: 23000

Value in mafia's data/mallprices.txt: 25000

In theory, it should have returned 23000, as updateprices.php has a more recent timestamp for that entry. I believe this is due to the following line of code in MallPriceDatabase.java:
Code:
if ( id < 1 || id > ItemDatabase.maxItemId() ||
	price < 1 || price > 999999999 || timestamp <= 0 )
{	// Something's fishy with this file...
	return count;
}
This stops processing the data if it encounters an invalid line, particularly if the item ID is out of range.

If we check http://kolmafia.us/scripts/updateprices.php?action=getmap we find this line, among others:
Code:
5985988	1405180833	800
Which means item ID 5985988. This is out of bounds, and prevents the rest of the prices from being properly read. So everything up to that point is updated in mafia's mall price tracking, but the rest is discarded.

Maybe it should just continue instead of stopping?

Also, sharePriceData isn't set to true by default. Should mafia be changed so it always loads updateprices.php at login for accurate pricing information, regardless of that setting? That's just my personal feeling, kind of like how I'd love for svnUpdateOnLogin to be true by default.
 

Veracity

Developer
Staff member
Something IS fishy with the file if it has invalid itemIds in it. I wonder where the bogus itemIds are coming from?

It would be easy enough to simply skip the bogus entries.
 

lostcalpolydude

Developer
Staff member
17703 continues updating prices in that case.

It looks like the code was meant to actually update none of the prices if that happened.

I'm guessing fewyn's setup will never automatically remove the bad line.
 

xKiv

Active member
Code:
>>> datetime.datetime.fromtimestamp(1405180833)
datetime.datetime(2014, 7, 12, 18, 0, 33)

This is years old, too.
 

Veracity

Developer
Staff member
Also, sharePriceData isn't set to true by default. Should mafia be changed so it always loads updateprices.php at login for accurate pricing information, regardless of that setting? That's just my personal feeling, kind of like how I'd love for svnUpdateOnLogin to be true by default.
Personally, I don't like svnUpdateOnLogin, so opinions vary there.

Regarding sharePriceData, you propose that we always do that, no setting needed? Or only to load price data on login, whether or not you choose to share on logout?

I could live with former - or for making the default value for the setting be "true", at least.
The latter, allowing taking without giving, not so much.

Other than that, this bug is fixed, right?

(Although, perhaps fewyn would be willing to remove all the bogus entries from the file he is hosting, in order to speed up processing it at login?)
 

heeheehee

Developer
Staff member
IMO, svnUpdateOnLogin became much less obnoxious when we merged in xKiv's (iirc) patch to parallelize svn updating scripts.
 

lostcalpolydude

Developer
Staff member
I have that turned off to be friendly to sourceforge, mostly. Half of my installed scripts use SVN for ease of installing, and are unlikely to ever need an update. The ones that have to keep up with the game get regular manual update checks, and some niche scripts can get checked twice a year when I might actually use them.
 
Top