Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Frames inheriting SecureActionButtonTemplate no longer execute SecureActionButton_OnClick #268

Closed
brittyazel opened this issue Sep 8, 2022 · 8 comments
Labels
Feature Request Requests for features Mainline This issue affects the current live Retail client

Comments

@brittyazel
Copy link

brittyazel commented Sep 8, 2022

In Shadowlands and prior, code implementing a simple button such as below would execute the macro text (or action, spell, whatever) upon being left-clicked.

local macroBtn = CreateFrame("Button", "myMacroButton", UIParent, "SecureActionButtonTemplate")
macroBtn:SetAttribute("type1", "macro") -- left click causes macro
macroBtn:SetAttribute("macrotext1", "/raid zomg a left click!") -- text for macro on left click

In Dragonflight, the same code doesn't register any Lua errors; however, upon clicking the button does nothing.

Using hooksecurefunc on SecureActionButton_OnClick, I can verify that SecureActionButton_OnClick is called successfully when an built-in button is clicked, but is not executed when an addon button inheriting from SecureActionButtonTemplate is clicked. This effectively breaks addons that implement custom buttons to perform actions such as Neuron, etc.

@brittyazel brittyazel changed the title [Dragonflight] SecureActionButtonTemplate frames not executing SecureActionButton_OnClick [Dragonflight] Frames inheriting SecureActionButtonTemplate no longer execute SecureActionButton_OnClick Sep 8, 2022
@Nevcairiel
Copy link
Collaborator

Nevcairiel commented Sep 8, 2022

This works fine for me, although there is one limitation I have discovered. You need to make sure that RegisterForClicks matches the CVar ActionButtonUseKeyDown. If KeyDown is enabled, then you need RegisterForClicks("AnyDown"), if its not, you need "AnyUp" ... or just go with RegisterForClicks("AnyUp", "AnyDown") to catch both cases.

Its a bit unfortunate that its enforced on a global level now, but at least its easy enough worked around - and it gives you consistency in button press behavior, I suppose.

We could ask for a new attribute so that addons can individually control the on-down/on-up behavior of their buttons, or at least to make SecureActionButtons register for AnyDown/AnyUp by default, but this issue can be worked around for now, so maybe bigger fish to fry.

On second thought, you likely need AnyDown registered if you ever want to deal with empowered spells. So maybe this is the only way for now without complex solutions.

@Meorawr Meorawr added Feature Request Requests for features Mainline Beta labels Sep 8, 2022
@Meorawr Meorawr changed the title [Dragonflight] Frames inheriting SecureActionButtonTemplate no longer execute SecureActionButton_OnClick Frames inheriting SecureActionButtonTemplate no longer execute SecureActionButton_OnClick Sep 8, 2022
@Meorawr
Copy link
Collaborator

Meorawr commented Sep 8, 2022

I'll flag this as a feature request since it's in the blurry mess between an awkward undesirable behavior and "well if we had this it'd be nice...".

If adding an registerForClicks="AnyDown, AnyUp" attribute to SecureActionButtonTemplate would fix the issue cleanly then I think we could easily ask for this as a quick tweak. If an addon is broken by the change they can always change their click registrations to the exact thing they want.

@brittyazel
Copy link
Author

Hrm ok. I'll have to try this when I'm home from work. My addon allows users to set the on click behavior for buttons on an actionbar by actionbar basis, with the default being RegisterForClicks("AnyUp"). This change to the game potentially breaks this feature in my code, as I can't have this option be specific per actionbar as well as dependent on a global cvar. I may have to start mandating RegisterForClicks("AnyUp", "AnyDown") and take away the user selectable feature unless they return the behavior to how it was prior

@Nevcairiel
Copy link
Collaborator

If you create actual actionbars with normal abilities, you won't get around supporting both up and down in any case, because empowered evoker abilities definitely need it, as they start channeling on "down" and fire on "up", which is why I think its probably not the most terrible requirement.

@brittyazel
Copy link
Author

brittyazel commented Sep 8, 2022

Good point, I hadn't even thought about the evokers.

I tested, and you are correct, self:RegisterForClicks("AnyDown", "AnyUp") does allow the buttons to execute. That said, this is a somewhat messy solution (not just for the above mentioned reasons), but it makes click to drag the spells around slightly more difficult as no my abilities are executing on the downclick before the drag has started. The old way was far more convenient tbh.

@brittyazel brittyazel reopened this Sep 8, 2022
@TimothyLuke
Copy link

TimothyLuke commented Sep 8, 2022

Following on from this:

local mybutton = CreateFrame("Button", "MYBUTTON", nil, "SecureActionButtonTemplate,SecureHandlerBaseTemplate")
mybutton:SetAttribute("type", "macro")
mybutton:RegisterForClicks("AnyUp", "AnyDown")
mybutton:WrapScript(mybutton, "OnClick", [[
  print("Hello")
  self:SetAttribute('macrotext', "/say Im here")
  print("world")
]]

With the following test, the Hello and World appear but the /say line is never displayed via /click MYBUTTON

Update: thanks @Nevcairiel
/click MYBUTTON no longer defaults to LeftButton but needs to be specific: /click MYBUTTON LeftButton

@kemayo
Copy link
Collaborator

kemayo commented Sep 22, 2022

This hit me as well for SilverDragon, which is faking unit frames that do /target with macrotext. Having to register AnyDown does make them inevitably feel less like the unitframes they're pretending to be, which is unfortunate.

@Meorawr
Copy link
Collaborator

Meorawr commented Sep 23, 2022

I'm going to close this in favour of splitting things into two tickets to make what we're asking clearer.

  1. SecureActionButtonTemplate should register for up and down clicks by default #282
  2. "/click" doesn't pass expected "down" parameter to Button:Click() #283

@Meorawr Meorawr closed this as completed Sep 23, 2022
@Meorawr Meorawr added Mainline This issue affects the current live Retail client and removed Mainline Beta labels Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request Requests for features Mainline This issue affects the current live Retail client
Projects
None yet
Development

No branches or pull requests

5 participants