Refactor Job Action System

[Tekner]

Summary

What this is

I was messing around with different changes/additions to the existing job action replacement logic and felt like there was probably a better way to organize it all. Then I saw the changes you made in the RPR class and it seemed to me like you were starting to experiment with organizing the data better, so I thought I'd submit what I came up with. It's a very heavy rewrite and I wouldn't blame you for pushing back heavily or refusing to implement it, but I had fun working on it either way.

Goals of this refactor

Make it easier to find and tweak both values (skill/buff IDs) and logic.

You already had the skill & buff IDs separated into data container classes, so all I had to do was then also move the skill logic per job into its associated job class.

Encapsulate the ubiquitous patterns in the code/structure to allow better abstraction and optimization (DRY.)

I identified and attempted to encapsulate & abstract the following patterns:

• All action replacements were conditional to flags being turned on
• Most action replacement code blocks only affected a single skill at a time
• All actions have a level requirement (at least 1, technically)
• Most of the action replacements were for combos which followed the same set of logic
  • The next most common action replacements were for skill "upgrades"
• Most actions we care about in replacement logic are conditional to
  • Having a buff
  • Job gauge status
  • Being part of a combo

You should be able to tell easily at a glance what a given custom combo preset flag does without comments/a description.

I created some sub-classes whose sole purpose is to organize the replacement logic by custom combo preset flags. This makes writing the logic much more abstracted and also much more readable (in my opinion.) I created convenience functions to make writing the most common types of replacement logic (as identified in the previous point) easier.

The GetIconDetour function sometimes has to go through all of the top level conditionals if it doesn't short-circuit early when realistically the plugin should follow a tree structure and always short circuit regardless of if a replacement occurs.

Since all replacement logic is now encapsulated within the pre-existing job classes and both the job classes and job action instances are mapped to their own dictionaries, no massive amount of conditional checking is necessary; a tree gets constructed on load, linking all jobs, flags, and actions together.

What this does specifically

Additions

Two new classes were added:

1. Job (with JobConfig & JobConfigData subclasses)
2. JobAction

Job is the new base class for all pre-existing data container classes for the jobs. It contains helper functions and logic to facilitate writing action replacement more easily.

JobAction is a data container class which mostly just encapsulates the action ID and level requirement. You can, however, also tack on a "condition" which is what allows you to simply list actions together in a combo without an explicit conditional in code there.

Things that changed

Action replacement logic

All action replacement logic moved from within the GetIconDetour to its respective job class. All logic is now contained within one of three functions:

1. ForAction
2. ForComboActions
3. UpgradeAction

How the new action ID gets passed around

1. The current class is gotten
2. The map to that class' flags is retrieved
3. Each flag that is enabled is sequentially checked if it maps to the current action
4. Once the action is found, the logic is ran and the tree traversal ends with the current action either being replaced or not.
5. If the action is not mapped, or the flag that maps it is disabled, then only part of the tree is traversed, no logic is ran, and it shortcuts early into the same iconHook.Original that it used to.

Many things got rescoped

• The action IDs & metadata are now scoped to their respective classes as opposed to being public static.
• This means subclassing the level and buff data is no longer necessary because it isn't accessed outside of its respective class.
• Because everything is now class-scoped, you can create helper functions to further simplify coding action replacement logic. E.g. Red Mage has a HasMana function since checking for a minimum amount of black & white mana is used a few times.

Action usability is split from how actions are grouped

• Combos are defined by grouping actions together, but the conditions for when certain skills in a combo are usable are separately defined. This both makes things more concise and readable, and has the added benefit of being able to handle job action upgrades more easily because you can simply set the upgraded action's condition to be whether its predecessor is usable, and then just chain them together in their respective combos.
• This also makes it so that if a given action is used in multiple combos, you only have to define its conditions once, both saving on code and also creating a single source of truth which makes updating/tweaking things much easier in the future.
• The minimum level requirement for actions are defined during JobAction construction and is always respected regardless of any other defined conditions, further simplifying coding and tweaking.

A brief rundown of how the program flows with this refactor

Initialization

1. The IconReplacer constructor is called and initializes everything as before.
2. The static Job.Initialize() function is called, passing in the clientState, Configuration, and iconHook variables.
3. Reflection is used to get all Job children with the JobAttribute as well as the passed in job abbreviations.
4. Each Job child is instantiated and mapped in a dictionary to its abbreviation, which is then returned back to IconReplacer and stored for later use.

Main logic

1. GetIconDetour is called and lastMove, comboTime, and level are derived as before with the addition of classAbbreviation.
2. The jobsMap dictionary returned by Job.Initialize() is checked against the current class' abbreviation and the corresponding class' ReplaceIcon function is called which will return true if the current action is mapped (and its flag is enabled.)
3. The icon is either replaced or not, and the program falls through to passing the current action back as before.

JobConfig

1. When a Job is instantiated, its constructor will contain calls to ForFlag which maps an internal dictionary entry for that custom combo preset flag with a JobConfigData instance.
2. The returned JobConfigData instance can then have a few functions called on it to set up action replacement logic. These functions are: ForAction, ForComboActions, and UpgradeAction
3. Each of these functions vary slightly in how, but they all then map one or more actions to a function which returns the new action ID, finishing up the tree data.

Job

1. Once the instance is constructed, each action that needs it has its usability condition set.
   • Additional variables like a class gauge can also be set up.
2. ForFlag, ForAction, ForComboActions, and UpgradeAction are then called to map Flags => Actions => Logic.
3. ForAction usually utilizes GetBestAvailableAction which simply returns the right-most action which is usable based on the action conditions set.
4. ForComboActions and UpgradeAction are just convenience wrappers of ForAction and GetBestAvailableAction.
   • ForComboActions automatically maps the logic to the last action listed (because the precedent for combo actions is the last action in the combo gets replaced.)
   • UpgradeAction automatically maps the logic to the first action listed (so that it reads like the first action will be "upgraded" with any of the actions that follow.)

Example

Given: A Job has a combo using actions A, B, and C and the level requirement for B is 10 and C is 20.

• You would define all three actions at the top of the Job class and set the level requirements for B and C.
• In the constructor, you would set the condition of B to LastMoveWasInCombo(A) and the condition of C to LastMoveWasInCombo(B). This translates as you would expect to the original comboTime > 0 && lastMove == B/C.
• After the relevant ForFlag call, you would then call ForComboActions(A, B, C).
  • ForAction is then called internally and returns GetBestAvailableAction([A, B, C]) while mapping to all three abilities separately in the internal JobConfigData.actionLogicMap dictionary.
  • GetBestAvailableAction then calls CanUse on its passed array of [A, B, C] to filter it down to only the currently-useable actions. This means if you are only level 1, B and C will be filtered out due to not meeting level requirements. Similarly, if the last move you used was A, then C will be filtered out.
  • Finally it returns the last (right-most) action remaining after the filter. This is why you should always pass actions in the order of most to least accessible to GetBestAvailableAction. This has the additional benefit of easily being able to pass in upgraded action variants right after the normal variant and they'll automatically be used instead as long as you set up their condition logic correctly (see Red Mage's melee combo.)

Notes

• If you have standard combo logic, but you want to map it to a different action than the last/first one (or want to map it to all/multiple actions), then you have to call ForAction yourself and cannot rely on the convenience functions. E.g. both of Bard's combos map to two abilities in the combo instead of just one like a standard tank 1,2,3 combo.
• You can chain the three action functions together on each flag to handle multiple actions' replacement logic.
• You cannot (should not) use the same action in multiple ForAction calls. Because this all works on tree (dictionary) traversal and not iteration, you can only map an action once.
  • Technically speaking, you could map the same action once to multiple flags and as long as only one of those flags is enabled at a time, it will work as expected, but since the config currently does not support mutually-exclusive flags, this is a bad idea. I'm not sure if providing variant action replacement logic meshes well with the ethos of this plugin. Could be a future feature potentially.

Possible improvements

1. Since everything is mapped to a tree, a further optimization could be made where when the plugin is loaded or the user changes the config settings, you selectively map classes and flags in the internal dictionaries only when they are enabled (in the case of classes when they have at least one flag enabled.) I don't know how much of a performance improvement this would yield since there isn't any high level iteration happening regardless, but it should theoretically improve it.
2. I'm not sure how/if this would improve anything, but now that all job actions are internally mapped, the accursed CheckIsIconReplaceableDetour could actually check to see if the actionID it's passed is mapped (and its flag is enabled.) I'm assuming this would affect how many skills are passed to GetIconDetour, so combining this with the first improvement might make a measurable difference on lower-end machines with certain setups.
3. Defining standard combos could potentially be further simplified where the only condition of actions is LastMoveWasInCombo. I didn't because I had to draw the line somewhere on how "convenient" to make everything before it just got ridiculous.

[dferrari23]

Hi. Do you guys have the ID table for all skills or let me know where I can find it?