Klixxx / ElvUI_AdiBags

This is a modified version of AdiBags to make it look like ElvUI again!
17 stars 17 forks source link

Dragonflight Update #42

Open l3uGsY opened 1 year ago

l3uGsY commented 1 year ago

Hoping this addon will be updated for Dragonflight.

sepdotgg commented 1 year ago

Looking forward to this being done since Masque support is still in dev in Adibags. 😢

Added to BountySource for some traction on this. Would love to see an update before DF launch.

Bauxite commented 1 year ago

There is a more recent fork. It has also not been updated for DF yet, but is more likely to see an update since the author is still active: https://github.com/Fiffers/ElvUI_AdiBags_Wrath. I have made changes locally to this fork and gotten it working 99%, but still working out a few kinks with button skinning.

Beet4 commented 1 year ago

There is a more recent fork. It has also not been updated for DF yet, but is more likely to see an update since the author is still active: https://github.com/Fiffers/ElvUI_AdiBags_Wrath. I have made changes locally to this fork and gotten it working 99%, but still working out a few kinks with button skinning.

Do you mind sharing it if you get it to work? :)

Bauxite commented 1 year ago

@Beet4 - this is untested on TBC and Wrath, but it should be functional again.

https://github.com/Bauxite/ElvUI_AdiBags/|

CC @Seputaes if you want to test this out as well

Edit: Broken again with 10.0.2, working on fixing it up.

Beet4 commented 1 year ago

@Bauxite I was albe to try your fork before 10.0.2 hit and it seemed to work very well. :)

I noticed that the maintainer of main AdiBags branch will be fixing it when he gets back from his work trip in a couple of days. Might be best to wait until the main brach is fixed and then apply the skinning from that?

Bauxite commented 1 year ago

@Beet4 - yeah, that's the plan, I'd prefer to keep things as core AdiBags + the skinning code, rather than diverging and maintaining a complete fork of AdiBags. There is a PR up which fixes AdiBags, but it's not merged yet.

Shirofune commented 1 year ago

AdiBags just released an stable version :)

Bauxite commented 1 year ago

My fork is updated based on 1.9.48. Keep in mind core AdiBags has not added support for the new reagent bag yet, so this fork doesn't support it either. I also removed the custom overlays, which were used for Azerite/corrupted equipment in BFA and conduits in SL. I'll add it back if there's anything interesting in DF to add custom button borders for.

https://github.com/Bauxite/ElvUI_AdiBags/

Beet4 commented 1 year ago

Awesome, thanks! Hoping the main AdiBags will get more fixes this week before the launch of DF. There are some nice fixes in this PR https://github.com/AdiAddons/AdiBags/pull/790 I hope will merge:

Bauxite commented 1 year ago

@Beet4 - definitely plan to keep up with the AdiBags releases over the next week. Hopefully by DF launch either Klix of Fyffor will pick back up their "official" releases, I wasn't planning on supporting this long-term, but I'm going to keep fixing locally as long as I need to because I can't go back to stock AdiBags visuals.

Long-long-term, Masque support is in the works for AdiBags, which might make this fork redundant anyhow.

Beet4 commented 1 year ago

@Bauxite awesome, thanks! Will keep your branch bookmarked for now until/if Klix gets back. Noticed you merged with 1.9.48 as well, thank you for that!

Out of curiosity - is it much work to merge the skinning with every new release?

Edit: Same here - would be really hard to go back to stock AdiBags (or any other bag addon/default bags for that matter) when using ElvUI, to much inconsistency between the two.

Bauxite commented 1 year ago

Out of curiosity - is it much work to merge the skinning with every new release?

Not particularly difficult, the skinning code itself hasn't been impacted much by the API changes, so it's just a matter of merging it back in. The tedious part is that it has to be done in a dozen plus files, and I can't do it automatically since there is some AdiBags code that is deliberately overridden.

Beet4 commented 1 year ago

Author of ElvUI WindTools just released this: https://www.curseforge.com/wow/addons/adibagselvuiskin

Bauxite commented 1 year ago

Author of ElvUI WindTools just released this: https://www.curseforge.com/wow/addons/adibagselvuiskin

I'm a bit sketched out by the deliberately obfuscated lua, and his excuse is really odd - the author is claiming that he's afraid the AdiBags devs would block his ability to change the visuals so he has to try to hide how he's doing it? Really strange because I don't think he's malicious, WindTools is a well-established addon and doesn't have the same issue with obfuscation.

There were several comments asking about the obfuscated lua on the Curse page as well, and he just disabled comments entirely. Very weird behavior all around.

Cidan commented 1 year ago

Hi everyone,

Core AdiBags developer here. I'd like to take these changes and incorporate them into AdiBags properly ASAP. I am unfamiliar with how ElvUI skinning works in general, so, I have a request.

Would someone send a PR to the AdiBags repo with ElvUI skinning support? A few things to note for anyone that wants to take this up:

The current AdiBags master branch is up to date and working with DragonFlight, so that's a good starting point.

Thanks :)

Beet4 commented 1 year ago

@Cidan OMG AMAZING I LOVE YOU. Hope someone can do a PR of this :) ping @Bauxite

Bauxite commented 1 year ago

@Cidan - I'll gladly look into this once the launch excitement dies down. The current approach in this fork is pretty naïve, there are just hardcoded if blocks that check for the presence of ElvUI (or KlixUI, since the original fork was done by Klix) and overrides the styling inline. I'd want to rewrite this entirely if it's going to be built-in to AdiBags and configurable. There are also a handful of other changes in this fork that you may or may not want to take into core AdiBags:

Cidan commented 1 year ago

@Bauxite Thank you! Since my last message, I've actually reached out and had a chat with one of the ElvUI authors. One of the paths he suggested we evaluate is integrating with AddOnSkins. This would allow for a more generalized approach to using skins and offer much greater flexibility in how skins are applied to AdiBags. Ideally, I'd like to support a wide amount of visual styles, while minimizing the material work that needs to go into AdiBags.

After toying around with it for a bit in-game, it does feel like AddOnSkins is the way to go here. Would you be willing to take a look at that as an option?

Thanks!

Cidan commented 1 year ago

@Bauxite Sorry, re: features. I'd like to evaluate those separately from the skinning discussion. I'm not against any of these, but they should be considered independently.

Beet4 commented 1 year ago

@Bauxite Thank you! Since my last message, I've actually reached out and had a chat with one of the ElvUI authors. One of the paths he suggested we evaluate is integrating with AddOnSkins. This would allow for a more generalized approach to using skins and offer much greater flexibility in how skins are applied to AdiBags. Ideally, I'd like to support a wide amount of visual styles, while minimizing the material work that needs to go into AdiBags.

After toying around with it for a bit in-game, it does feel like AddOnSkins is the way to go here. Would you be willing to take a look at that as an option?

Thanks!

Just a heads up: AddOnSkins did have support for AdiBags up until a few years ago. The author dropped support for it because of the complexity of skinning AdiBags externally.

Cidan commented 1 year ago

@Beet4 Correct -- a part of this change will be making AdiBags itself externally skin friendly. I was not suggesting simply implementing AddOnSkins on its own, but to modify AdiBags such that external methods for skinning the bag are possible.

The idea is to surface within AdiBags an API for skinning, and then allowing AddOnSkins (or other modules/plugins for AdiBags) to apply skins to AdiBags. I'm favoring something similar to what Baggins does, but I'm not dead set on it.

Beet4 commented 1 year ago

@Cidan I see, sounds good!