MKhayle / XIVComboExpanded

Plugin version of the icon replacement features in dalamud
GNU General Public License v3.0
78 stars 59 forks source link

[RDM] Another Weirder Suggestion: Option to make "Verproc into Jolt" Behave like in Vanilla version #132

Closed AkiraChisaka closed 2 years ago

AkiraChisaka commented 2 years ago

So currently, you usually have 3 spells for Black and White mana. image

After you finish 3 melee hits, it looks like this: image

And after you use Verholy, it looks like this: image

In Vanilla XIV Combo, the one by Attick, after melee hits it looks like this: image

Followed by this: image

I was wondering if we can have an option to switch to this Vanilla behaviour.

Because after finishing my melee hits, I often look at my hotbar to check whether I have a fire or stone proc. Since in Expanded the fire and stone gets blocked by flare and holy, I can't see them anymore.

Yeah, pretty weird issue. Obligatory Relevant XKCD: image

AkiraChisaka commented 2 years ago

The solution to fix this should be to add a condition at HERE and ALSO HERE:

internal class RedMageVerstoneVerfire : CustomCombo
    {
        protected internal override CustomComboPreset Preset { get; } = CustomComboPreset.RedMageVerprocCombo;

        protected override uint Invoke(uint actionID, uint lastComboMove, float comboTime, byte level)
        {
            if (actionID == RDM.Verstone)
            {
                var gauge = GetJobGauge<RDMGauge>();

                // HERE - make replacing into Scorch and Resolution conditional depending on a setting

                if (lastComboMove == RDM.Scorch && level >= RDM.Levels.Resolution)
                    return RDM.Resolution;

                if ((lastComboMove == RDM.Verflare || lastComboMove == RDM.Verholy) && level >= RDM.Levels.Scorch)
                    return RDM.Scorch;

                if (level >= RDM.Levels.Verholy && gauge.ManaStacks == 3)
                    return RDM.Verholy;

                if (level >= RDM.Levels.Verflare && gauge.ManaStacks == 3)
                    return RDM.Verflare;

                if (IsEnabled(CustomComboPreset.RedMageVerprocComboPlus))
                {
                    if (level >= RDM.Levels.Veraero && (HasEffect(RDM.Buffs.Dualcast) || HasEffect(RDM.Buffs.Acceleration) || HasEffect(RDM.Buffs.Swiftcast) || HasEffect(RDM.Buffs.LostChainspell)))
                        // Veraero3
                        return OriginalHook(RDM.Veraero);
                }

                if (IsEnabled(CustomComboPreset.RedMageVerprocOpenerFeatureStone))
                {
                    if (level >= RDM.Levels.Veraero && !InCombat() && !HasEffect(RDM.Buffs.VerstoneReady))
                        // Veraero3
                        return OriginalHook(RDM.Veraero);
                }

                if (HasEffect(RDM.Buffs.VerstoneReady))
                    return RDM.Verstone;

                // Jolt
                return OriginalHook(RDM.Jolt2);
            }

            if (actionID == RDM.Verfire)
            {
                var gauge = GetJobGauge<RDMGauge>();

                // ALSO HERE - make replacing into Scorch and Resolution conditional depending on a setting

                if (level >= RDM.Levels.Resolution && lastComboMove == RDM.Scorch)
                    return RDM.Resolution;

                if (level >= RDM.Levels.Scorch && (lastComboMove == RDM.Verflare || lastComboMove == RDM.Verholy))
                    return RDM.Scorch;

                if (level >= RDM.Levels.Verflare && gauge.ManaStacks == 3)
                    return RDM.Verflare;

                if (IsEnabled(CustomComboPreset.RedMageVerprocComboPlus))
                {
                    if (level >= RDM.Levels.Verthunder && (HasEffect(RDM.Buffs.Dualcast) || HasEffect(RDM.Buffs.Acceleration) || HasEffect(RDM.Buffs.Swiftcast) || HasEffect(RDM.Buffs.LostChainspell)))
                        // Verthunder3
                        return OriginalHook(RDM.Verthunder);
                }

                if (IsEnabled(CustomComboPreset.RedMageVerprocOpenerFeatureFire))
                {
                    if (level >= RDM.Levels.Verthunder && !InCombat() && !HasEffect(RDM.Buffs.VerfireReady))
                        // Verthunder3
                        return OriginalHook(RDM.Verthunder);
                }

                if (HasEffect(RDM.Buffs.VerfireReady))
                    return RDM.Verfire;

                // Jolt
                return OriginalHook(RDM.Jolt2);
            }

            return actionID;
        }
    }
Grammernatzi commented 2 years ago

Isn't this caused by the AoE combo setting? I figure Attick didn't update it for endwalker now having a RDM 'aoe' combo, which is a little odd, but, eh, he does what he does.

AkiraChisaka commented 2 years ago

Isn't this caused by the AoE combo setting? I figure Attick didn't update it for endwalker now having a RDM 'aoe' combo, which is a little odd, but, eh, he does what he does.

I checked, the Veraero II being replaced by Scorch and Resolution is caused by the "Red Mage AOE Combo" option. But the Jolt / Stone being replaced by Holy is still behaviour for "Verproc into Jolt".

But yeah, it's really just something that only "break my workflow" lol

daemitus commented 2 years ago

Do you typically use the flare>scorch>resolution on your melee button?

kaedys commented 2 years ago

Alternative: an option that causes both Verthunder and Veraero to auto-select the appropriate finisher, instead of being each their own (ie. if you have low white mana and lack a stone proc, both Verthunder and Veraero become Verholy). Basically extending the existing melee button functionality of auto-selection to the Veraero and Verthunder. Same for the two AoE buttons. Basically, a new option Verholy/Verflare Auto-Select Feature: Replace Veraero, Verthunder, Veraero II, and Verthunder II with Verholy or Verflare, whichever is more appropriate, at 3 mana stacks. (for Scorch/Resolution, the two AoE buttons already have the capstone feature built in, and there's an existing option for Veraero/Verthunder, so no changes needed there).

Might solve the OP's issue innately, since it negates the need to check one's bars after the melee combo for which finisher to use*. I know you already have this option on Verstone/Verfire, but this would put it on Verthunder/Veraero, where those spells otherwise would exist, which is, imo, a better solution (the Verstone/Verfire one seems to be based on the assumption that the Verproc and Verproc Plus options are being used, and thus Veraero and Verthunder aren't on your bar).

*Though, as an aside, ideally you should know that going in, since your goal is not having a proc on your low mana, so you get the guaranteed proc out of it. RDM has a ton of flex about when they cast their melee combo now, so ideally you should be sitting on your mana and continuously using your low mana proc (either into the same mana finisher, if you already have the opposite proc too, or into the opposite mana as a "teeter-totter"), until your low mana doesn't have a proc, and only then using your melee combo, guaranteeing an extra proc from that Verflare/Verholy. That's not really something this mod can fix, either; if you go into the melee combo with a proc on your low mana, you're either munching a proc or wasting a guaranteed proc regardless.

Unrelated: looking at the combo preset file, why are the two Verproc Opener features marked as conflicts? From the text, one replaces Verfire with Verthunder, the other Verstone with Veraero, when out of combo. Those don't appear to conflict.

AkiraChisaka commented 2 years ago

Do you typically use the flare>scorch>resolution on your melee button?

No, I personally just use my melee combos as a single button, then use Thunder 3 Button (as Flare), then Stone button twice (which is replaced by jolt, and also by Scorch and Resolution).

AkiraChisaka commented 2 years ago

It seems like the update 26f65b706c426020191583f68a7fd16cd226fdd8 basically already implemented everything I want now!

The only issue I have now, is that if you turn off "Verstone/Verfire Capstone Combo", then Stone and Fire will not be overwritten by Scorch and Resolution anymore...

As in, when you don't have a proc, it works as intended, since Stone and Fire gets overwritten by Jolt with "Verstone/Verfire Feature". However, when you have procs, Stone and Fire will be prioritized over Scorch and Resolution, which is a bit weird.

So, like this example here: image

Verfire got a proc, so it overwrites the Scorch and Resolution. Verstone don't got a proc so it's overwritten by Jolt, which get replaced by Scorch and Resolution.

This is what happens when only "Verstone/Verfire Feature" is on, and all the Capstone Combo options are off.

My propose would be to make "Verstone/Verfire Feature" enable Stone and Fire being overwritten by Scorch and Resolution even if they have a proc.

AkiraChisaka commented 2 years ago

Alternative: an option that causes both Verthunder and Veraero to auto-select the appropriate finisher, instead of being each their own (ie. if you have low white mana and lack a stone proc, both Verthunder and Veraero become Verholy). Basically extending the existing melee button functionality of auto-selection to the Veraero and Verthunder. Same for the two AoE buttons. Basically, a new option Verholy/Verflare Auto-Select Feature: Replace Veraero, Verthunder, Veraero II, and Verthunder II with Verholy or Verflare, whichever is more appropriate, at 3 mana stacks. (for Scorch/Resolution, the two AoE buttons already have the capstone feature built in, and there's an existing option for Veraero/Verthunder, so no changes needed there).

Might solve the OP's issue innately, since it negates the need to check one's bars after the melee combo for which finisher to use*. I know you already have this option on Verstone/Verfire, but this would put it on Verthunder/Veraero, where those spells otherwise would exist, which is, imo, a better solution (the Verstone/Verfire one seems to be based on the assumption that the Verproc and Verproc Plus options are being used, and thus Veraero and Verthunder aren't on your bar).

Thanks for the suggestion! But I don't think this is the feature I am looking for. Maybe submit a new issue for this feature? In the new update https://github.com/daemitus/XIVComboPlugin/commit/26f65b706c426020191583f68a7fd16cd226fdd8, we got a feature replacing the melee button with the whole combo, which is really nice!

*Though, as an aside, ideally you should know that going in, since your goal is not having a proc on your low mana, so you get the guaranteed proc out of it. RDM has a ton of flex about when they cast their melee combo now, so ideally you should be sitting on your mana and continuously using your low mana proc (either into the same mana finisher, if you already have the opposite proc too, or into the opposite mana as a "teeter-totter"), until your low mana doesn't have a proc, and only then using your melee combo, guaranteeing an extra proc from that Verflare/Verholy. That's not really something this mod can fix, either; if you go into the melee combo with a proc on your low mana, you're either munching a proc or wasting a guaranteed proc regardless.

True, to be honest I probably don't need this feature as much as I thought I would lol

Unrelated: looking at the combo preset file, why are the two Verproc Opener features marked as conflicts? From the text, one replaces Verfire with Verthunder, the other Verstone with Veraero, when out of combo. Those don't appear to conflict.

Yeah, this do feel like a weird thing. I think it was because those two features are originally designed with making RDM a one button Verglare mage?

As in, a single button that you spam that automatically splits out the correct spells.

daemitus commented 2 years ago

regarding the opener: if you leave it on all the time, in a raid situation, you open with a long cast. in the overworld you dgaf and want jolt. If you lock them both to long-casts.. well. I hope you like sitting there and waiting.

kaedys commented 2 years ago

Fair, but I don't see why that should be mandated. Players are perfectly able to select which of the two, or both, they want active.

Edit: anyway, off topic for this feature, sorry for the derail.

daemitus commented 2 years ago

ok. we're getting into TLDR territory now.

The ask is for an option to do: stone/fire to do holy/flare, but not scorch>resolution?

daemitus commented 2 years ago

Yeah, that seems like the ask. So since some spells already natively transform into flare/holy. those that dont are getting a "ManaStacks" feature to do so. Then the capstone combo for all of them is just scorch/resolution.

AkiraChisaka commented 2 years ago

Thanks Dae! Everything I wanted in now implemented!