cdmichaelb / Outfitter

Outfitter - Classic
MIT License
10 stars 23 forks source link

Update for 3.4.1 container changes #129

Closed SabreValkyrn closed 1 year ago

SabreValkyrn commented 1 year ago

Locally hacked mine up, this got things mostly working.

To OutfitterEquipment.lua, add at top

local PickupContainerItem = C_Container and C_Container.PickupContainerItem or _G.PickupContainerItem

To Outfitter.lua

local GetContainerNumSlots = C_Container and C_Container.GetContainerNumSlots or _G.GetContainerNumSlots
local GetContainerItemLink = C_Container and C_Container.GetContainerItemLink or _G.GetContainerItemLink
local GetContainerNumFreeSlots = C_Container and C_Container.GetContainerNumFreeSlots or _G.GetContainerNumFreeSlots
local ContainerIDToInventoryID = C_Container and C_Container.ContainerIDToInventoryID or _G.ContainerIDToInventoryID

to OutfitterInventory.lua

local GetContainerNumSlots = C_Container and C_Container.GetContainerNumSlots or _G.GetContainerNumSlots
local GetContainerItemLink = C_Container and C_Container.GetContainerItemLink or _G.GetContainerItemLink
local GetContainerItemInfo = C_Container and C_Container.GetContainerItemInfo or _G.GetContainerItemInfo
local GetContainerItemGems = C_Container and C_Container.GetContainerItemGems or _G.GetContainerItemGems

to OutfitterBar

local GetContainerNumSlots = C_Container and C_Container.GetContainerNumSlots or _G.GetContainerNumSlots
local GetContainerItemLink = C_Container and C_Container.GetContainerItemLink or _G.GetContainerItemLink
local GetContainerItemInfo = C_Container and C_Container.GetContainerItemInfo or _G.GetContainerItemInfo
Kongolwow commented 1 year ago

No dice for me, are you just pasting the code for each right at the top of each file?

Coch commented 1 year ago

Same -- still getting LUA related to SetGradientAlpha (not expecting that to change based on what you shared) and app is still mostly non-functional

gotmilk0112 commented 1 year ago

Yeah, doesn't work for me even after adding those lines at the tops of the specified files.

https://i.imgur.com/dScKzEn.png

jasonb4610 commented 1 year ago

The SetGradientAlpha issue is something of a different issue but the fix is fairly straightforward. The API call changed to SetGradient and the method signature is slightly different ...

In OutfitterUITools.lua self.Background.Shadow:SetGradient( "HORIZONTAL", CreateColor(1,1,1,1), CreateColor(1,1,1,0))

gotmilk0112 commented 1 year ago

That just makes the window background transparent. Everything else is still broken.

https://i.imgur.com/iyCjbvP.png

jasonb4610 commented 1 year ago

There is a chunk of code in OutfitterInventory that is referencing gems and AzeriteCodes (which I doubt we need for Wrath version of anything) - I commented those out and things seem to be working now.

Around line 86 - 99 for my version with the previous changes listed above, comment out starting at itemInfo.Gem1, itemInfo.Gem2, etc. all the way to itemInfo.AzeriteCodes = line

gotmilk0112 commented 1 year ago

Still not working. I even tried deleting and re-downloading a fresh Outfitter, then making those changes, still nothing.

https://i.imgur.com/LtFA8GS.png https://i.imgur.com/OHMfSt8.png

robertzak commented 1 year ago

The error I'm seeing with the changes from above is:

Message: Interface/AddOns/Outfitter/OutfitterInventory.lua line 89:
   Usage: GetItemInfo(itemID|"name"|"itemlink")
Debug:
   [string "=[C]"]: GetItemInfo()
   [string "@Interface/AddOns/Outfitter/OutfitterInventory.lua"]:89: GetBagItemInfo()
   [string "@Interface/AddOns/Outfitter/OutfitterInventory.lua"]:838: Synchronize()
   [string "@Interface/AddOns/Outfitter/OutfitterInventory.lua"]:749: GetInventoryCache()
   [string "@Interface/AddOns/Outfitter/Outfitter.lua"]:4026: GetNewItemsOutfit()
   [string "@Interface/AddOns/Outfitter/OutfitterEquipment.lua"]:984: RestoreSavedStack()
   [string "@Interface/AddOns/Outfitter/OutfitterEquipment.lua"]:962: Initialize()
   [string "@Interface/AddOns/Outfitter/Outfitter.lua"]:4786: Function()
   [string "@Interface/AddOns/Outfitter/Libraries/MC2SchedulerLib/MC2SchedulerLib.lua"]:242: OnUpdate2()
   [string "@Interface/AddOns/Outfitter/Libraries/MC2SchedulerLib/MC2SchedulerLib.lua"]:178: OnUpdate()
   [string "@Interface/AddOns/Outfitter/Libraries/MC2SchedulerLib/MC2SchedulerLib.lua"]:20:
      ...fitter/Libraries/MC2SchedulerLib/MC2SchedulerLib.lua:20

I'm not LUA developer, but According to this page https://wowpedia.fandom.com/wiki/API_GetContainerItemGems

that function was removed in Legion and then added back for WotLK Classic. It does appear to be a valid function of C_Container, but either it's not returning the same thing it used to, or the call to GetItemInfo needs to be modified.

I followed what @jasonb4610 said and commented out that section of code an it's "working" again, but I worry if that will mess something up based on which gems you have in your items. I don't know enough about the code to know what that info is being used for though.

robertzak commented 1 year ago

Here is code commented out if that's helpful to anyone:

    --itemInfo.Gem1, itemInfo.Gem2, itemInfo.Gem3, itemInfo.Gem4 = GetContainerItemGems(bagIndex, slotIndex)
    --if itemInfo.Gem1 ~= nil then
    --  itemInfo.Gem1Link = select(2, GetItemInfo(itemInfo.Gem1))
    --end
    --if itemInfo.Gem2 ~= nil then
    --  itemInfo.Gem2Link = select(2, GetItemInfo(itemInfo.Gem2))
    --end
    --if itemInfo.Gem3 ~= nil then
    --  itemInfo.Gem3Link = select(2, GetItemInfo(itemInfo.Gem3))
    --end
    --if itemInfo.Gem4 ~= nil then
    --  itemInfo.Gem4Link = select(2, GetItemInfo(itemInfo.Gem4))
    --end
    -- itemInfo.AzeriteCodes = self:GetAzeriteCodesForLocation(location)
jasonb4610 commented 1 year ago

https://imgur.com/a/ZYsZ4TJ

It doesn't seem to have any effect on item swapping even from drastically different gearsets like Prot to Ret for example so I think it is safe to at least have something functional until people smarter than I give a better fix.

gotmilk0112 commented 1 year ago

Yeah, I'm not sure what's going on; that's the exact block of code that I commented out, and my Outfitter is still not working. Gonna try deleting the WTF data and see if that does anything.

SabreValkyrn commented 1 year ago

I also commented out the azerite and gems locally. Think I made one more change, was trying to get the plethora of lua spam to stop so I could raid Naxx10+25+eoe25+os25 back to back. :joy:

I'll try to make a PR later tonight with my local hacks/changes. Looks like owners are actively working on changes, https://github.com/cdmichaelb/Outfitter/compare/3.0.2...master

robertzak commented 1 year ago

@gotmilk0112 You still need all the changes SabreValkyrn posted too. If it's still not working for you, I'm not sure. Do you have any errors you can share?

I'm not sure if blizzard normally gives a stack trace, or if I have a mod that does that..I run so many mods..

gotmilk0112 commented 1 year ago

Yeah, I tried those changes too and they didn't do anything. The addon is not throwing any bugs/errors, at least, BugSack isn't detecting anything.

gotmilk0112 commented 1 year ago

Oh, there we go. Not sure what I changed but it's working now. https://i.imgur.com/sfhjCWg.png Alrighty then.

jasonb4610 commented 1 year ago

It looks like the code for gems in there is related to unique gems for probably just jewelcrafting? Band aids are band aids for now

bimrin commented 1 year ago

Yeah its bizarre that commenting out Gems fixed it when that function should be valid, I did that and changed things to use the SetGradient call instead and seem to be in a working state.

ssateneth commented 1 year ago

The addon is shitting the bed because GetContainerItemGems now returns a table instead of jsut the regular returns as expected. Since an empty table is still considered non-nil, it tries to pass the empty table to the GetItemInfo function which expects not a table.

Unpack the table first

ssateneth commented 1 year ago

I located

itemInfo.Gem1, itemInfo.Gem2, itemInfo.Gem3, itemInfo.Gem4 = GetContainerItemGems(bagIndex, slotIndex)

and changed to

itemInfo.Gem1, itemInfo.Gem2, itemInfo.Gem3, itemInfo.Gem4 = unpack(GetContainerItemGems(bagIndex, slotIndex))

No more errors, addon works as expected now (in addition to the changes in post 1 and 5)

ssateneth commented 1 year ago

here is patched outfitter, let me know how it works. I am not a hardcore coder, I just check APi changes and adjust as needed.

https://cdn.discordapp.com/attachments/852761173559738421/1065366172901585049/Outfitter.zip

redownload if you downloaded earlier. i left in a debug print by accident, fixed

inno-simon commented 1 year ago

here is patched outfitter, let me know how it works. I am not a hardcore coder, I just check APi changes and adjust as needed.

https://cdn.discordapp.com/attachments/852761173559738421/1065366172901585049/Outfitter.zip

redownload if you downloaded earlier. i left in a debug print by accident, fixed

this works for me, thanks homie

Persefonee commented 1 year ago

here is patched outfitter, let me know how it works. I am not a hardcore coder, I just check APi changes and adjust as needed.

https://cdn.discordapp.com/attachments/852761173559738421/1065366172901585049/Outfitter.zip

redownload if you downloaded earlier. i left in a debug print by accident, fixed

This works for me also. Thank you!!

GovtGeek commented 1 year ago

Not sure if you are waiting for packages, but the code itself has had several updates over the last day. Some of your suggested changes were even added in.

ssateneth commented 1 year ago

where was stuff "added in"? the latest commit here was in december.

GovtGeek commented 1 year ago

The last package was created then, but individual files have been updated, including the 4 mentioned at the beginning of this thread.

I do have a forked version with a release if you want to try, but I would like to keep this repo updated too.

robertzak commented 1 year ago

@GovtGeek It looks like your version is still missing ssateneth's fix in OutfitterInventory.lua line 89:

itemInfo.Gem1, itemInfo.Gem2, itemInfo.Gem3, itemInfo.Gem4 = unpack(GetContainerItemGems(bagIndex, slotIndex))
GovtGeek commented 1 year ago

@robertzak I now have the change made exactly as you suggested. I have not been able to log in to verify that it works, so hopefully it's all right! :)

robertzak commented 1 year ago

I can try after work in a few hours. I tried your previous version real quick this morning, but had to roll it back to my own hacked up version since it was still broken.

Gordory commented 1 year ago

@ssateneth thanks! That works! I am testing it for two days and it works perfectly

@GovtGeek I can make a PR with ssateneth changes, or you'll do it yourself?

GovtGeek commented 1 year ago

@Gordory There are some more changes I think need to be made. I'll try to get them completed soon, but there's a pull request outstanding, so I'm not sure if or when pull requests will be applied.

ssateneth commented 1 year ago

Theres an issue with my zipped version where left clicking an armor piece in the list of accessories/odds an ends produces a lua error, because UseContainerItem has been moved to C_Container.UseContainerItem (just add a localized version referencing the new location and it fixes)

Theres still an issue with icons not appearing in the odds n ends list too. I don't particularly care enough to look into how to fixing that though. Also I know nothing of how to do source pulls, pushes, forks, merge requests, etc. You guys can do whatever you want with the patched versions. I guess TECHNICALLY you need the author's permission, but they are MIA and if they have a problem with copies, alterations, they can let us know.

@Gordory you can do whatever you want with my zipped versions.

New zipped version https://cdn.discordapp.com/attachments/852761173559738421/1066517079966093322/Outfitter.zip

GovtGeek commented 1 year ago

I'm working on fixing those issue. Maybe tomorrow as I'm busy today.

cdmichaelb commented 1 year ago

long time no see @ssateneth :p

GovtGeek commented 1 year ago

Glad to see you're around @cdmichaelb. I've made more updates to remove the local overrides (the workaround that was started in this thread). At least one call fundamentally changed, so it was breaking things down the line with the override and it wasn't indicating the real error.

Think we can get an official release soon so curse will have a good copy?

Gogo1951 commented 1 year ago

Thanks @GovtGeek @SabreValkyrn @robertzak @ssateneth - anyone else I'm missing! (=

PR Merged.

GovtGeek commented 1 year ago

@Gogo1951 I had one extra commit that made it into the pull by accident. You may want to revert the Readme file. Sorry!