Masque

Masque

7M Downloads

Bug: `nil` Index Error

SebastianMuskalla opened this issue ยท 19 comments

commented

Since Masque 8.1.2-Alpha, I'm getting the following error whenever a buff runs out.

Message: Interface\AddOns\Masque\Core\Regions\Normal.lua:50: attempt to index local 'Normal' (a nil value)
Time: Sun Apr  7 19:15:32 2019
Count: 3
Stack: Interface\AddOns\Masque\Core\Regions\Normal.lua:50: attempt to index local 'Normal' (a nil value)
[C]: ?
Interface\AddOns\Masque\Core\Regions\Normal.lua:50: in function <Interface\AddOns\Masque\Core\Regions\Normal.lua:45>
[C]: in function `SetNormalTexture'
Interface\AddOns\PitBull4\Modules\Aura\Controls.lua:220: in function `onDelete'
Interface\AddOns\PitBull4\Controls\Controls.lua:130: in function `Delete'
Interface\AddOns\PitBull4\Modules\Aura\Update.lua:650: in function <Interface\AddOns\PitBull4\Modules\Aura\Update.lua:582>
Interface\AddOns\PitBull4\Modules\Aura\Update.lua:758: in function `UpdateAuras'
Interface\AddOns\PitBull4\Modules\Aura\Aura.lua:77: in function `UpdateFrame'
Interface\AddOns\PitBull4\Modules\Aura\Update.lua:952: in function `OnUpdate'
Interface\AddOns\PitBull4\Modules\Aura\Aura.lua:21: in function <Interface\AddOns\PitBull4\Modules\Aura\Aura.lua:18>

Locals: 
commented

Yeah, I just found the problem. In versions prior to 8.1, Masque would set any layer it couldn't find to false. This was actually erroneous because for the Normal region, Masque will actually skip that region if it's set to false. And with the way skins work, there should almost always be a Normal region since that's where the majority of the artwork is.

It also turned out that it was causing some issues with other add-ons so I fixed it. Masque no longer sets missing layers to false, but add-ons can still force Masque to skip the Normal region by explicitly setting it to false in the Regions (ButtonData) table. Note that should only be done if absolutely necessary. Eg, in the case of PB4 (here):

		group:AddButton(control, {
			Icon = control.texture,
			Normal = false, -- Explicitly set this to false to disable it.
			Cooldown = control.cooldown,
			Border = control.border,
			-- Count = control.count_text,
			-- Duration = control.cooldown_text,
		})

Interestingly, this particular issue has brought to light other problems, one of which you touched on: That Masque will still be hooked after a button's been removed from a group. I'm debating on whether to use AceHook-3.0 so I can use its "unhook" capabilities or just throw another boolean into the button's table and having all hooks check for it before executing. (The latter would be the most efficient, I think) It should also check for nil on that texture before trying to call its methods, of course.

commented

While you're here: I'm not sure if you've read the change log for the alpha, but there's some significant changes. Most notably, when you add a button to a group, you'll want to specify the type of button (third parameter), because that tells Masque which regions to look for. You can also tell Masque to skip search for any missing regions (fourth parameter). Assuming I deal with the hook issue and using the relevant code above, you'd end up with something like:

		group:AddButton(control, {
			Icon = control.texture,
			Cooldown = control.cooldown,
			Border = control.border,
			-- Count = control.count_text,
			-- Duration = control.cooldown_text,
		}, "Aura", true) -- Note the additional parameters.

The third parameter, "Type", can be either "Aura" (buff), "Debuff" or "Enchant" (they use different default border textures). The fourth, Strict will force Masque to skip missing regions, if set to true.

commented

I've added an alpha that should fix both the error and the issues you're having, @nebularg.

commented

Thanks, I knew about Normal = false and saw the new type stuff, but, skipping the normal texture would make most skins look weird ๐Ÿ˜’ It's working now without any changes, but for some reason all of the included skins (Classic, Zoomed, etc) are broken. Don't have the issue with other addons or skins and they work in 8.0.1 ๐Ÿ˜•

Classic: Classic PitBull: PitBull

commented

Did this error not occur in the previous release? Very little has changed in how that particular piece of code is handled. If you haven't, can you install the latest release (8.0.1) and see if it occurs? It doesn't make any sense that it would occur now but not before.

commented

Thanks for the quick reply.

I reinstalled 8.0.1 and the error stopped occurring.

commented

Interesting. Did you discover when it occurs? I know it's because PB4 unsets the normal texture in its OnDelete() handler, but I need to be able to know how to reproduce the error so I can figure why it's doing it.

commented

It occurs everytime a buff is removed.

commented

Alright. I'll check it when I get home. Thanks for the report.

commented

Hello @StormFX, I was just looking into this. The aura widgets are recycled so my hacky way of disabling Masque from doing anything to the normal texture when a widget is released is doing widget.__MSQ_NormalTexture = nil then widget:SetNormalTexture(nil). In 8.0.1 that was fine, not so much in 8.1.2, which errors because Normal is nil and updating my code to use __MSQ_Normal it would just push the error further down.

What I want is to be able to do widget:SetNormalTexture(nil) without Masque trying to reapply the default skin. My first thought after looking at the new hook was to just set __MSQ_NormalSkin to { Hide = true }, but maybe you have a better idea ๐Ÿ˜

Edit: Or just unset the texture directly instead of using :SetNormalTexture lol, I'll look at it later tonight.

commented

Ah. That's probably because I put the :SetBlendMode() call inside the color block, so it's setting the blend mode for that particular texture to "BLEND" instead of "ADD", which shows the whole texture since it doesn't have an alpha channel. I'll fix it in the morning.

commented

The new alpha should fix the issues and make it so you don't have to unset anything.

commented

Let me install it and see what's going on.

Edit: Actually, I see what's going on. Masque is attempting to index a button that's been passed as a nil value. Previously, Masque would exit if the button wasn't a table. With the update, I added a more detailed check but also added a shortcut to bypass the check on subsequent calls. It appears that shortcut is doing more harm than good.

On your end, you should probably make sure that nil values aren't getting passed. Also, I added some new parameters for the AddButton() method to filter out unused regions. The third parameter, a string, tells Masque the type of button being passed. Eg, "Action", "Aura", "Debuff", "Enchant", "Item", etc. This limits the regions that are iterated to find any that are missing. The fourth parameter, a boolean, tells Masque to not look for missing regions (if passed as true).

I suppose I need to do a full write-up on the changes to save wear on my old fingers. :p

commented

I'm getting the same/similar error in nivBuffs

2x Masque\Core\Group-Group.lua:69: attempt to index local 'Button' (a nil value)
Masque\Core\Group-Group.lua:69: in function `AddButton'
nivBuffs\nivBuffs.lua:1356: in function `?'
nivBuffs\nivBuffs.lua:127: in function <nivBuffs\nivBuffs.lua:127>

anything I need to change?

commented

@Stanzilla : The latest alpha should get rid of the error, but you should still check to make sure the button isn't nil before passing it. :p

commented

Awesome, thanks!

commented

No problem. That seems to cover everything so far, so I'll close this.

Ref: 7f6602b

commented

@StormFX just here then, yeah? https://github.com/Stanzilla/nivBuffs/blob/37985f5f785023965eb033fe1fca1d3ec4066c4c/nivBuffs.lua#L275 I tested with checking that btn exists but that still errord.

commented

I've moved this in its own issue. #59