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

Commit „Fixed tbc missing spell error“ breaks the lib on classic #75

Open
Nils89 opened this issue Apr 12, 2021 · 18 comments
Open

Commit „Fixed tbc missing spell error“ breaks the lib on classic #75

Nils89 opened this issue Apr 12, 2021 · 18 comments

Comments

@Nils89
Copy link

Nils89 commented Apr 12, 2021

With the commit "Fixed tbc missing spell error" the lib always returns nil for durations

@d87
Copy link
Member

d87 commented Apr 12, 2021

Classic what? BC or Vanilla? If BC then the whole lib is disabled there, because you don't need it. And if on vanilla then i don't see how 1.64...master any of these changes could break anything
And I don't want to resub just to find out for sure

@Nils89
Copy link
Author

Nils89 commented Apr 13, 2021

I mean vanilla.
Strange is that I have embedded version 64 and Weak Auras has now with there latest update 66. And I do not get any duration back. Only if I raise the 64 to 67 anything is working fine

@mrbuds
Copy link
Contributor

mrbuds commented Apr 13, 2021

I can assist if something needs to be fixed but so far no one gave a good step by step example to reproduce their issue, and the lib is working perfectly fine in weakauras using version 66 on classic

@Nils89
Copy link
Author

Nils89 commented Apr 13, 2021

Have some more people reported that issue?

Current state:
Gw2 uses version 64 which works fine and we got the timers.

But with the latest WA and version 66 we got no timers from the lib.

So in that case throbbing from WA is loaded (higher number)

@cont1nuity
Copy link

cont1nuity commented Apr 13, 2021

And I don't want to resub just to find out for sure
Any way I could support the investigation?

From my tests:

  • LCD v64 Addon (loading first)
  • Plater loading with v64 embed
  • WeakAuras loading with v66 embed
    -> No aura durations in Plater.
    Disabling the stand-alone addon fixes the issue somehow...

Using the following atm:

local LCD = LibStub:GetLibrary("LibClassicDurations", true)
if LCD then
	LCD:Register(Plater)
	LCD.RegisterCallback(Plater, "UNIT_BUFF", function(event, unit)end)
	UnitAura = LCD.UnitAuraWithBuffs
end

@Nils89
Copy link
Author

Nils89 commented Apr 13, 2021

Same for gw2

@d87
Copy link
Member

d87 commented Apr 13, 2021

jfc, I guess i'll just rollback to 64. Now that WOW_PROJECT_BURNING_CRUSADE_CLASSIC is here lib is going to be disabled in BC regardless of the changes i made at the start of beta
EDIT: Done

@mrbuds
Copy link
Contributor

mrbuds commented Apr 13, 2021

Thanks for the rollback but we will need an increase of the minor version to be > 66 to make sure this is the loaded version of the lib, then we can publish a new release of weakauras

@cont1nuity
Copy link

Increasing the minor version actually triggers the error.
I copied v64 to WA libs and did a version increase there -> issue is back.

@d87
Copy link
Member

d87 commented Apr 13, 2021

Nothing was broken in 66, it was just disabling itself in BC.
The issue most likely stems from reference to library and its functions being obtained during load time by GW2/Plater, instead of PLAYER_LOGIN when the highest version has been loaded overwriting the embedded ones. And these functions maybe had some closures to locals in them, idk.
But if it's such an issue then how come it never came up before after updates.

@Nils89
Copy link
Author

Nils89 commented Apr 13, 2021

Can you try version 70?

In gw2 we set the Version local to 70 and this works fine

@cont1nuity
Copy link

Can you try version 70?

In gw2 we set the Version local to 70 and this works fine

The number is not the issue. It is purely about local references to the LCD calls, e.g. UnitAuraWithBuffs. The old local references are no longer working when a newer version is loaded.

@cont1nuity
Copy link

cont1nuity commented Apr 13, 2021

Maybe something like this to keep the wrapper in tact could help?

if not lib.UnitAuraWrapper then
	function lib.UnitAuraWrapper(unit, ...)
		return lib.FillInDuration(unit, UnitAura(unit, ...))
	end
end

Edit: This seems to fix the issue, but requires a new version rollout on all affected addons.

@d87
Copy link
Member

d87 commented Apr 13, 2021

Short term solutions:
a) WA goes back to 64 and nothing gets overwritten anymore, all good
b) You initialize the lib after all addons are loaded

Long term i'll investigate this and especially if i'll be doing updates.
But the future of the lib is hazy. If blizzard are going to use single client for both classic BC and Vanilla, there's a chance they'll just lift the restriction and go with TBC UnitAura

@cont1nuity
Copy link

The following appears to work with the scenario as well:

if not lib.UnitAuraWithBuffs then
	function lib.UnitAuraWithBuffs(unit, index, filter)
		return lib.UnitAuraDirect(unit, index, filter)
	end
end

@cont1nuity
Copy link

Short term solutions:
a) WA goes back to 64 and nothing gets overwritten anymore, all good
b) You initialize the lib after all addons are loaded

I could live with this. Requires changes in all related addons, as far as I can tell: GW2_UI, Plater and WA.

Long term i'll investigate this and especially if i'll be doing updates.
But the future of the lib is hazy. If blizzard are going to use single client for both classic BC and Vanilla, there's a chance they'll just lift the restriction and go with TBC UnitAura

Yes, I agree... Makes motivation to fix things even lower.

@d87
Copy link
Member

d87 commented Apr 18, 2021

Made a proper fix, turns out the issue was only present when you had either standalone LCD or PallyPower installed.
Also initializing during load time should be fine now

@Xruptor
Copy link

Xruptor commented Apr 18, 2021

Thanks for keeping this addon updated! Both my addons xanDebuffTimers and xanBuffTimers utilize this wonderful library! Having it around is a godsend for the classic servers to track buff/debuff durations since it's not built in. Have been using this library since it was committed. Thanks a ton!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants