Skip to content

Add Ninja Burst consolidation features - #499

Open
Judduuk wants to merge 4 commits into
MKhayle:masterfrom
Judduuk:master
Open

Add Ninja Burst consolidation features#499
Judduuk wants to merge 4 commits into
MKhayle:masterfrom
Judduuk:master

Conversation

@Judduuk

@Judduuk Judduuk commented Apr 4, 2025

Copy link
Copy Markdown

As per Issue: [Request] Ninja - Burst Cooldowns | #496

I have made some changes to reduce some button bloat, mainly around the busy burst window that uses a lot of buttons, and therefore gives strain, mainly to controller players.

This PR solves 2 points:

Mug / Dokumori > Ten Chi Jin > Meisui > Tenri Jindo
Kassatsu > Trick Attack / Kunai's Bane > Assassinate / Dream Within a Dream

These combos have been made and tested in-game and look to work well, but more testing is appreciated, especially as this is my first PR request.

  1. Dokumori, Ten Chi Jin, plus the 3 step mudra combo, meisui, and tenri jindo are all now one button.
  2. Kassatsu will only be replaced by trick attack when kassatsu is off cooldown and shadow walker is active, this is more in line with the current meta, as we ideally want to setup shadow walker before using Kassatsu. This combo also replaces with Dream within a dream when both are off CD also.

Let me know if you require more info on this PR.

Thanks!

return NIN.TenChiJin;

// TCJ sequence
// TODO - Find a way to get the mudra steps for ten chi jin consolidated also.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dug into the Dalamud structure on this a while back, and the job gauge object in Dalamud lacks anything related to mudra steps. So I think a change to that job gauge in Dalamud would be needed, because afaik nothing else except the internal game state would track it (no buffs or anything like that do, afaik).

@vitharr137 vitharr137 Aug 9, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only way I could find to get some parse on the steps of TCJ was the differing actionIDs similar to Mudras. They may have changed with the new patch but these were the ones I found below that were working in 7.2:

        Ten = 2259,
        Ninjutsu = 2260,
        Chi = 2261,
        Jin = 2263,
        MudraTen = 18805,
        MudraChi = 18806,
        MudraJin = 18807,
        TCJFumaTen = 18873,
        TCJFumaChi = 18874,
        TCJFumaJin = 18875,
        TCJKatonTen = 18876,
        TCJRaitonChi = 18877,
        TCJHyotonJin = 18878,
        TCJHutonTen = 18879,
        TCJDotonChi = 18880,
        TCJSuitonJin = 18881,

They can be used to sequence the TCJ combo like so:

if (OriginalHook(NIN.Ten) == NIN.TCJFumaTen)
{
    return OriginalHook(NIN.Ten);
}

if (OriginalHook(NIN.Chi) == NIN.TCJRaitonChi)
{
    return OriginalHook(NIN.Chi);
}

if (OriginalHook(NIN.Jin) == NIN.TCJSuitonJin)
{
    return OriginalHook(NIN.Jin);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that was the way I was suggesting in another comment. The central problem is, what Jujitsu changes into only indicate the last symbol used, afaik (except maybe in the case of TCJ, based on the above snippet). When handling standard Mudras, if Ninjitsu is showing, sat, Raiton, that means the last symbol used was Chi, but it could have been Ten Chi or Jin Chi, and afaik there's no way to differentiate that off spell ID during normal usage (outside TCJ)

@vitharr137 vitharr137 Aug 9, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, TCJ is a special case for this, the previous used action falls out of the usability list, so it filters down on its own each step. A Ten cast will make the other two change their action IDs as it progresses through the possible actionIDs, and a Chi cast creates a different path through it, etc, all with their different action IDs progressing in unique chains. Still no answer on base mudras to chain them together, but TCJ gave that little work-around

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya, makes sense, during TJC each symbol casts the actual ninjutsu rather than just applying the symbol, so they'd need different spell IDs for that.

Comment on lines +324 to +340
internal class KassatsuTrickDream : CustomCombo
{
protected internal override CustomComboPreset Preset { get; } = CustomComboPreset.NinjaDreamyTrickAttack;
protected override uint Invoke(uint actionID, uint lastComboActionID, float comboTime, byte level)
{
if (actionID == NIN.Kassatsu)
{
if (!IsCooldownUsable(NIN.Kassatsu) && HasEffect(NIN.Buffs.ShadowWalker))
return OriginalHook(NIN.TrickAttack);
}

if (actionID == NIN.Kassatsu && !IsCooldownUsable(NIN.Kassatsu) && !IsCooldownUsable(NIN.TrickAttack))
return NIN.DreamWithinADream;

return actionID;
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd strongly recommend implementing this into the existing block for Kassatsu that handles the existing Kassatsu->Trick Attack combo, and making the new 'Dreamy" combo a sub-combo rather than conflicting combo with the existing Kassatsu->Trick Attack combo. Main reason is, the logic between the two is essentially already compatible, and multiple combo blocks using the same action ID are rather bug-prone, especially as logic is rewritten or expanded in the future.

Comment on lines +329 to +336
if (actionID == NIN.Kassatsu)
{
if (!IsCooldownUsable(NIN.Kassatsu) && HasEffect(NIN.Buffs.ShadowWalker))
return OriginalHook(NIN.TrickAttack);
}

if (actionID == NIN.Kassatsu && !IsCooldownUsable(NIN.Kassatsu) && !IsCooldownUsable(NIN.TrickAttack))
return NIN.DreamWithinADream;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're duplicating a lot of logic in these conditionals. Wouldn't this make more sense as:

Suggested change
if (actionID == NIN.Kassatsu)
{
if (!IsCooldownUsable(NIN.Kassatsu) && HasEffect(NIN.Buffs.ShadowWalker))
return OriginalHook(NIN.TrickAttack);
}
if (actionID == NIN.Kassatsu && !IsCooldownUsable(NIN.Kassatsu) && !IsCooldownUsable(NIN.TrickAttack))
return NIN.DreamWithinADream;
if (actionID == NIN.Kassatsu)
{
if (!IsCooldownUsable(NIN.Kassatsu)
{
if (HasEffect(NIN.Buffs.ShadowWalker))
return OriginalHook(NIN.TrickAttack);
if (!IsCooldownUsable(NIN.TrickAttack))
return NIN.DreamWithinADream;
}
}

Or even:

Suggested change
if (actionID == NIN.Kassatsu)
{
if (!IsCooldownUsable(NIN.Kassatsu) && HasEffect(NIN.Buffs.ShadowWalker))
return OriginalHook(NIN.TrickAttack);
}
if (actionID == NIN.Kassatsu && !IsCooldownUsable(NIN.Kassatsu) && !IsCooldownUsable(NIN.TrickAttack))
return NIN.DreamWithinADream;
if (actionID == NIN.Kassatsu && !IsCooldownUsable(NIN.Kassatsu)
{
if (HasEffect(NIN.Buffs.ShadowWalker))
return OriginalHook(NIN.TrickAttack);
if (!IsCooldownUsable(NIN.TrickAttack))
return NIN.DreamWithinADream;
}

I'd also recommend conditioning DreamWithinADream on it actually being off cooldown, though. As written, you won't ever be able to see the remaining cooldown on Kassatsu unless Trick Attack is available but you lack Shadow Walker. If you do the sequence as planned, it will show the remaining CD on Dream Within a Dream instead until Kassatsu actually comes off cooldown.


// TCJ sequence
// TODO - Find a way to get the mudra steps for ten chi jin consolidated also.
if (level >= NIN.Levels.TenChiJin && !IsCooldownUsable(NIN.TenChiJin) && HasEffect(NIN.Buffs.TenChiJin))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than duplicating the first two clauses of this conditional, why not merge this and the conditional above it, with the effect check then being an if/else inside it?

if (level >= NIN.Levels.TenChiJin && !IsCooldownUsable(NIN.TenChiJin)
{
    if (HasEffect(NIN.Buffs.TenChiJin))
    {
        float tcj = FindEffect(NIN.Buffs.TenChiJin).RemainingTime;
        if (tcj > 5)
            return OriginalHook(NIN.Ten);
        if (tcj > 4)
            return OriginalHook(NIN.Chi);
        if (tcj > 3)
            return OriginalHook(NIN.Jin);
    }
    else
        return NIN.TenChiJin;
}

Also, using remaining time on TenChiJin is going to be EXTREMELY brittle. Instead, I would recommend checking the OriginalHook() value for one of the Mudras. It will always be Shuriken for the first mudra, and then the 2nd and 3rd will be defined by whichever mudra you're looking at. Best bet, given the sequence desired, would be to look at Jin. So if OriginalHook() of Jin is Shuriken, no mudras have been cast, so return Ten. If Jin's OriginalHook is Hyoton, then one mudra has been cast, so return Chi (ie. Raiton). And then if Jin is showing Suiton, then return Jin as is. That ensures that a small delay doesn't completely screw over the combo and result in something like a Chi Jin Jin Bunny.

}
else {
// Tenri Jindo
return OriginalHook(NIN.TenChiJin);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be conditioned on having the Tenri Jindo Ready buff, not simply lacking Shadowalker or Meisui being on cooldown.

Comment on lines +1338 to +1343
[IconsCombo([NIN.Kassatsu, UTL.ArrowLeft, NIN.TrickAttack, UTL.ArrowLeft, NIN.DreamWithinADream])]
[SectionCombo("Ninjutsu features")]
[ConflictingCombos(NinjaKassatsuTrickFeature)]
[SecretCustomCombo]
[CustomComboInfo("Optimised Kassatsu Trick", "Replace Kassatsu With Trick only while Suiton is up and Kassatsu Is off cooldown. Also replaces with Dream within a dream if both on cooldown.", NIN.JobID)]
NinjaDreamyTrickAttack = 3093,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[IconsCombo([NIN.Kassatsu, UTL.ArrowLeft, NIN.TrickAttack, UTL.ArrowLeft, NIN.DreamWithinADream])]
[SectionCombo("Ninjutsu features")]
[ConflictingCombos(NinjaKassatsuTrickFeature)]
[SecretCustomCombo]
[CustomComboInfo("Optimised Kassatsu Trick", "Replace Kassatsu With Trick only while Suiton is up and Kassatsu Is off cooldown. Also replaces with Dream within a dream if both on cooldown.", NIN.JobID)]
NinjaDreamyTrickAttack = 3093,
[IconsCombo([NIN.Kassatsu, UTL.ArrowLeft, NIN.TrickAttack, UTL.ArrowLeft, NIN.DreamWithinADream])]
[SectionCombo("Ninjutsu features")]
[ParentCombo(NinjaKassatsuTrickFeature)]
[SecretCustomCombo]
[CustomComboInfo("Optimised Kassatsu Trick", "Replace Kassatsu with Trick only while Shadow Walker is up and Kassatsu is on cooldown. Also replaces Kassatsu with Dream Within a Dream if both it and Trick Attack are on cooldown.", NIN.JobID)]
NinjaDreamyTrickAttack = 3093,

Recommended rewording. Also, shifting to parented on the existing Kassatsu/Trick Attack combo, rather than conflicting, as I recommended in a prior comment.

Comment on lines +1358 to +1363
[IconsCombo([NIN.Dokumori, UTL.ArrowLeft, NIN.TenChiJin, UTL.ArrowLeft, NIN.Ten, UTL.ArrowLeft, NIN.Chi, UTL.ArrowLeft, NIN.Jin, UTL.ArrowLeft, NIN.Meisui])]
[SectionCombo("Ninjutsu features")]
[ExpandedCustomCombo]
[ConflictingCombos(NinjaTCJMeisuiFeature)]
[CustomComboInfo("2 Minute Cooldowns Consolidation", "Consolidate Dokumori, TenChiJin (The Move), its combo steps, and Meisui with eachother.", NIN.JobID)]
NinjaDokumoriConsolidationCombo = 3025,

@kaedys kaedys Apr 7, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, I think this could be a sub-combo of the existing NinjaTCJMeisuiFeature combo, since it also includes Meisui. However, it should also definitely be an Accessibility rather than Expanded type combo, given how much it is consolidating. I also recommended some rewording below:

Suggested change
[IconsCombo([NIN.Dokumori, UTL.ArrowLeft, NIN.TenChiJin, UTL.ArrowLeft, NIN.Ten, UTL.ArrowLeft, NIN.Chi, UTL.ArrowLeft, NIN.Jin, UTL.ArrowLeft, NIN.Meisui])]
[SectionCombo("Ninjutsu features")]
[ExpandedCustomCombo]
[ConflictingCombos(NinjaTCJMeisuiFeature)]
[CustomComboInfo("2 Minute Cooldowns Consolidation", "Consolidate Dokumori, TenChiJin (The Move), its combo steps, and Meisui with eachother.", NIN.JobID)]
NinjaDokumoriConsolidationCombo = 3025,
[IconsCombo([NIN.Dokumori, UTL.ArrowLeft, NIN.TenChiJin, UTL.ArrowLeft, NIN.Ten, UTL.ArrowLeft, NIN.Chi, UTL.ArrowLeft, NIN.Jin, UTL.ArrowLeft, NIN.Meisui])]
[SectionCombo("Ninjutsu features")]
[AccessibilityCustomCombo]
[ParentCombo(NinjaTCJMeisuiFeature)]
[CustomComboInfo("2 Minute Cooldowns Consolidation", "After Dokumori is used, replace it with Ten Chi Jin (the action), its mudra steps, Meisui, and Tenri Jindo in sequence.", NIN.JobID)]
NinjaDokumoriConsolidationCombo = 3025,

TenMudra = 18805, // No-cooldown version that only appears during a Mudra cast, after the first symbol
ChiMudra = 18806,
JinMudra = 18807;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

o.O Why was this line removed?

@Judduuk

Judduuk commented Apr 7, 2025

Copy link
Copy Markdown
Author

Thanks for the feedback @kaedys I’ll try get onto this when I get the chance! C sharp and these plugins are new to me but it’s getting there!

@MKhayle

MKhayle commented Apr 10, 2025

Copy link
Copy Markdown
Owner

Hello! I'll check this out once I'll have time to test it ingame. I'm currently bedridden and tired, sorry for the delay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants