Quantcast SecureHandlerAttributeTemplate optimization - WoWInterface
Thread Tools Display Modes
03-16-19, 10:48 AM   #1
Terenna
A Warpwood Thunder Caller
AddOn Author - Click to view addons
Join Date: Jun 2016
Posts: 98
SecureHandlerAttributeTemplate optimization

Good afternoon,

I along with other users have noticed a recent change with BFA where certain quests utilize an attribute change on action buttons to show them. The best example of this is during the quest Righteous Retribution: https://www.wowhead.com/quest=49741/...ution#comments

After mounting the gryphon Galeheart, you enter an Overridebar state. However, you initially don't have any buttons. The buttons are later added via an attribute change. Addons such as rActionBar, and I'm sure others, do not account for this, and assume the buttons will be there during their '_onstate-page' secure snippets. As such, I first came up with a solution to simply hooksecureframe a function that ran just as this occurred. While this code:
Lua Code:
  1. hooksecurefunc('ActionBarController_UpdateAll', function()  
  2.     for i = 1, 12 do
  3.         local button = _G['ActionButton'..i]
  4.         local overrideButton = _G['OverrideActionBarButton'..i]
  5.         local _, spellID
  6.         if overrideButton then
  7.             _, spellID = GetActionInfo(overrideButton.action)
  8.         end
  9.  
  10.         if ((HasOverrideActionBar() or HasVehicleActionBar()) and (spellID and spellID > 0)) or (not HasOverrideActionBar() and not HasVehicleActionBar()) then
  11.             button:SetAttribute('statehidden', false)
  12.             button:Show()
  13.         else
  14.             button:SetAttribute('statehidden', true)
  15.             button:Hide()
  16.         end
  17.     end
  18. end)

works wonderfully, it causes taint, as I'm changing the attributes of buttons outside of a secureframe.

As such, and not knowing much about SecureHandler*Template coding, I embarked on this endeavor. I came up with the following code:
Lua Code:
  1. local AttributeChangedFrame = CreateFrame('frame', nil, UIParent, 'SecureHandlerAttributeTemplate')
  2. for i = 1, 12 do
  3.     local button = _G['ActionButton'..i]
  4.     AttributeChangedFrame:SetFrameRef('ActionButton'..i, button)
  5. end
  6.  
  7. for i = 1, 6 do
  8.     local overrideButton = _G['OverrideActionBarButton'..i]
  9.     AttributeChangedFrame:SetFrameRef('OverrideActionBarButton'..i, overrideButton)
  10. end
  11. AttributeChangedFrame:Execute([[
  12.     buttons = table.new()
  13.     for i = 1, 12 do
  14.         table.insert(buttons, self:GetFrameRef('ActionButton'..i))
  15.     end
  16.     overridebuttons = table.new()
  17.     for i = 1, 6 do
  18.         table.insert(overridebuttons, self:GetFrameRef('OverrideActionBarButton'..i))
  19.     end
  20. ]])
  21.  
  22. for i = 1, 6 do
  23.     local overrideButton = _G['OverrideActionBarButton'..i]
  24.     overrideButton:HookScript('OnAttributeChanged', function()
  25.         AttributeChangedFrame:Execute[[
  26.             for i = 1, 6 do
  27.                 if not overridebuttons[i]:GetAttribute('statehidden') then
  28.                     buttons[i]:SetAttribute('statehidden', false)
  29.                     buttons[i]:Show()
  30.                 else
  31.                     buttons[i]:SetAttribute('statehidden', true)
  32.                     buttons[i]:Hide()
  33.                 end
  34.             end
  35.         ]]
  36.     end)
  37.    
  38.     local button = _G['ActionButton'..i]
  39.     button:HookScript('OnAttributeChanged', function()
  40.         AttributeChangedFrame:Execute[[
  41.             for i = 1, 6 do
  42.                 if (not HasOverrideActionBar() and not HasVehicleActionBar() and buttons[i]:GetAttribute('statehidden')) then
  43.                     buttons[i]:SetAttribute('statehidden', false)
  44.                     buttons[i]:Show()
  45.                 end
  46.             end
  47.         ]]
  48.     end)
  49. end

The basic idea is to hook into the OnAttributeChanged functions for both the normal actionbar button (ActionButton1-12) and the OverrideActionBarButton1-6. Here, I would execute secure code to change the attributes of the buttons to properly display the actionbutton when you "page" into an actionbar but the buttons come later. Just looking at this code, I feel like it is not great. I'm sure it can be better optimized, but I wanted to reach out to people who perhaps have better experience with SecureHandlers to determine my next steps.

Any feedback is greatly appreciated. Please note, the bottom code functions precisely how I'd like it to. It is taint free, and handles every scenario I can think of without fail. All I'm asking is for optimization so it doesn't run 36 times instead of 6 for example.
  Reply With Quote
03-16-19, 04:12 PM   #2
Resike
A Pyroguard Emberseer
AddOn Author - Click to view addons
Join Date: Mar 2010
Posts: 1,289
You don't have to use table.insert in Execute snippets, you can just `button[i] = ...`.

I think you have a flaw in the inner for cycle:
Lua Code:
  1. for j = 1, 6 do
  2.     if not overridebuttons[i]:GetAttribute('statehidden') then
  3.         buttons[j]:SetAttribute('statehidden', false)
  4.         buttons[j]:Show()
  5.     else
  6.         buttons[j]:SetAttribute('statehidden', true)
  7.         buttons[j]:Hide()
  8.     end
  9. end
  Reply With Quote
03-16-19, 04:52 PM   #3
Terenna
A Warpwood Thunder Caller
AddOn Author - Click to view addons
Join Date: Jun 2016
Posts: 98
Thanks for the reply.

Changed the i's inside of the i loops to j's and k's. Removed table.insert and set to button[i] =.

I wish there was a way to somehow access the i from outside of the secure execute snippet so I didn't have to change all 6 buttons every time any one of them changed.
  Reply With Quote
03-17-19, 11:22 PM   #4
Terenna
A Warpwood Thunder Caller
AddOn Author - Click to view addons
Join Date: Jun 2016
Posts: 98
As it turned out, there was a taint that was possible with the previous code.

I have completely rectified this and tested the hell out of it in several really weird edge cases and as far as I can tell, no taint. If anyone is interested in the code to alleviate the lack of button population in certain circumstances, you may find it here:
Lua Code:
  1. --[[
  2.     Assess whether the attribute that was changed and triggered the blizzard _onattributechanged event was the 'statehidden' attribute
  3.     Assess whether the player has an OverrideActionBar or VehicleActionBar
  4.     If they do, determine if the 6 (< 7) OverrideActionBarButtons which are used for VehicleActionBar, too, are hidden or not
  5.     If the player has a special bar and the OverrideActionBarButton is hidden, also hide the respective ActionButton that is part of the paging bar
  6.     However, if they have a special bar but the OverrideActionBarButton is shown, also show the respective ActionButton that is part of the paging bar
  7.     Hide the ActionButtons7-12 if the player has a special bar
  8.     If the player doesn't have an OverrideActionBar nor VehicleActionBar, show ActionButtons1-12 that are part of the paging bar
  9. ]]--
  10. local _onAttributeChanged = [[
  11.     if name == 'statehidden' then
  12.         if (HasOverrideActionBar() or HasVehicleActionBar()) then
  13.             for i = 1, 12 do
  14.                 if i < 7 then
  15.                     if overridebuttons[i]:GetAttribute('statehidden') then
  16.                         buttons[i]:SetAttribute('statehidden', true)
  17.                         buttons[i]:Hide()
  18.                     else
  19.                         buttons[i]:SetAttribute('statehidden', false)
  20.                         buttons[i]:Show()
  21.                     end
  22.                 else
  23.                     buttons[i]:SetAttribute('statehidden', true)
  24.                     buttons[i]:Hide()
  25.                 end
  26.             end
  27.         else
  28.             for i = 1, 12 do
  29.                 buttons[i]:SetAttribute('statehidden', false)
  30.                 buttons[i]:Show()
  31.             end
  32.         end
  33.     end
  34. ]]
  35. --[[
  36.     Generate a secure frame, AttributeChangedFrame, that is used to 'hook' into the _onattributechanged
  37.     Set frame references for both the ActionButtons and the OverrideActionBarButtons, which are used for both VehicleActionBar and OverrideActionBar bars
  38.     Make two secure tables, buttons and overridebuttons; populate it with the buttons using the just set frame references
  39.     'Hook' our secure frame to run the secure _onAttributeChanged snippet when the blizzard event, _onattributechanged, fires
  40.     Finish by registering the secure frame as a secure state driver, because for some reason this is required for the _onattributechanged to properly be hooked
  41. ]]--
  42. local AttributeChangedFrame = CreateFrame('Frame', nil, UIParent, 'SecureHandlerAttributeTemplate')
  43. for i = 1, 12 do
  44.     local button = _G['ActionButton'..i]
  45.     AttributeChangedFrame:SetFrameRef('ActionButton'..i, button)
  46. end
  47.  
  48. for i = 1, 6 do
  49.     local overrideButton = _G['OverrideActionBarButton'..i]
  50.     AttributeChangedFrame:SetFrameRef('OverrideActionBarButton'..i, overrideButton)
  51. end
  52.  
  53. AttributeChangedFrame:Execute([[
  54.     buttons = table.new()
  55.     for i = 1, 12 do
  56.         buttons[i] = self:GetFrameRef('ActionButton'..i)
  57.     end
  58.  
  59.     overridebuttons = table.new()
  60.     for j = 1, 6 do
  61.         overridebuttons[j] = self:GetFrameRef('OverrideActionBarButton'..j)
  62.     end
  63. ]])
  64.  
  65. AttributeChangedFrame:SetAttribute('_onattributechanged', _onAttributeChanged)
  66. RegisterStateDriver(AttributeChangedFrame, 'visibility', '[overridebar][shapeshift][vehicleui][possessbar] show; hide')

I've put in a bunch of documentation for why I did what I did. One thing I can't resolve, is why I need to RegisterStateDriver for visibility for the :SetAttribute('_onattributechanged', _onAttributeChanged) to actually fire. Without this RegisterStateDriver(...) line, this doesn't happen. Is there anyone who can explain that finding?

Last edited by Terenna : 03-18-19 at 07:51 AM.
  Reply With Quote
03-22-19, 10:25 PM   #5
siweia
A Flamescale Wyrmkin
 
siweia's Avatar
AddOn Author - Click to view addons
Join Date: Jul 2011
Posts: 114
I am fine with button 7-12 showing on vehicle bar.
Would that work if I just keep the second part of the code?
  Reply With Quote
03-23-19, 06:21 AM   #6
Vrul
A Rage Talon Dragon Guard
 
Vrul's Avatar
AddOn Author - Click to view addons
Join Date: Nov 2007
Posts: 312
Originally Posted by Terenna View Post
One thing I can't resolve, is why I need to RegisterStateDriver for visibility for the :SetAttribute('_onattributechanged', _onAttributeChanged) to actually fire. Without this RegisterStateDriver(...) line, this doesn't happen. Is there anyone who can explain that finding?
In the WoW Lua enviroment events are global and scripts are like a mini-event unique to an individual object. So when you do something like:
Code:
buttons[i]:SetAttribute('statehidden', true)
Each button[i]'s OnAttributeChanged script is called if their 'statehidden' attribute wasn't already true. You didn't hook each button[i]'s OnAttributeChanged script but rather made a secure frame with it's own OnAttributeChanged script. In that case you have to register a state driver for that new secure frame that changes states at the times you want to evaluate those buttons.

Hopefully I explained that in a way that answered your question.
  Reply With Quote
03-23-19, 11:41 AM   #7
Terenna
A Warpwood Thunder Caller
AddOn Author - Click to view addons
Join Date: Jun 2016
Posts: 98
That does make sense and your explanation was both succinct and understandable. When I did try and hook each individual button with the 2nd block of code in the original post for this thread, I got tainting. Curious if it was from shitty secure frame code that wasn't actually secure, I did something as simple as this:
Lua Code:
  1. overrideButton:HookScript('OnAttributeChanged', function()
  2. end)

and/or

Lua Code:
  1. button:HookScript('OnAttributeChanged', function()
  2. end)

that is, a blank function that simply hooked into the script, it caused tainting. So that's when I thought I couldn't hook into the individual buttons no matter what I did. To fairly easily recreate the taint, you can use the BfA "turtles" faction quests that involve you becoming the crab or defending the baby turtles. Allow yourself to get in combat by shooting something and then leave and then reenter and you'll taint with any sort of HookScript on the override or ActionButtons.

That being said, is my solution using the secure code snippets and the registered state driver the "correct" way to go about my initial desire? Or is there a less verbose method that doesn't involve secureframes?

Last edited by Terenna : 03-23-19 at 11:47 AM.
  Reply With Quote
03-23-19, 11:44 AM   #8
Terenna
A Warpwood Thunder Caller
AddOn Author - Click to view addons
Join Date: Jun 2016
Posts: 98
Originally Posted by siweia View Post
I am fine with button 7-12 showing on vehicle bar.
Would that work if I just keep the second part of the code?
If I'm understanding you correctly, you want your paged action bar's buttons 7-12 to still show if you enter a vehicle/override bar state. If so, try replacing the _onAttributeChanged secure snippet with this:

Lua Code:
  1. local _onAttributeChanged = [[
  2.     if name == 'statehidden' then
  3.         if (HasOverrideActionBar() or HasVehicleActionBar()) then
  4.             for i = 1, 12 do
  5.                 if i < 7 then
  6.                     if overridebuttons[i]:GetAttribute('statehidden') then
  7.                         buttons[i]:SetAttribute('statehidden', true)
  8.                         buttons[i]:Hide()
  9.                     else
  10.                         buttons[i]:SetAttribute('statehidden', false)
  11.                         buttons[i]:Show()
  12.                     end
  13.                 else
  14.                     buttons[i]:SetAttribute('statehidden', false)
  15.                     buttons[i]:Show()
  16.                 end
  17.             end
  18.         else
  19.             for i = 1, 12 do
  20.                 buttons[i]:SetAttribute('statehidden', false)
  21.                 buttons[i]:Show()
  22.             end
  23.         end
  24.     end
  25. ]]

If not, please give me more detail so I can help you better
  Reply With Quote
03-23-19, 07:44 PM   #9
Vrul
A Rage Talon Dragon Guard
 
Vrul's Avatar
AddOn Author - Click to view addons
Join Date: Nov 2007
Posts: 312
Originally Posted by Terenna View Post
That being said, is my solution using the secure code snippets and the registered state driver the "correct" way to go about my initial desire? Or is there a less verbose method that doesn't involve secureframes?
I would say your solution is ok. However, you are emulating a paging state driver with your visibility state driver. It could be simplified slightly(dry coded / untested):
Code:
local header = CreateFrame('Frame', nil, nil, 'SecureHandlerStateTemplate')

for id = 1, NUM_ACTIONBAR_BUTTONS do
    local name = 'ActionButton' .. id
    header:SetFrameRef(name, _G[name])
end

for id = 1, NUM_OVERRIDE_BUTTONS do
    local name = 'OverrideActionBarButton' .. id
    header:SetFrameRef(name, _G[name])
end

header:Execute(([[
    actionButton = table.new()
    for id = 1, NUM_ACTIONBAR_BUTTONS do
        actionButton[id] = self:GetFrameRef('ActionButton' .. id)
    end
 
    overrideButton = table.new()
    for id = 1, NUM_OVERRIDE_BUTTONS do
        overrideButton[id] = self:GetFrameRef('OverrideActionBarButton' .. id)
    end
]]):gsub('NUM_ACTIONBAR_BUTTONS', NUM_ACTIONBAR_BUTTONS):gsub('NUM_OVERRIDE_BUTTONS', NUM_OVERRIDE_BUTTONS))

header:SetAttribute("_onstate-override", ([[
    if newstate then
        for id = 1, NUM_OVERRIDE_BUTTONS do
            local stateHidden = overrideButton[id]:GetAttribute('statehidden')
            actionButton[id]:SetAttribute('statehidden', stateHidden)
            actionButton[id][stateHidden and 'Hide' or 'Show'](actionButton[id])
        end
        for id = NUM_OVERRIDE_BUTTONS + 1, NUM_ACTIONBAR_BUTTONS do
            actionButton[id]:SetAttribute('statehidden', true)
            actionButton[id]:Hide()
        end
    else
        for id = 1, NUM_ACTIONBAR_BUTTONS do
            actionButton[id]:SetAttribute('statehidden', false)
            actionButton[id]:Show()
        end
    end
]]):gsub('NUM_ACTIONBAR_BUTTONS', NUM_ACTIONBAR_BUTTONS):gsub('NUM_OVERRIDE_BUTTONS', NUM_OVERRIDE_BUTTONS))
RegisterStateDriver(header, "override", "[overridebar][vehicleui] true")
Since you are running an additional paging state driver at this point wouldn't it be easier to just change the parameters of the paging state driver for the addon that is misbehaving? This is what I use and haven't had any issues (I hardly play any more so that may be a factor, but I did complete Righteous Retribution without issue):
Code:
[vehicleui] 12; [overridebar] 14; [possessbar] [@vehicle, exists] 12; [shapeshift] 13; [bar:2] 2; [bar:3] 3; [bar:4] 4; [bar:5] 5; [bar:6] 6; [bonusbar:1] 7; [bonusbar:2] 8; [bonusbar:3] 9; [bonusbar:4] 10; [bonusbar:5] 11; 1
For future reference, to hook a script and run secure code you need to create a frame using one of the SecureHandler templates and then use it's WrapScript method:
Code:
local header = CreateFrame('Frame', nil, nil, 'SecureHandlerBaseTemplate')
header:WrapScript(ActionButton1, 'OnAttributeChanged', [[
    print(self:GetName(), 'OnAttributeChanged', name, value)
    -- Your secure snippet here
]])
  Reply With Quote
03-23-19, 08:11 PM   #10
siweia
A Flamescale Wyrmkin
 
siweia's Avatar
AddOn Author - Click to view addons
Join Date: Jul 2011
Posts: 114
I am using rActionbar atm, and do meet the overridebar update problem as you mentioned. But I need the button grids stay shown.
It is so weired that these code below actually work properly on vehicle buttons for rActionbar. Maybe it just need an update.
Lua Code:
  1. local AttributeChangedFrame = CreateFrame("Frame", nil, UIParent, "SecureHandlerAttributeTemplate")
  2. for i = 1, 12 do
  3.     local name = "ActionButton"..i
  4.     AttributeChangedFrame:SetFrameRef(name, _G[name])
  5. end
  6.  
  7. AttributeChangedFrame:Execute([[
  8.     buttons = table.new()
  9.     for i = 1, 12 do
  10.         buttons[i] = self:GetFrameRef("ActionButton"..i)
  11.     end
  12. ]])
  13. AttributeChangedFrame:SetAttribute("_onattributechanged", ([[
  14.     if HasOverrideActionBar() or HasVehicleActionBar() then
  15.         for i = 1, 12 do
  16.             buttons[i]:SetAttribute("state", newstate)
  17.         end
  18.     end
  19. ]]))
  20. RegisterStateDriver(AttributeChangedFrame, "visibility", "[overridebar][vehicleui][possessbar][shapeshift] show; hide")

Last edited by siweia : 03-24-19 at 03:07 AM.
  Reply With Quote
03-23-19, 08:15 PM   #11
siweia
A Flamescale Wyrmkin
 
siweia's Avatar
AddOn Author - Click to view addons
Join Date: Jul 2011
Posts: 114
Originally Posted by Vrul View Post
Since you are running an additional paging state driver at this point wouldn't it be easier to just change the parameters of the paging state driver for the addon that is misbehaving? This is what I use and haven't had any issues (I hardly play any more so that may be a factor, but I did complete Righteous Retribution without issue):
The page state driver works for most of the time. But it do have some update problem in some quests.
eg https://www.wowhead.com/quest=47261/...-your-direhorn
After mounting the gryphon Galeheart, you enter an Overridebar state. However, you initially don't have any buttons. The buttons are later added via an attribute change.

Last edited by siweia : 03-24-19 at 03:12 AM.
  Reply With Quote
03-24-19, 03:04 AM   #12
siweia
A Flamescale Wyrmkin
 
siweia's Avatar
AddOn Author - Click to view addons
Join Date: Jul 2011
Posts: 114
I just came up with an idea, the code below does fix the override buttons update as well. (for rActionbar)
Not sure whether it taints or not.
Lua Code:
  1. local updater = CreateFrame("Frame")
  2. updater:RegisterEvent("UPDATE_VEHICLE_ACTIONBAR")
  3. updater:SetScript("OnEvent", function()
  4.     for i = 1, 12 do
  5.         local button = _G["ActionButton"..i]
  6.         local texture = GetActionTexture(button.action)
  7.         if texture then
  8.             button.icon:SetTexture(texture)
  9.             button.icon:Show()
  10.         else
  11.             button.icon:Hide()
  12.         end
  13.     end
  14. end)

Last edited by siweia : 03-24-19 at 04:53 AM.
  Reply With Quote
04-04-19, 02:46 AM   #13
Nightness
An Aku'mai Servant
AddOn Author - Click to view addons
Join Date: Feb 2009
Posts: 32
Keep in mind...

Keep in mind Blizzard will not let you f*ck with attribute values while in combat or out of control... Your use of SetAttribute().

Last edited by Nightness : 04-04-19 at 03:03 AM.
  Reply With Quote
04-08-19, 04:51 AM   #14
zork
A Pyroguard Emberseer
 
zork's Avatar
AddOn Author - Click to view addons
Join Date: Jul 2008
Posts: 1,739
I solved the issue by manually page swapping when on such a quest. The onpage event fires before the actual button attribute change takes effect. If I had such a problem I would switch bar1 to bar5 and back and the buttons would show. It works because you manually trigger the onpage event with that swap. By the time you do it the button attributes are the correct ones.

My guess is that certain quests show the overridebar (with or without ui) and progressively add buttons to that bar based on events. rActionbar uses only the actionbar1 and uses the visibility state driver to trigger the actionbar page swap.

It might be possible to do what Vrul wrote. You try to track the attribute of the overridebar buttons and listen for the event change. It might be possible to apply the button attributes of the overridebar to the actionbar1 buttons or just trigger a pageswap (like the manual back and forth).
__________________
| Simple is beautiful.
| WoWI AddOns | GitHub | Zork (WoW) | TDMOG

"I wonder what the non-pathetic people are doing tonight?" - Rajesh Koothrappali (The Big Bang Theory)
  Reply With Quote
04-09-19, 08:35 PM   #15
siweia
A Flamescale Wyrmkin
 
siweia's Avatar
AddOn Author - Click to view addons
Join Date: Jul 2011
Posts: 114
Originally Posted by zork View Post
It might be possible to do what Vrul wrote. You try to track the attribute of the overridebar buttons and listen for the event change. It might be possible to apply the button attributes of the overridebar to the actionbar1 buttons or just trigger a pageswap (like the manual back and forth).
I have been using the texture show/hide way to solve this problem lately, it works perfectly atm without taint issues.
  Reply With Quote
04-10-19, 08:33 PM   #16
Xrystal
nUI Maintainer
 
Xrystal's Avatar
Premium Member
AddOn Author - Click to view addons
Join Date: Feb 2006
Posts: 5,345
Vrul's code should allow it to work as he helped me fix this for nUI, and it worked for a non nUI custom bar addon that I used for testing before applying the changes to nUI with Vruls help again and testing again. And that was for this specific quest chain.

Of course, if blizzard made changes to the bar since the expansion came out it may have effected it but no one has of yet reported an action bar problem with nUI.
__________________
  Reply With Quote

WoWInterface » Developer Discussions » General Authoring Discussion » SecureHandlerAttributeTemplate optimization

Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off