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

Compat updates for LFGBulletinBoard #235

Closed
juemrami opened this issue Aug 21, 2024 · 4 comments
Closed

Compat updates for LFGBulletinBoard #235

juemrami opened this issue Aug 21, 2024 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@juemrami
Copy link
Contributor

Describe the bug

  1. Issue from Minimap button never stays in one place Vysci/LFG-Bulletin-Board#301
  2. Compatibility for Cataclysm results in lua errors

Additional context

  1. The DF.Compatibility:LFGBulletinBoard runs newOnUpdate(btn) during addon load, when the user is not dragging the minimap button, and this causes its saved position value to be updated ever addon reload.

  2. Lines here check for the existence of our addon (on any client) and tries to run the compatibility patch, however in Compatibility.lua the patch is only defined for Era clients. Results in following error
    WowClassicT_w7d1UFOWbX

Versions:
Era/Cata Clients.

Notes:
In Vysci/LFG-Bulletin-Board#302. Ive opted any user with LibDBIcon-1.0 installed into using it to manage their button. You guy bundle it, so any overlapping users should be able to take advantage of that and the button will just play nicely.

Where it doesn't play nicely is when a user opts out using LibDBIcon so that they can free offset the button the minimap.

As a workaround, ive change the name of our minimap button frame so that your patch wont run, but that leaves an unskinned button whenever our users arent using the LibDBIcon compatibility. (see image)
WowClassicT_87n5MYJRsg

@juemrami
Copy link
Contributor Author

juemrami commented Aug 21, 2024

Our changes arent on the release version yet.
Atm they're just out on a beta release, for the user that reported the issue.

So no rush on your end.

@Karl-HeinzSchneider
Copy link
Owner

Hey,
thanks for the very well written issue, and PR! I'll include your fix in the next update.

What is your reason to not just use LibDBIcon? Felt a bit odd when browsing your code.

My compatibility is kind of a hack, yeah. Not using it on Cata(/Wrath) was an oversight, but somehow nobody reported any error.
I hope it was not too much work to investigate on your end 😅

LFGBulletinBoard is definitly one of the addons I recommend everyone for Era/SoD!

@juemrami
Copy link
Contributor Author

juemrami commented Aug 22, 2024

What is your reason to not just use LibDBIcon? Felt a bit odd when browsing your code.

There really is none, i just came into the codebase about 4 months ago, i also had alot of questions lol. We will move to LibDBIcon at some point.
The biggest concern for me is that the repo owner doesnt use bigwigs packager whenever hes drops a release, and im worried about adding external libs because he might forget to include them at some point since hes still manually packaging.

All that "LibGPI" in our codebase stuff is inherited from the OLD version of the addon (2019 classic release). I guess the original author was against bundling these external libs in his addon so he made his own half-baked "libraries" (the minimap stuff seems to mostly be copy paste from LibDBIcon).

@Karl-HeinzSchneider
Copy link
Owner

I also started without really any Lib, but some are really useful, especially LibDBIcon seems great cause it makes it easy to style all other addon buttons at once 😅

Manual packaging is my worst nightmare, the bigwigs packager makes it so easy, setup is like 15min if you only need the basic stuff. 1-2 times I had to make a small hotfix, when my packager wasn't set up for that and I had already merged something into main.. manuall file editing and repackaging, checked everything 3-4 times and still something went wrong lol.
You should definitly push for adding the auto packager!

I merged your PR into the next version, which I will release later today.
Style should work, only the position would be not perfect till your addon got the update with LibDBIcon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development

No branches or pull requests

2 participants