local function activate(self, old, deactivate)
if deactivate then
deactivate(old)
end
local hook = old.HookedSellValue
if hook then
self.oldSellValue = old.oldSellValue
else
self.oldSellValue = GetSellValue
GetSellValue = function (item)
local self = AceLibrary("ItemPrice-1.0") -- not an upvalue
local value = self:GetSellValue(item)
if value then return value end
if self.oldSellValue then
return self.oldSellValue(item)
end
end
end
self.HookedSellValue = true
end
AceLibrary:Register(Lib, MAJOR_VERSION, MINOR_VERSION, activate)
Thanks, Jerry. I think the essential part of your suggestion basically is "transporting" the boolean flag indicating that the hook has already been done. So I think it can be done as simple as the following:
local function activate(self, old, deactivate)
if deactivate then deactivate(old) end
if not old.hasHookedGetSellValue then
local oldSellValue = _G.GetSellValue
_G.GetSellValue = function(item)
return self:GetSellValue(item, oldSellValue)
end
end
self.hasHookedGetSellValue = true
end
Note that, although the self parameter is an upvalue, I believe that when activate is called, it is actually passed exactly the same as AceLibrary("ItemPrice-1.0") will return. If that's correct, it means that the normal upgrade functionality of libraries will work when calling the library method self:GetSellValue(). If that's not correct, then a local variable could instead be set to AceLibrary("ItemPrice-1.0") the first time GetSellValue is called. Something like this:
local lib
_G.GetSellValue = function(item)
lib = lib or AceLibrary(MAJOR_VERSION)
return lib:GetSellValue(item, oldSellValue)
end
Btw, the reason I am keeping the call to the hook in the library method is mainly to follow Tekkub's advice of only computing the itemId once. It may require some costly string matching and a call to GetItemInfo. Another reason is to keep the hooking code as simple as possible so that I can easily change the main code later in the ordinary library method if the need arises.
If that's not correct, then a local variable could instead be set to AceLibrary("ItemPrice-1.0") the first time GetSellValue is called.
Keeping the upvalue is incorrect, because in the scenario (THE problematic scenario) where an older library is updated by a later version, GetSellValue will still call the old one.
The second version you provide is only better if no one calls GetSellValue between the activation of the first (old) version and the activation of the last.
That is why I personnaly think that it's safer to call AceLibrary each time.
But... that confuses me. Is this upvalue inside the activation function any different than the way addons normally use a library? They also have a local variable to the library instance returned by AceLibrary. For example:
local Abacus = AceLibrary("Abacus-2.0")
It is my understanding that this is perfectly safe because AceLibrary takes care of replacing the fields of the library table no matter if you obtained the reference before or after a new version is loaded. At least my second suggestion does essentially the same.
But... that confuses me. Is this upvalue inside the activation function any different than the way addons normally use a library?
No, it's the same. That probably why sometimes you get funky behavior, because libs that should be unique might not be, older version being kept as upvalue.
Quote from Bam »
It is my understanding that this is perfectly safe because AceLibrary takes care of replacing the fields of the library table no matter if you obtained the reference before or after a new version is loaded.
No, I don't think so. You only get a reference to the latest version available at the time of the call.
Look at the last parameter to AceLibrary:Register, externalFunc, to always update an upvalue to the latest version of a library. You would do :
local abacus = AceLibrary:HasInstance("Abacus-2.0") and AceLibrary("Abacus-2.0")
local function external(self, major, instance)
if major == "Abacus-2.0" then
abacus = instance
end
end
AceLibrary:Register(lib, MAJOR, MINOR, nil, nil, external)
After a lengthy chat on IRC about it, I've decided to go with the upvalue version. We read the code for AceLibrary and believe it is indeed safe to do. If it turns out there is a problem anyway, I will take the blame, of course. :)
Thus, ItemPrice-1.0 now supports the suggestion in http://www.wowwiki.com/API_GetSellValue. As mentioned previously, since ItemPrice-1.0 is a load-on-demand library, it is not enough to simply install it as an addon (i.e. stand-alone). It must also be explicitly loaded by "something". Typically this would happen if any addon has it as OptDependency in the TOC file. Another way could be calling AceLibrary("ItemPrice-1.0") manually using the /script slash command or something similar. If you already have an addon using the library, none of that is necessary, of course.
I just discovered that Blizzard's very own armory (http://armory.worldofwarcraft.com) lists sell values too. I don't know if it's a new thing or if I just haven't been paying attention. :)
In any case, I have mined prices from the armory and updated the library with these prices. I expect this will be the way I will update prices in the future. Blizzard says that all items that have been "discovered" are searchable in the armory. I assume this pretty much means that all official sell values can also be obtained from the armory. Assuming that is true, I think the library now is as accurate and complete as is possible. (Obviously, this is also assuming that my datamining scripts work correctly.) My first datamining of the armory resulted in the following compared to my last update from wowhead:
Total: 21738 items, 15855 prices
Removed 234 items, 3 prices
Added 143 items, 13 prices
Changed 14 items, 5 prices
Thus, not many changes overall to the sell values. That gives me good confidence that the mining was done correctly. And it also shows that Wowhead had pretty good data. The biggest change is that quite a large number of items have been removed. Looking over the names of those removed items it seems they are mostly items seen on the PTR at some point (some had funny names like "Test Gear" etc.) or they are items that no longer exist in the game.
I think this is pretty good news. There is one caveat, however. The armory does not provide any way to search for items with sell values. Thus, the only way to mine them is to use "brute force" and scan every single item-id from 1 to some high number (I chose 35000). This takes several hours and it not something I want to do too often. But since the prices are most likely much more correct and complete, it may not be necessary either. I will see how it goes...
Can't you just scan frequently for items that has no sell value in ItemPrice and only search those that already have a value not so often? Sorry if i missed something obvious ^^
Can't you just scan frequently for items that has no sell value in ItemPrice and only search those that already have a value not so often? Sorry if i missed something obvious ^^
Yes, I plan to do something like that. But that is still about 14.000 items to scan. So I don't know how frequently I can reasonably do that. The entire scan from 1-35.000 took over 9 hours! :shock:
But I will find a strategy that is both feasible for my computer and that doesn't put too much load on Blizzard's armory. I do not want my IP banned. :D
Can't you just scan frequently for items that has no sell value in ItemPrice and only search those that already have a value not so often? Sorry if i missed something obvious ^^
Yes, I plan to do something like that. But that is still about 14.000 items to scan. So I don't know how frequently I can reasonably do that. The entire scan from 1-35.000 took over 9 hours! :shock:
But I will find a strategy that is both feasible for my computer and that doesn't put too much load on Blizzard's armory. I do not want my IP banned. :D
Ask blizzard for a itemid / price dump every maintence day :-P
With the armory as a base, what is the status on items listed on the armory but not having a sell value? Can these be confirmed to be unsellable and thus could the API return a specific value for them?
With the armory as a base, what is the status on items listed on the armory but not having a sell value? Can these be confirmed to be unsellable and thus could the API return a specific value for them?
Just like wowhead, the armory doesn't specifically indicate "No sell price" or something of that nature. However, since the armory represents official data from Blizzard, I think it is fairly safe to make the assumption that if an item is listed without sell value, then it is not sellable. Thus, I think I will incorporate that information into the library.
(Due to the way the compression works, this is actually a bit tricky. The only way I can do this is by choosing a number between 0 and 256^3-1 and designating this as a "fake" nil so to speak. The library function that returns price then has to test for this number and return nil. A bit awkward but doable and will of course happen entirely behind the scenes so users of the library should not need to care.)
(Due to the way the compression works, this is actually a bit tricky. The only way I can do this is by choosing a number between 0 and 256^3-1 and designating this as a "fake" nil so to speak. The library function that returns price then has to test for this number and return nil. A bit awkward but doable and will of course happen entirely behind the scenes so users of the library should not need to care.)
To be precise. The issue is not having an information "no sell price", that you can easily embed as "has a sell price of 0", but items for which the sell price is unknown, because the item isn't listed in the armory.
I would put '\255\255\255' for such cases. This corresponds to 1677 gold, the highest known sell price being 82 gold, you still have some margin.
I would put '\255\255\255' for such cases. This corresponds to 1677 gold, the highest known sell price being 82 gold, you still have some margin.
Yes, that's one possibility. Though I think I will choose a number that takes up fewer characters in the Lua-file since there will be quite a lot of entries having this number. I just need to pick a number and make sure the compression script asserts that it doesn't clash with a real price. Might be overkill, but it's not much more work and will not affect users of the library since it all happens at code-generation time. :)
By API 0 would be the value to return in case you know that the item cannot be sold. Since there is a difference between the item not being listed or being listed without a sell value I think 0 is safe untill a vendor tells something else.
Rollback Post to RevisionRollBack
To post a comment, please login or register a new account.
local function activate(self, old, deactivate) if deactivate then deactivate(old) end local hook = old.HookedSellValue if hook then self.oldSellValue = old.oldSellValue else self.oldSellValue = GetSellValue GetSellValue = function (item) local self = AceLibrary("ItemPrice-1.0") -- not an upvalue local value = self:GetSellValue(item) if value then return value end if self.oldSellValue then return self.oldSellValue(item) end end end self.HookedSellValue = true end AceLibrary:Register(Lib, MAJOR_VERSION, MINOR_VERSION, activate)?
Note that, although the self parameter is an upvalue, I believe that when activate is called, it is actually passed exactly the same as AceLibrary("ItemPrice-1.0") will return. If that's correct, it means that the normal upgrade functionality of libraries will work when calling the library method self:GetSellValue(). If that's not correct, then a local variable could instead be set to AceLibrary("ItemPrice-1.0") the first time GetSellValue is called. Something like this:
Btw, the reason I am keeping the call to the hook in the library method is mainly to follow Tekkub's advice of only computing the itemId once. It may require some costly string matching and a call to GetItemInfo. Another reason is to keep the hooking code as simple as possible so that I can easily change the main code later in the ordinary library method if the need arises.
Keeping the upvalue is incorrect, because in the scenario (THE problematic scenario) where an older library is updated by a later version, GetSellValue will still call the old one.
The second version you provide is only better if no one calls GetSellValue between the activation of the first (old) version and the activation of the last.
That is why I personnaly think that it's safer to call AceLibrary each time.
local Abacus = AceLibrary("Abacus-2.0")It is my understanding that this is perfectly safe because AceLibrary takes care of replacing the fields of the library table no matter if you obtained the reference before or after a new version is loaded. At least my second suggestion does essentially the same.
No, it's the same. That probably why sometimes you get funky behavior, because libs that should be unique might not be, older version being kept as upvalue.
No, I don't think so. You only get a reference to the latest version available at the time of the call.
Look at the last parameter to AceLibrary:Register, externalFunc, to always update an upvalue to the latest version of a library. You would do :
local abacus = AceLibrary:HasInstance("Abacus-2.0") and AceLibrary("Abacus-2.0") local function external(self, major, instance) if major == "Abacus-2.0" then abacus = instance end end AceLibrary:Register(lib, MAJOR, MINOR, nil, nil, external)Thus, ItemPrice-1.0 now supports the suggestion in http://www.wowwiki.com/API_GetSellValue. As mentioned previously, since ItemPrice-1.0 is a load-on-demand library, it is not enough to simply install it as an addon (i.e. stand-alone). It must also be explicitly loaded by "something". Typically this would happen if any addon has it as OptDependency in the TOC file. Another way could be calling AceLibrary("ItemPrice-1.0") manually using the /script slash command or something similar. If you already have an addon using the library, none of that is necessary, of course.
In any case, I have mined prices from the armory and updated the library with these prices. I expect this will be the way I will update prices in the future. Blizzard says that all items that have been "discovered" are searchable in the armory. I assume this pretty much means that all official sell values can also be obtained from the armory. Assuming that is true, I think the library now is as accurate and complete as is possible. (Obviously, this is also assuming that my datamining scripts work correctly.) My first datamining of the armory resulted in the following compared to my last update from wowhead:
Thus, not many changes overall to the sell values. That gives me good confidence that the mining was done correctly. And it also shows that Wowhead had pretty good data. The biggest change is that quite a large number of items have been removed. Looking over the names of those removed items it seems they are mostly items seen on the PTR at some point (some had funny names like "Test Gear" etc.) or they are items that no longer exist in the game.
I think this is pretty good news. There is one caveat, however. The armory does not provide any way to search for items with sell values. Thus, the only way to mine them is to use "brute force" and scan every single item-id from 1 to some high number (I chose 35000). This takes several hours and it not something I want to do too often. But since the prices are most likely much more correct and complete, it may not be necessary either. I will see how it goes...
Yes, I plan to do something like that. But that is still about 14.000 items to scan. So I don't know how frequently I can reasonably do that. The entire scan from 1-35.000 took over 9 hours! :shock:
But I will find a strategy that is both feasible for my computer and that doesn't put too much load on Blizzard's armory. I do not want my IP banned. :D
Ask blizzard for a itemid / price dump every maintence day :-P
-
View User Profile
-
View Posts
-
Send Message
Forum ModeratorJust like wowhead, the armory doesn't specifically indicate "No sell price" or something of that nature. However, since the armory represents official data from Blizzard, I think it is fairly safe to make the assumption that if an item is listed without sell value, then it is not sellable. Thus, I think I will incorporate that information into the library.
(Due to the way the compression works, this is actually a bit tricky. The only way I can do this is by choosing a number between 0 and 256^3-1 and designating this as a "fake" nil so to speak. The library function that returns price then has to test for this number and return nil. A bit awkward but doable and will of course happen entirely behind the scenes so users of the library should not need to care.)
To be precise. The issue is not having an information "no sell price", that you can easily embed as "has a sell price of 0", but items for which the sell price is unknown, because the item isn't listed in the armory.
I would put '\255\255\255' for such cases. This corresponds to 1677 gold, the highest known sell price being 82 gold, you still have some margin.
Yes, that's one possibility. Though I think I will choose a number that takes up fewer characters in the Lua-file since there will be quite a lot of entries having this number. I just need to pick a number and make sure the compression script asserts that it doesn't clash with a real price. Might be overkill, but it's not much more work and will not affect users of the library since it all happens at code-generation time. :)
-
View User Profile
-
View Posts
-
Send Message
Forum Moderator