evil-morfar / RCLootCouncil2

RCLootCouncil - addon for World of Warcraft
https://rclootcouncil.com
GNU Lesser General Public License v3.0
19 stars 29 forks source link

Fix colorInfo error #193

Closed snowlove closed 4 years ago

snowlove commented 4 years ago

Removed LootSlot() API call because it should not be called when MasterLooting, it will cause a chain of errors through Blizzards LootFrame.lua, specifically the MasterLooterFrame_Show() Function, since the Slot[index] was never "Selected" by OnClick, when the MasterLooterFrame pops up. This can also be resolved by _G["LootButton"..index]:Click("RightButton") which fires off the approriate events to "Select" Slot[Index]

function MasterLooterFrame_Show()
    local itemFrame = MasterLooterFrame.Item;
    itemFrame.ItemName:SetText(LootFrame.selectedItemName);
    itemFrame.Icon:SetTexture(LootFrame.selectedTexture);
-----------------------
    local colorInfo = ITEM_QUALITY_COLORS[LootFrame.selectedQuality]; --[[<- Is nil because, 
 LootFrame.selectedQuality only populates when OnClick() occurs, which LootSlot() does not fire]]
-----------------------
    itemFrame.IconBorder:SetVertexColor(colorInfo.r, colorInfo.g, colorInfo.b);
    itemFrame.ItemName:SetVertexColor(colorInfo.r, colorInfo.g, colorInfo.b);

    MasterLooterFrame:Show();
    MasterLooterFrame_UpdatePlayers();
    MasterLooterFrame:SetPoint("TOPLEFT", DropDownList1, 0, 0);

    CloseDropDownMenus();
end

and is redundant for RCLootCouncil_Classsic at least, because GiveMasterLoot() is the right API to use for both awarding to someone else and self

evil-morfar commented 4 years ago

Interesting. I didn't make that code, but I seem to remember something about GiveMasterLoot not always working. But of course it's the proper function to use. Thanks.

snowlove commented 4 years ago

I can attest to GiveMasterLoot being very wonky, I've spent the past 2 days writing a module for my AddOn to auto loot AQ40 mounts to players based on color preference (which is absurd considering how easy it should have been). It seems when awarding to someone else it would just randomly throw the item out here, however if only looting to self like "Award Later" in RCLootCouncil_Classic it seems always loots properly to the current ML (I've spent way too many hours in Stockades with threshold set to common /endjoke)

Further Deep Dive I rewrote my AwardSlot function about 10 times and just couldn't get it to work with any method I was trying then I looked at the dumped blizzard code to see how it functions normally

I think they didn't really plan for people to use GiveMasterLoot() as a direct call because the way it operates is (slotindex, MasterLooterFrame[playerIndex].id) which I could only obtain with consistency from populating MasterLooterFrame with OnClick() (I tried every other combination of APIs GetMasterLootCandidate(), UnitInRaid(), etc), so my work around was to:

This code has nothing to do with RCLootCouncil, just an example of how I managed to utilize GiveMasterLoot() with consistency

local function AwardSlot(index, name, quality, callback)
       --lootsession is a table that is populated on loot opened
       --struct: { ['index']=slotid, ['name']=itemname, ['quality']=itemQuality, ['looted']=bool, ['link']=itemLink } using tinsert()
      --[[ ['looted'] because I clear items from session out on the event LOOT_SLOT_CLEARED
            and this can fire multiple times for 1 slot leading to some double looting attempts if I didn't (verified by /etrace)
        ]]
    local Threshold = GetLootThreshold()
    if quality < Threshold then LootSlot(index) return end --because it's either coins or below threshold

    if callback then
        index = lootsession.session.index
        name = lootsession[index].name
        quality = lootsession[index].quality
        lootsession.session.index = nil
    else
        lootsession.session.index = index
        _G["LootButton"..lootsession[index].index]:Click("RightButton")
        return
    end

        local MLFrameIndex
    for i=1, 40 do
        MLFrameIndex = "player"..i
        if MasterLooterFrame[MLFrameIndex] then
            if MasterLooterFrame[MLFrameIndex].Name:GetText() == winner.name then
                winner.index = MasterLooterFrame[MLFrameIndex].id
            end
            if MasterLooterFrame[MLFrameIndex].Name:GetText() == UnitName("PLAYER") then
                winner.mlindex = MasterLooterFrame[MLFrameIndex].id
            end
            print(tostring("player:"..MasterLooterFrame[MLFrameIndex].Name:GetText().." at: "..MasterLooterFrame[MLFrameIndex].id))
        end
    end
    --[[
          .......rest of code and call to GiveMasterLoot()
      ]]

With the callback being fired from LootButton[index]:Click("RightButton") and the event OPEN_MASTER_LOOT_LIST

  if event == "OPEN_MASTER_LOOT_LIST" then
    if not GlimmAUTOLOOT then return end

    if MasterLooterFrame:IsVisible() then
    MasterLooterFrame:Hide()
    AwardSlot(_,_,_,true)
    end
  end

And not to my surprise print(tostring("player:"..MasterLooterFrame[MLFrameIndex].Name:GetText().." at: "..MasterLooterFrame[MLFrameIndex].id)) debug string was returning different IDs each loot session even though the player was the same and the party size hasn't changed I believe it's due to Blizzard populating the MasterLooterFrame player1-player40 keys randomly or some other inconsistent way, just my theory.

I'm going to go take an aspirin now, thank you for the AddOn and your time!

evil-morfar commented 4 years ago

GetMasterLooterCandidate not being consistent is known, and has always been so (see the wiki).

Trust me, I've spend many an hour in Stockades as well ;)

Also, do you know why MasterLooterFrame_Show would be called by calling LootSlot?

snowlove commented 4 years ago

Yeah, because if you are master looting and the threshold is say 4==Epic and you loot a body, with an Epic on it (>=4 it's a Master Looted item)

And if you call LootSlot(index) on a Master Looted item it will drop down the "Who do you want this to go to menu?" because it doesn't know where to go since LootSlot() only has a slot parameter and no "Who" parameter like GiveMasterLoot(slot, who) has, as you know. Which then enters the Master_Loot functions territory

so LootSlot() won't start the chain of events that are required in Blizzards LootFrame.lua, because they only have OnClick() scripts attached to them. Which will cause variables to be nil and cascade a chain of errors through LootFrame.lua

You also have 1 more reference to LootSlot() in your codebase but it's used safely (coin checking) so I didn't touch it

~~if I'm not making sense let me know I tend to over complicate trying to explain things pseudocode would be something like:~~

OnLoot('item index')-> MasterLootThreshold(bool) ? WhoWins() : LootItem('done nothing needs to happen')

WhoWins()-> GetItemQuality()-> ColorBorder()-> Populate_Players_In_Group('Widgets w/Tables attached'){Event: Widget_OnClick()}-> ShowList()

Widget_OnClick()->GiveMasterLoot('item index', {Table.id])

the 2nd line isn't in that exact order it was a few days ago that I was stack tracing everything that happens when an item is looted and I didn't write them down in order just enough that I could work with it within my own AddOn.

This is honestly one of the most troubling things I've run into with WoWs API since vanilla I wish they could be more consistent with the API GetMasterLooterCandidate it would make everything so much easier than my bulky file to "emulate" master looting, I've even managed to cause a critical exception in the WoW executable itself just a moment ago and submitted a crash report with a link to my most recent repo with my module code, it seems to be linked to CloseLoot() 😛 not the first time I've managed to crash the entire game with AddOn code and I'm sure it won't be the last

snowlove commented 4 years ago

Sorry, I way over complicated that. it's easier to just see what's happening https://i.imgur.com/m7gYZFq.png when LootSlot() is called on that silk cloth it pops the MasterLooterFrame which never populated because of OnClick() script, however if you notice the debug text the table still lingers from the last session and so does the item as you can see in the DropDownMenu, but the list is never re-populated and updated, and if you're looting for the first time when you login that whole table is nil so that's when the error occurs.

evil-morfar commented 4 years ago

Sorry for the delay. Nice findings, but it still seems odd that LootSlot would have any side effects on MasterLooterFrame. I mean it's entire purpose is to loot a slot, not to go through any ML stuff. I also can't find any correlation in the Blizzard code for the looting frame, so it must be a side effect of the C call.