Quantcast SecureHandlerAttributeTemplate optimization - WoWInterface
Thread Tools Display Modes
03-16-19, 10:48 AM   #1
Terenna
A Cliff Giant
AddOn Author - Click to view addons
Join Date: Jun 2016
Posts: 73
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,281
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 Cliff Giant
AddOn Author - Click to view addons
Join Date: Jun 2016
Posts: 73
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 Cliff Giant
AddOn Author - Click to view addons
Join Date: Jun 2016
Posts: 73
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
Yesterday, 10:25 PM   #5
siweia
A Flamescale Wyrmkin
 
siweia's Avatar
AddOn Author - Click to view addons
Join Date: Jul 2011
Posts: 101
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
Today, 06:21 AM   #6
Vrul
A Frostmaul Preserver
 
Vrul's Avatar
AddOn Author - Click to view addons
Join Date: Nov 2007
Posts: 297
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
Today, 11:41 AM   #7
Terenna
A Cliff Giant
AddOn Author - Click to view addons
Join Date: Jun 2016
Posts: 73
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 : Today at 11:47 AM.
  Reply With Quote
Today, 11:44 AM   #8
Terenna
A Cliff Giant
AddOn Author - Click to view addons
Join Date: Jun 2016
Posts: 73
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

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