Sellorio / mega-macro

A World of Warcraft AddOn that replaces the built in macro functionality allowing more and bigger macros.
17 stars 13 forks source link

New feature: Native Macros #188

Closed aurelion314 closed 6 months ago

aurelion314 commented 1 year ago

New capability to use MegaMacro with the native Blizzard Macros. IE, instead of stubcode replacing blizzard macros, we write the actual macro code, taking only 5 characters at the start to indicate Macro Index. In the process of coding this, many small improvements have been added.

Summary of changes:

Default to Blizzard Click behaviour. -When macros are below 250 chars, they will automatically be saved as real macros, instead of the MegaMacro variant, allowing for unaltered macro behaviour. This ensures click behaviour works without issue. It also means macros are more easily recovered if MegaMacro breaks (at least for the last used char), since they are bound as regular macros. Longer macros will still bind as 'stubcode'. -This also allows for using Blizzard's Icon rendering. A new config tab contains an option to swap between MegaMacro rendering and Blizzard rendering.

Improved imports. Imports will no longer fail. Instead, excess macros move to a new 'Inactive' tab to be fixed.

Uninstall option. Removes MegaMacro alterations from macro code, allowing for painless uninstall. Of course, if spec/class macros have been added, only those on the current character are kept, since there isn't room for more. Accessed via a button in the config tab.

Combine Class and Class-Spec tabs There is almost never a need for class-spec when you have a large spec tab already. I merged them and converted the extra tab into the Inactive section.

Does not automatically use all macros MegaMacro will only create macros as needed, potentially leaving space for other addons to create new macros. This should resolve conflicts with addons like GSE.

Tab highlighting for Move feature When dragging a macro, relevant tabs will light up, indicating the macro can be dragged to them. This should help make it clear that macros can be moved between tabs.

Bug Fixes Fixed the highlight border for selected macro Fixed multi number modifier strings (stace:1/2/3) Fixed highlighting of "known" conditional Fixed fallback for missing texture and displayName

jquick commented 1 year ago

This is a great idea to consolidate the storage!

Loosing the longer macros would be a little sad. This is the main reason I use the addon personally. Would it be possible to link multiple Bliz macros together in this case? They would not quite work the same going back to the default but if you could clearly see the code and intent I think that would be good enough. They just could not be ran together anymore.

aurelion314 commented 1 year ago

Would it be possible to link multiple Bliz macros together in this case? They would not quite work the same going back to the default but if you could clearly see the code and intent I think that would be good enough. They just could not be ran together anymore.

I'm not quite sure what you mean by linking multiple macros together. To clarify, with this change, the full oversized macro can still be read and edited in MegaMacro, it just gets truncated when actually being used since it saves it to a blizzard native macro.

aurelion314 commented 7 months ago

Hey I came back to this and enhanced it significantly. I realized that by using native macros as described in the original PR, I could also make importing and uninstalling a lot easier, so I went ahead and did that.

I'd love if @durandal42 or @Sellorio or anyone else could try it out and see if it works. (feel free to save a copy of WTF, but I doubt it will be necessary). The core logic is working well for me but I'm sure it could use some polish here and there.

Here's a summary:

Improved imports. Imports will no longer fail. Instead, excess macros move to a new 'Inactive' tab to be fixed. Imports are far less destructive due additional changes below.

Uninstall option. Removes MegaMacro alterations from macro code, allowing for painless uninstall. Of course, if spec/class macros have been added, only those on the current character are kept, since there isn't room for more.

Default to use Blizzard Icon and Click behaviour. -When macros are below 250 chars, they will automatically be saved as real macros, instead of the MegaMacro variant, allowing for unaltered macro behaviour. This ensures click and icon behaviour works without issue. It also means macros are more easily recovered if MegaMacro breaks (at least for the last used char), since they are bound as regular macros. Longer macros will still bind as 'stubcode'. -You can return to using MegaMacro Icon rendering in the new config tab, but I found it to be a bit buggy and wasn't able to find an easy fix.

Combine Class and Class-Spec Tabs There is almost never a need for class-spec when you have a large spec tab already. I merged them and converted the extra tab into the Inactive section.

durandal42 commented 7 months ago

Well, this is a pleasant surprise! Every one of your improvements sounds like great news, and none of the changes seem to cost me anything I care about.

I've just installed the latest from https://github.com/aurelion314/mega-macro/tree/master and I'll report back if encounter any problems!

Sellorio commented 7 months ago

I like the feature list. I'd like to suggest one change: having the character count change colour based on the type of macro to be used. Also having #showtooltip added automatically for macros that don't have it would be good. Naturally factor that auto insert into which type of macro is used.

Also an option to always use mega macros system since the macro icon logic is still slightly improved over base. Especially stuff like how it takes the icon from the first spell if none match (good for healing macros where you have [help, nodead] to prevent self casting accidentally.

aurelion314 commented 7 months ago

@Sellorio Good ideas. I'll add those in a bit.

The option to use mega macro icon logic is already there in the new config tab. The naming might be confusing, and its probably not clear when its on or off. Maybe there should be a little note somewhere on the main tabs to show if it is using native icons or mega macro icons? Not sure.

Sellorio commented 7 months ago

Sounds guchi. I trust your judgement.

I'm only coming back to wow for cata classic in a few months so won't be able to test it out.

aurelion314 commented 7 months ago

How's this for adding color when it passes 250, and a hover over tooltip explaining the limits: image

Sellorio commented 7 months ago

Nitpicking here but can we have the inactive tab only appear if you have macros there?

Sellorio commented 7 months ago

I see that you can specify slashes for stuff like form and stance. Didn't realise that. While you're in the code, did you want to add syntax highlight support for that?

aurelion314 commented 7 months ago

I'll try to add the slashes. I took a quick look at the parsing logic and its not immediately clear how to do that. I'll have to dig in a bit later.

Dabeuliou commented 7 months ago

I'll try to add the slashes. I took a quick look at the parsing logic and its not immediately clear how to do that. I'll have to dig in a bit later.

About the syntax highlighting, it is broken with the "known" modifier, would it be possible to look into that too? :)

image

image

durandal42 commented 7 months ago

I have one Holy-paladin-specific macro (HolyLight/CrusaderStrike depending on target being help/harm) that keeps popping up on the action bars of my other characters, always in a location where there's supposed to be a spec-specific macro from a different class/spec (e.g. Frost DK's HungeringCold + PillarOfFrost + trinkets, or Arms Warrior's ColosussSmash + trinkets).

Edited to add: happened again with another Holy-paladin-specific macro (HolyShock). Weird that so far both times have been from the same class/spec.

aurelion314 commented 7 months ago

Ok thanks. I found a binding bug as well that might be related. Working on that still.

I fixed the parsing of slashes and 'known' keyword. Will add that once I figure out this bug.

aurelion314 commented 7 months ago

Okay! I fixed the import logic @durandal42. Hopefully it fixes the issue you were having too. Can you try it? It was sometimes making new macros when it didn't need to, leading to duplicates, which I think is why it didn't rebind your char macro, as it probably bound a duplicate instead.

I added logic to fix broken macros and alert us to what was wrong (simple print right now). If something odd happens, check the chat and let me know if it showed anything. I can hide these once we're confident there aren't any more issues.

Known keyword is fixed, though on my version it didn't look the same as Dabeuliou (Mine just caused parsing colors to be off, not entirely grey).

Also fixed stance/form multi number parsing and a tooltip error.

Let me know if it works!

durandal42 commented 7 months ago

It was sometimes making new macros when it didn't need to, leading to duplicates, which I think is why it didn't rebind your char macro, as it probably bound a duplicate instead.

I certainly can confirm this! Many copies of spec-specific macros now exist under my "Global" tab. I don't know if this had already happened before upgrading to your latest master version (just now).

durandal42 commented 7 months ago

I certainly can confirm this! Many copies of spec-specific macros now exist under my "Global" tab.

So far, every macro I've seen show up on the wrong character is one of these, which got duplicated into my Global macros tab. So far, the original(?) copies are still in the spec-specific tab.

durandal42 commented 7 months ago

(Good news: I haven't seen any taint issues!)

aurelion314 commented 7 months ago

Ah yeah, sorry for the clutter. The macro gods thank you for your service :)

Can you delete those and see if it happens again? I suspect (hope) it was the previous version so they shouldn't reappear.

durandal42 commented 7 months ago

Yeah, no worries, this is honestly giving me a very convenient checklist for alts I need to check for broken macros! As I confirm that binding the spec-specific version works properly, I'm deleting the global ones, and soon the tidying will be done.

aurelion314 commented 7 months ago

Ok great. Also did you know you can move macros by dragging them to other tabs!? I literally just found out about this after seeing a move method in the code and wondering what it was for. Should we make that more obvious? Its an awesome feature but rather hidden. Maybe a 'move' button between delete and exit?

durandal42 commented 7 months ago

I did know they can be moved, and I did a lot of that in the early days when everything was imported as either a global macro or a character-specific macro. Moving from (Character -> Class) or (Character -> Spec) did a lot of work for me. These days, I'm sufficiently used to the organization that I just make macros in the "right" place by default (and, as you observed earlier, that place is never in the "Character+Spec" tab).

I'll never complain about features becoming more discoverable, but in this case I'm not asking for it. :)

aurelion314 commented 7 months ago

Guess it was just me missing out heh. What do you think about a hint that shows only on the Inactive tab, since there is where moving macros is most relevant. Keeps the other tabs clean. Does this look OK, or is it too noisy? image

durandal42 commented 7 months ago

Looks good to me (that was dead space otherwise), but be warned: I'm a backend engineer, not a frontend engineer. ;)

How hard would it be to make other tabs glow while a macro is being dragged? Dragging a macro onto a bar is a common activity, and the glow would tip off the user that ... well, something. :)

sellorio2 commented 7 months ago

Is the currently selected macro border not working?

aurelion314 commented 7 months ago

Indeed the macro border is not working. That's probably been broken since the original Dragonflight changes. I did some digging and not sure why it isn't working. Its still a CheckButton and still calls setChecked(true) without error, but nothing shows up. I tried digging into wow's macro code but couldn't figure out what we are missing (its not part of the "MacroButtonMixin") For reference: (https://github.com/tomrus88/BlizzardInterfaceCode/blob/master/Interface/AddOns/Blizzard_MacroUI/Blizzard_MacroUI.xml)

Might try again later but need to stop for now.

aurelion314 commented 7 months ago

I figured out the selected macro highlight. Turned out to be something simple but took a while to find. Fixed the icons on the inactive tab (were all default texture before). Added the move hint text. durandal42's idea of having the tabs light up when you start to drag is great, and I'd love to do that instead of the text, but it didn't seem like a quick thing so I'm using the text hint for now. It only shows on the inactive tab so its not too distracting.

aurelion314 commented 6 months ago

@durandal42 I added tab highlighting on drag like you suggested. It's super slick. Removed the hint text.

Fixed another bug affecting deletions. I've gone through a number of my characters, creating/moving/deleting lots of macros and I think I've gotten most the bugs ironed out.

aurelion314 commented 6 months ago

@Sellorio, Is there anything we can do to get this into more hands? I understand that merging such a big change to the main branch should be done very carefully, so it would be nice to have more people try this branch to ensure it is safe to release. It has enough improvements that I think it's worth trying to move forward with it.

Sellorio commented 6 months ago

If you think it's ready I'm happy to release. The updates are pretty exciting and if I understand correctly don't disable/backtrack any of the features that Mega Macro has now.

There's no migration issues with updating to the newer version?

aurelion314 commented 6 months ago

Indeed it doesn't remove anything. I do have it defaulting to blizzard icon rendering though, with an option to toggle. Is that OK? I defaulted it this way because I was getting occasional errors with the live version. durandal mentioned seeing errors too.

Anyway if you're happy once I'm happy, let me just do some final testing tonight. I haven't had any issues all week, but I'll do some last checks to be sure!

aurelion314 commented 6 months ago

As for migration, it should just work. I didn't change how macros are saved besides adding the Inactive section. Plus, I've uninstalled/reinstalled a number of times and it seems robust. Nonetheless, it is critical enough that it still might be worth having another user test, but up to you. I'll test it again tonight anyway.

Sellorio commented 6 months ago

That sounds amazing. I'll arrange the release once you're satisfied :)

aurelion314 commented 6 months ago

Sorry I got caught up with some work items. I'll hopefully finish testing soon.

aurelion314 commented 6 months ago

Fixed a minor Id issue with merging Char-Spec macros when porting from an existing install. Fixed another minor issue where the very first install required a reload before char specific macros were draggable.

Also fixed static icons.. that Blizz just broke with today's update. Guess it was good I took a few days to finish testing >_>

Anyway I switched between this branch and live a bunch of times, and even found someone to install it from scratch. The bugs I found were not overly serious and I fixed them, so I think its ready! (Fingers crossed)

Sellorio commented 6 months ago

https://github.com/Sellorio/mega-macro/releases/tag/1.6.0

Good work guys! GitHub release is up and I'll update Curse right now.

Curse done too. Hopefully the world doesn't end. But hey, if it does, at least they can uninstall easily :P

aurelion314 commented 6 months ago

Already has over 500 download and no comments about losing macros, so that's good!

Glad this was able to be go live. Hopefully more people will be willing to install and keep the addon now.