DogTag support is done in such a way that will break other addons
ascott18 opened this issue ยท 7 comments
Hey, just an FYI that your implementation you added to shoehorn in support for nameplate units to LDT is going to break other addons. This is because you overwrote the metatable of IsLegitimateUnit with your own, erasing the one that is already declared by LDT-Unit.
I think its super great that you want to extend LDT with better support for nameplate units, and to use it in TP, but it would be better for all of us if you contributed this change directly to LDT. The repo is open for push by anyone - https://www.wowace.com/projects/libdogtag-unit-3-0/source
You are right. I was (far) too optimistic that this hack would not have any sideeffects. I will fix it and also only enable the LibDogTag part if custom status texts are enabled.
Creating a pull request for LibDogTag is also something I need to do, you are right. When the pull request is merged into LDT (hopefully) I can remove the other code.
Thanks for your feedback!
The sideffect should now be gone with 10.1.3. The patch for LDT-User is submitted, but not sure if or when it will be integrated.
The LDT repos are open and can be pushed to by anyone. That said, I'll go ahead and implement this myself. Nevermind, appears this feature was taken out of Curseforge after the overwolf transition. Nevermind, I was using the wrong password.
The current solution you implemented in d47bc24 is even more broken than the original implementation - the attempt to cache the old metadata of IsLegitimateUnit at
is incorrect and is just invoking the__index
metamethod of that function with "__index"
as the key. You'd have to call getmetatable
to get the current value of the __index
metamethod.
What's interesting is that I don't get this error, even with TMW enabled. So it's hard to verify that my bug fix is actually working.
Anyway, that error should only occur if you use custom status text (with LibDogTags) in Threat Plates. If you don't use them, there should not be any error. So it's easy to get rid of it.
Ok, so, after there is a now a version for LDT-Unit available that supports nameplates, I removed the workaround. Thanks for your help in fixing this issue.
Yeah, you'd have to have a TMW icon (or some other addon using LDT) that is checking an unorthodox unit that isn't statically declared in IsLegitimateUnit already in order to trigger this.
I didn't realize that Parnic had moved LDT over to Github when I made my original comments. Otherwise I would have just referred you there for a PR - sorry about that.