andrew6180 / TradeSkillMaster

TradeSkillMaster 2.8 backported to 3.3.5
49 stars 14 forks source link

[Core] Ore convert price not available #31

Closed ProfessorMinius closed 1 year ago

ProfessorMinius commented 1 year ago

Saronite/cobalt/titanium ore conversion prices will not show and don't work for custom prices. Here's a screenshot. Eternals and other things work, but ores don't. image

Arcitec commented 1 year ago

@ProfessorMinius I just tested this on Warmane, with the TradeSkillMaster: Revived fork.

image

The market value is calculated correctly, as expected.

There is nothing in TSM's pricing algorithm that would treat ores differently than any other item. TSM doesn't even know what an item is. It just grabs the item ID and then grabs the market value from the AuctionDB (as seen above, the market value is working).

When posting the item, it calculates the automatic pricing correctly:

image

You might be experiencing some random bug in the old TSM, which my new project has fixed. Alternatively, your custom pricing formula is wrong.

Here's my advanced pricing algorithm, which you should put in your "Operations: Auctioning: (Your operation): Post" rules. It automatically calculates reasonable prices for items and refuses to sell things unless you can earn more than 2x the vendor sell value (if it's so close to vendor sell, you should just go vendor it without wasting time and losing deposit costs).

image

ProfessorMinius commented 1 year ago

@Bananaman the problem isn't the price calculation itself, it's that convert() doesn't work for ores (which if I remember correctly, in the retail version of TSM it should work) i.e. it doesn't return the destroy (prospect) value of the ore.

Arcitec commented 1 year ago

@ProfessorMinius Your screenshot says that you are using convert(dbmarket). I just tried the same, and didn't get any errors, but also didn't get any prospecting value either.

I decided to trace the code a bit...

I'll have a quick look again to see if it's truly sending the item string properly, hold on a bit... Edit: Yes it sends the string properly. targetItem = the item being scanned, and priceSource = dbMarket.

Arcitec commented 1 year ago

@ProfessorMinius I've found the issue. convert() doesn't do what you thought it does.

TSMAPI:GetConvertCost(targetItem, priceSource) does the following:

["item:36927:0:0:0:0:0:0"] = { -- Twilight Opal
        ["item:36909:0:0:0:0:0:0"] = {rate=.01, source="prospect"},
        ["item:36912:0:0:0:0:0:0"] = {rate=.04, source="prospect"},
        ["item:36910:0:0:0:0:0:0"] = {rate=.04, source="prospect"},
    },

That's what the convert function does. There's unfortunately no bug here. It's clearly used by various crafting algorithms to figure out the cost of getting reagents.

In short: convert() does NOT figure out how much ore is worth if prospected into gems. It tells you how much a GEM is worth when prospected FROM the cheapest ore. That is why you got the error "convert(dbmarket) is a valid custom price but did not return a value for Saronite Ore". Because it's not what convert is for.

Arcitec commented 1 year ago

@ProfessorMinius Furthermore, a really quick glance in TradeSkillMaster/Core/Prices.lua shows that it doesn't seem to have any operator to do what you want (finding the "prospect worth of an ore"). This is an old version of TSM, remember. It's more basic than retail TSM. :/

Arcitec commented 1 year ago

Oh and my advice about pricing formulas still stands. Those formulas I provided are great for automatically figuring out prices for all items without being ripped off. If something is currently too cheap on the AH, it refuses to post your items.

ProfessorMinius commented 1 year ago

@Bananaman alright, I understand. Do you know of a way to get the prospect value, like the value you see in the UI? Would be handy when buying ores when having to calculate the profit margin and turnout. As I said, the "destroy" value source in newer TSM returns that value of prospecting ores, just as it returns the "disenchanting" value for equipment, like we have in TSM2.

Arcitec commented 1 year ago

@ProfessorMinius The tooltip uses the following code:

https://github.com/Bananaman/TradeSkillMaster/blob/9f78dc9097a812a564a966cb2d1d1e181500668d/TradeSkillMaster/TradeSkillMaster.lua#L501-L540

From that we can see that it uses a few functions:

Next step: Let's see what calls that function:

 ~/./a/c/d/b/b/P/d/W/I/AddOns $ rg GetProspectValue TradeSkillMaster*
TradeSkillMaster/TradeSkillMaster.lua
503:        local prospectValue = TSM:GetProspectValue(itemString)
640:function TSM:GetProspectValue(itemString)

Answer: No... There is no way in this old TSM version to get the prospect values in the automatic pricing algorithms. It's only used in the tooltip code. Furthermore, the code of "GetProspectValue" is very limited. It cannot define a pricing source. It just assumes "100% dbmarket" as its source.

The way TSM parses the custom pricing algorithm strings (like max(200% vendorsell, 70% dbmarket)) is also an utter mess. It was the code I mentioned earlier when investigating what convert() does. Basically, it's some of the worst spaghetti code I have seen in my life. There's zero structure, tons of repetition, and it's a miracle that the existing code even works at all.

One thing is for sure: Modifying convert(dbmarket) is out of the question. It could have effects that break lots of areas of TSM's code which rely on the old conversion algorithm. The best solution would be introducing a new codeword like max(200% vendorsell, 70% dbmarket, 100% prospect) and hooking the "prospect" keyword into TSM:GetProspectValue(itemString). But I am not offering to do that. It would require lots of research, testing and ensuring that nothing else breaks by introducing more codewords. It'd be a few hours of work to do it properly to be sure there aren't any new bugs, since TSM's price string parsing code is an utter fucking mess, and it'd require deep research into all areas of TSM that use the price string parser. Their parser is literally one huge run-on code block with the most insane code "logic" I've seen in years. Massive mess. I just came out of a few days of massive TSM rewrites and the last thing I want is another job in one of the messiest areas of TSM yet. ;)

ProfessorMinius commented 1 year ago

@Bananaman Ah yea, introducing new sources would be a mess. I saw the parsing code myself like a few months ago while diving into how the addon works. It's a bloody mess lol. Anyways, thank you for the help. I'll be closing this now.

Arcitec commented 1 year ago

The insane code I'm mentioning is here:

https://github.com/Bananaman/TradeSkillMaster/blob/tsm-revived/TradeSkillMaster/Core/Prices.lua

It's hundreds of line of utter fucking garbage code. One of the biggest messes I've ever seen. There's no intelligence to it. It even ends by dynamically creating a function from a huge string text-block, which in turn is a massive fucking bloater of a mess too. That's what would have to be researched, analyzed and properly edited to add a new "prospect" feature.

Arcitec commented 1 year ago

It's a bloody mess lol. Anyways, thank you for the help. I'll be closing this now.

That's a good way to describe it. It's definitely in a messy league of its own. :)

I'll be heading into WoW to relax now with the working TSM fork, finally. Been having four days off from the game just working 8 hours a day on the rewrite instead. Can't wait to get back to the fun! :) By the way, do you happen to play on Warmane's Icecrown?

ProfessorMinius commented 1 year ago

By the way, do you happen to play on Warmane's Icecrown? I actully play on Lordaeron

Arcitec commented 1 year ago

@ProfessorMinius Ah that's awesome, for me it was hard to pick between Lordaeron and Icecrown. Both seemed great! I ended up as Gnomezilla on Icecrown's Alliance. I'm a returner to Warmane after 3 years away, so heck if anyone plays there feel free to message me ingame! :D Anyway, take care and have fun, perhaps I see you on Lordaeron someday if I want more challenging PvE content! ;) Have a great day! :)