Ace3 port

[brittyazel]

Here is a first-go at a working Ace3 port of Advanced Interface Options. I attempted to avoid touching parts of the code not necessary for the GUI nor did I add anything more than a simple 1:1 translation of the current addon to the new GUI framework.

The Ace3 framework libs were brought into a folder located at /libs/
Panels are split and defined independently in /gui/

Each panel was recreated as close to 1:1 as possible; however, the ordering of elements is not a perfect match to the current rendition. More refinement could easily be done to order options in a specific order, and we could explicitly group items together to create a more deliberate "columns" rather than the simple flow layout that is currently being used. This is most noticeable on the Floating Combat Text panel. Likewise, I did not thoroughly test for functionality across all 3 game clients, and it would be worthwhile to do so. Similarly, I made use of the "disabled" feature of Ace3-gui for elements not available for a certain client type, but "hidden" could be used instead of you want these options gone entirely rather than just disabled.

For the cVar Browser, I did not recreate this panel using the Ace3 framework. Instead, I used Ace3 to create a "stub" page, the container frame of this new stub panel is then provided to the cVar Browser as its new parent frame. This necessitated wrapping the bulk of the cVar browser code in a :PopulateCVarPanel() function that I could explicitly call once our stub panel is created and available. This code could be cleaned up, a lot, but I wanted to provide a working proof-of-concept first, with iteration to come in the future.

Lastly, there was "status text" GUI code present in the codebase, but there was no visible GUI panel for this present in the live version of the addon. As such, I did not create an Ace3 panel for these options. We could do so fairly trivially if these settings are worth exposing.

Future work: This addon could very cleanly make use of Ace3-addon (in lieu of Eve) for defined :OnInitialize() and :OnEnable() load targets, as well as for inheriting Ace3-event functionality. The same could be true for using Ace3-db to define and manage the addon database. However, as these changes are architecturally significant, I felt it better to reserve such changes for future discussion.

[brittyazel]

Added a quick few last minute aesthetic changes to have the embedded cVar browser match the Ace3 stylings. It looks effectively indistinguishable now.

Also, there may be a few deprecations with regard to WoW Classic. There were a few hooksecurefunc overrides for things like camera max distance that I don't believe I copied over. That said, with Classic's recent big UI code update, it may be unnecessary to have those overrides still.

[Stanzilla]

This looks pretty good already! There are some infra things that need to be fixed that I can handle like not putting the libs in the repo etc.

Small things I noticed:

• Mousewheel scroll does not work in Cvar browser
• In the chat section, "Disable Class Colors" is always greyed out

[brittyazel]

Hrm. I'm not sure about the mouse wheel scroll. That code is basically unchanged, all I did was adjust the load order a tad to accommodate injecting it into an Ace3 panel.

For the "Disable Class Colors" option, you had only exposed this setting in WoW Classic before, so I matched your logic, i.e:

disabled = function()
return not self.IsClassicEra()
end,

We could instead use "hidden" instead of "disabled" for the option to be completely absent for clients it doesn't belong to, I wasn't really sure which way to go tbh. It's an easy fix though.

[Stanzilla]

Hrm. I'm not sure about the mouse wheel scroll. That code is basically unchanged, all I did was adjust the load order a tad to accommodate injecting it into an Ace3 panel.

For the "Disable Class Colors" option, you had only exposed this setting in WoW Classic before, so I matched your logic, i.e:

disabled = function() return not self.IsClassicEra() end,

We could instead use "hidden" instead of "disabled" for the option to be completely absent for clients it doesn't belong to, I wasn't really sure which way to go tbh. It's an easy fix though.

Ah yes that makes sense re: class colors.

Hm okay the code looks straight forward for the browser but maybe something is fighting with the template there. Maybe there is a better way do scroll frames now anyway that we could look into, @Meorawr thoughts?

then we can get rid of the rest of semlar's widgets lib, I think! Otherwise could try just enabling mousescroll on that frame.

[brittyazel]

Yeah, I can try a few things. Question, as I'm not too familiar with PR reviews, how do I get your commit additions to my PR back to my own branch on my fork? I want to make sure we're working off the same revisions lol

EDIT: Oh nvm, I could just pull them in directly. Easy

[Stanzilla]

Yeah, I can try a few things. Question, as I'm not too familiar with PR reviews, how do I get your commit additions to my PR back to my own branch on my fork? I want to make sure we're working off the same revisions lol

EDIT: Oh nvm, I could just pull them in directly. Easy

Yep, I recommend pulling with rebase

[brittyazel]

I wonder if the scroll wheel not working as something to do with the new 11.0 mouse focus changes. It would make sense, as I didn't actually alter the logic at all (besides loading slight later in the load order).

And yeah, creating a new window like this using Ace3 widgets would probably make sense, I'm not sure exactly how to go about do that though, as I don't think any of provided widgets can easily provide a table layout like what you have listed. The rest of the panel, i.e. the edit box and the labels would be easily transferable to Ace3, definitely. That said, I've never mixed partial Ace3 widgets and partial custom widgets on the same panel before, hence why I just blanked the panel entirely before injecting your code.

Update: I just checked the current prod version on WoW Cataclysm Classic and mouse scrolling isn't working their either. Are you sure mouse scrolling worked before? If so, it seems like the deprecation happened before any of my changes.

[brittyazel]

Oh interesting, logging into WoW Classic, I do now see the status bar panel.

I can implement that panel, but I'm not sure exactly what to do with the getCustomVar and setCustomVar functions. I can put these two functions on the addon namespace and then we can invoke them from within our Ace3 config table.

[Stanzilla]

Oh interesting, logging into WoW Classic, I do now see the status bar panel.

I can implement that panel, but I'm not sure exactly what to do with the getCustomVar and setCustomVar functions. I can put these two functions on the addon namespace and then we can invoke them from within our Ace3 config table.

That fine, I think

[brittyazel]

Ok I implemented the Status Text panel. FYI, 3 of the frames that these settings reference have been removed in WoW Retail: PartyMemberFrame<1-5>, PlayerFrameAlternateManaBar, and MainMenuExpBar. I have added a check for these frames, and in the event the client can't see them I have those options set to be hidden.

Likewise, I had broken some of the functionality in the Floating Combat Text inadvertently, but I have restored it.

[brittyazel]

Ok I think I fixed any and all functionality that I unintentionally broke. Ready for a more thorough review from anyone willing to do so.

Wishlist items:

1. Port CVar Browser to Ace3 widgets and/or cleanup the existing code to more explicitly work inside the new Ace3 wrapper.
2. Implement mousewheel scrolling on the cVar Browser (appears to be non-functional even in current prod version).

[Stanzilla]

The modern way of doing a scrolling frame is using a ScrollBox instead of the ListFrame that the widgets currently use. Might be worth a try. https://warcraft.wiki.gg/wiki/Making_scrollable_frames

[brittyazel]

Yeah I stumbled on that page as well. I started giving it a go, but got pretty lost with regard to setting up the data engine, and how that would play with this particular information. It seems definitely doable, but it was a bigger task than I was hoping to get into atm.

Is that a blocker for merging in this PR? Perhaps we could port the CVar Browser as its own discrete task since the current iteration is working well enough currently.

[Stanzilla]

My current thinking is that it is not, since it's broken in the current release anyway. I'm going to merge this and let it sit in alpha for a bit to see if we get any reports about bugs.

Thank you so much for doing this!

I'm going to clean up the codebase a bit after the merge, also adding StyLua and formatting it properly.

[brittyazel]

Sounds good, happy to help!

Some quick notes on items you may want to take a look at:

1. In StatusTextConfigPanel.lua, statusTextVals makes references to PartyMemberFrame<#>, PlayerFrameAlternateManaBar, and MainMenuExpBar, which no longer exist on retail. I added checks in the GUI itself to look for these and hide the options if not found, but if this is functionality you do want exposed for Retail, we should find out what the new frames are.
2. In GeneralConfigPanel.lua, reverseCleanupBags and lootLeftmostBag have an issue where the "Get" function doesn't immediately reflect the new value after setting it with the "Set" function. I hacked around this by adding a delayed timer to trigger a GUI refresh, but this leads to issues of the checkbox having a delay and that could lead to user confusion.
3. Originally there was only a "IsClassic()" function, but with both Classic Era and Classic diverting in functionality, I created a second function to differentiate between Classic Era and Classic. Likewise, any logic that uses these conditionals should probably be vetted to make sure the limitations are still true for Classic Era, Classic, or both. Currently, any place that originally checked for IsClassic() now checks for IsClassicEra() or IsClassic() to preserve the existing funcitonality.

[brittyazel]

Oh, one last thing. In GeneralConfigPanel.lua, cameraDistanceMaxZoomFactor might need some custom hooks for WoW Classic Era. You originally had some hooksecurefunc overrides in there due to Classic Era overwriting the values on login, but I forgot to go back and re-implement them. I'm not sure if that's still a limitation of the Classic Era client, but if so we should definitely patch in that behavior again. Right now that setting just sets the CVar like all the other settings.