ADDON_ACTION_BLOCKED on ChangeTradeSkill
BannZay opened this issue ยท 8 comments
How to reproduce:
- Open 1 of 2 professions you have
- Quickly swap to another
- Quickly swap back
Expected: profession view swapped back.
Actual: profession swap broken, error thrown:
2x [ADDON_ACTION_BLOCKED] AddOn 'Skillet-Classic' tried to call the protected function 'CastSpellByName()'.
[string "@!BugGrabber\BugGrabber.lua"]:480: in function <!BugGrabber\BugGrabber.lua:480>
[string "=[C]"]: in function CastSpellByName'
[string "@Skillet-Classic\Skillet-30400.nil.lua"]:1493: in function ChangeTradeSkill'
[string "@Skillet-Classic\Skillet-30400.nil.lua"]:1464: in function ?'
[string "@Questie\Libs\AceTimer-3.0\AceTimer-3.0-17.lua"]:55: in function <...ceQuestie\Libs\AceTimer-3.0\AceTimer-3.0.lua:50>
Skillet-Classic [1.53]
I believe the operative word here is "quickly". If the Blizzard API function CastSpellByName() fails there's not much Skillet-Classic can do. I already have code to enforce a .5 second delay between changes. If you wish to increase that delay, then change line 1497 in Skillet.lua.
I was able to reproduce this error if I changed quickly enough but IMO, there's no need to change that quickly as that's all you are doing is switching back and forth. I have been seeing some lag since WotLKC went live and that could be having an effect.
Unless this is happening often enough under normal circumstances I don't think there is a need to change the code.
Honestly I`ve no idea what is going on in the code. Perphaps it happens because addon tries to call "CastSpellByName" using AceTimer and whole "enforrce a .5 second delay" code is indeed a cause of opened by me issue. Just assumption, it is hard to say when I see this addon for the first time.
But addon threw an exception and become unresponsive in a first minute of its usage by me.
I didnt find alternative so I decided to give it a second chance. This is why I am here, this is why I am still using it.
P.s.: I can constantly reproduce the issue, with relatively "slow" clicks (~0.4 delay).
In common, I would try to use pcall (if it is a thing for modern wow) for api calls which throws errors unless I know why it fails.
Update: I removed delays feature from "Skillet:ChangeTradeSkill" so I am not able to reproduce the issue anymore.
But I discovered another issue: if you open profession from spellbook and then click on profession icon from skillet frame then it become broken without any errors.
Are you using Skillet-Classic on WotLK Classic, Classic Era, or Season of Mastery?
This code has not been changed in years so I suspect that something else is causing this issue. Please download Skillet-Classic from CurseForge, verify the MD5, and reinstall. If the issue still exists, please disable all other addons and test again.
Please allow a reasonable amount of time between profession changes. ~0.4 seconds between clicks is not relatively "slow" in my opinion. The real issue here is that the Blizzard API is failing when the calls to CastSpellByName are too close together. I have no control over the Blizzard API.
Just downloaded Skillet 1.53 for wrath from CurseForge.
Retested with all other addons disabled. Wotlk classic client, english locale.
All I said still valid.
Please allow a reasonable amount of time between profession changes. ~0.4 seconds between clicks is not relatively "slow" in my opinion. The real issue here is that the Blizzard API is failing when the calls to CastSpellByName are too close together. I have no control over the Blizzard API.
After I changed the code (again) I am able to click on them like rly crazy (~0.1 delay). Zero api fails. All works as it should (apart from another issue I mentioned before)
Sorry I am not going to make PR, I dont feel competent enough for this but I`ll post updated (just commented out some stuff) code here. Just in case if someone ever need it:
function Skillet:ChangeTradeSkill(tradeID, tradeName)
DA.DEBUG(0,"ChangeTradeSkill("..tostring(tradeID)..", "..tostring(tradeName)..")")
-- if not self.delayChange then
local spellID = tradeID
if tradeID == 2575 then spellID = 2656 end -- Ye old Mining vs. Smelting issue
local spell = self:GetTradeName(spellID)
DA.DEBUG(1,"tradeID= "..tostring(tradeID)..", tradeName= "..tostring(tradeName)..", Mining= "..tostring(Mining)..", Smelting= "..tostring(Smelting))
if Skillet.db.profile.ignore_change or isClassic then
if not self.db.realm.tradeSkills[self.currentPlayer][tradeID].count or self.db.realm.tradeSkills[self.currentPlayer][tradeID].count < 1 then
DA.DEBUG(1,"ChangeTradeSkill: executing ChangeTrade("..tostring(tradeID).."), count= "..tostring(self.db.realm.tradeSkills[self.currentPlayer][tradeID].count)..", tradeName= "..tostring(tradeName))
self.db.realm.tradeSkills[self.currentPlayer][tradeID].count = 1
self.closingTrade = true
self:SkilletClose()
StaticPopup_Show("SKILLET_IGNORE_CHANGE")
return
end
end
DA.DEBUG(1,"ChangeTradeSkill: executing CastSpellByName("..tostring(spell)..")")
self.processingSpell = spell
CastSpellByName(spell) -- trigger the whole rescan process via a TRADE_SKILL_SHOW or CRAFT_SHOW event
self.delayTrade = tradeID
self.delayName = tradeName
self.delayChange = true
-- self:ScheduleTimer("DelayChange", 0.5)
-- else
-- DA.DEBUG(1,"ChangeTradeSkill: waiting for callback")
-- self.delayNeeded = true
-- end
end