Skillet-Classic

Skillet-Classic

445k Downloads

ADDON_ACTION_BLOCKED on ChangeTradeSkill

BannZay opened this issue ยท 8 comments

commented

How to reproduce:

  1. Open 1 of 2 professions you have
  2. Quickly swap to another
  3. 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]

commented

I am having Engineering and Smelting

commented

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.

commented

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).

commented

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.

commented

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.

commented

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.

commented

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)

commented

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