MKhayle / XIVComboExpanded

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

[SUPPORT] Dawntrail support #244

Closed MKhayle closed 3 months ago

MKhayle commented 3 months ago

Current status:

If a task is checked, it means the job should be okay at level 90 for the initial release

Adventurer

Ranged DPS

Tanks

Healers

Melees

Casters

PCT/VPR

DoL

actually not touching DoH for now

kaedys commented 3 months ago

I'm looking at doing the preliminary work on Pictomancer (spell IDs, buff IDs, levels, class file framework, etc), and set up some initial combos, especially to merge some abilities that really should have been merged in the first place (like Holy and Comet, since they're strictly exclusive and fill the exact same rotational role). I'm pulling the spell IDs and buff IDs off Garland Tools atm, because I don't have the Dalamud runtime for seeing the IDs in game, so that'll probably require some testing (also, I've never bothered to get the compilation setup locally, so I have no way to directly test the changes myself, even once Dalamud is ready for proper testing >.<).

Edit: oh, also, a number of the combos are going to be dependent on the Gauge structure being implemented, so I'll sketch out stubs for those in the mean time.

Do you have a target branch for the DT changes, or should I raise it as a PR to master? I'mma try to have the first pass on it done tomorrow.

MKhayle commented 3 months ago

There are no target branchs for now, PRs are open but I need to consider the limits of what I'll accept doing or not ; what that essentially means is that I am not aiming at becoming slothcombo2 and want to remain faithful to daemitus' original plugin

So for now I'm still thinking about what I'll add for VPR/PCT, and focus on fixing already supported jobs first, but any contribution will be welcome

MKhayle commented 3 months ago

Priorities for what I'll be working on over the next days by order are (based on my experience & knowledge of each role/job):

  1. Ranged DPS
  2. Tanks
  3. Healers
  4. Melees
  5. Casters
  6. Level 90-100 for aforementioned jobs
  7. PCT/VPR

PCT may get done earlier since I'm playing it for DT, but no guarantees

kaedys commented 3 months ago

For now, these are the combos I was planning to implement for Picto. The only really "questionable" ones are the two secret combos, which are in line with other avoid-overcapping-resources combos like Warriors automatically swapping in Fel Cleave when their gauge would overcap. Lemme know if you find any of them problematic:

    [CustomComboInfo("Holy Comet Combo", "Replace Holy in White with Comet in Black when usable.", PCT.JobID)]
    PictomancerHolyCometCombo = 4201,

    [SecretCustomCombo]
    [CustomComboInfo("Holy Autocast", "Replace Fire in Red, Fire II in Red, Blizzard in Cyan, Blizzard II in Cyan, and their combo chains, with Holy or Comet if the next cast would overcap the Paint Gauge.", PCT.JobID)]
    PictomancerHolyAutoCombo = 4202,

    [CustomComboInfo("Subtractive Single-Target Combo", "Replace Fire in Red and its combo chain with Blizzard in Cyan and its combo chain when Subtractive Palette is active.", PCT.JobID)]
    PictomancerSubtractiveSTCombo = 4203,

    [CustomComboInfo("Subtractive AoE Combo", "Replace Fire II in Red and its combo chain with Blizzard II in Cyan and its combo chain when Subtractive Palette is active.", PCT.JobID)]
    PictomancerSubtractiveAoECombo = 4204,

    [SecretCustomCombo]
    [CustomComboInfo("Subtractive Autocast", "Replace Fire in Red and Fire II in Red, and their combo chains, with Subtractive Palette if the next cast in the chain would overcap the Palette Gauge.", PCT.JobID)]
    PictomancerSubtractiveAutoCombo = 4205,

    [CustomComboInfo("Creature Muse/Motif Combo", "Replace Creature Motif (Pom Motif etc) with Living Muse (Pom Muse etc) when the Creature Canvas is painted.", PCT.JobID)]
    PictomancerCreatureMotifCombo = 4206,

    [CustomComboInfo("Creature Muse/Mog of the Ages Combo", "Also replace Creature Motif (Pom Motif etc) with Mog of the Ages and Retribution of the Madeen when they are usable.", PCT.JobID)]
    PictomancerCreatureMogCombo = 4207,

    [CustomComboInfo("Weapon Muse/Motif Combo", "Replace Hammer Motif with Striking Muse when the Weapon Canvas is painted.", PCT.JobID)]
    PictomancerWeaponMotifCombo = 4208,

    [ParentCombo(PictomancerWeaponMotifCombo)]
    [CustomComboInfo("Weapon Muse/Hammer Combo", "Also replace Hammer Motif with Hammer Brush and its combo chain when they are usable.", PCT.JobID)]
    PictomancerWeaponHammerCombo = 4209,

    [CustomComboInfo("Landscape Muse/Motif Combo", "Replace Starry Sky Motif with Starry Muse when the Landscape Canvas is painted.", PCT.JobID)]
    PictomancerLandscapeMotifCombo = 4210,

    [CustomComboInfo("Landscape Muse/Star Prism Combo", "Replace Starry Muse with Star Prism when it is usable.", PCT.JobID)]
    PictomancerLandscapePrismCombo = 4211,

Edit: Noodling on it further, the following might also be solid additions, which I could include if you think they sound good:

kaedys commented 3 months ago

Also, have started messing around with Viper, and here's what I'm thinking for some of theirs, modeled after the existing Reaper combos:

Notably, I'm trying to keep the oGCD abilities separate from the GCD ones (so weaving still takes intent, rather than just automatically happening on its own), and retain the choice aspect in the combo chain usage. I could see arguments for merging the ST and AoE combo chains into a single button that auto-selects, but Viper already feels like a fairly simply job in that respect, and I've never liked the idea of combo auto-selection swapping between different positionals. So with those combos, you'd have two single target combo buttons, two AoE combo buttons, a single oGCD button for weaving, and Reawaken as your burst sequence (still with oGCD weaves on that separate oGCD button, though). You'd still have a fair amount of button selection going on, since each step of the single target and AoE combos would still be selected between the two options by the player (exactly how it works baseline), and you'd still need to use the two separate combo buttons for the pair of twinblade attacks after Dreadwinder/Pit of Dread, it just means you don't have to allocate 3 slots for different flavors of oGCDs, and four extra slots for the Dreadwinder twinblades.

MKhayle commented 3 months ago

I am taking notes for all of this, thank you! My roadmap hasn't changed yet though for now

MKhayle commented 3 months ago

I have started working and testing in game, but some specific stuff (gauges, notably) from ClientStructs isn't updated yet.

Also, with the newly baked-in combo system, PCT & VPR (and to an extent, some other jobs potentially) may require a unique approach.

I have succesfully updated MCH so far (but haven't tested it above lvl 90 as I haven't been able to play) and will continue looking for what was broken by 7.0.

I'll be working on what I can and postpone what I can't immediately do, and I plan to push a repo update only once I can guarantee covering every job up to lvl 90.

MKhayle commented 3 months ago

for now I can only implement those for PCT because I need gauge-related stuff to be done upstream

[CustomComboInfo("Subtractive Single-Target Combo", "Replace Fire in Red and its combo chain with Blizzard in Cyan and its combo chain when Subtractive Palette is active.", PCT.JobID)] PictomancerSubtractiveSTCombo = 4203, [CustomComboInfo("Subtractive AoE Combo", "Replace Fire II in Red and its combo chain with Blizzard II in Cyan and its combo chain when Subtractive Palette is active.", PCT.JobID)] PictomancerSubtractiveAoECombo = 4204,

I updated the first post with what's currently being done locally, I'm not pushing anything before testing it as much as I can and ensuring at least lvl 90 features are available

kaedys commented 3 months ago

I've got the ones I listed implemented, with stubbed out (and commented out) conditionals for once the gauge itself is implemented, as well as all of the spell IDs and buff IDs I could find. Obvious untested as of yet, though. Feel free to yoink as much or as little as you like from it.

MKhayle commented 3 months ago

thanks! will be helpful, although I had already done some of it (and tested them)

Grammernatzi commented 3 months ago

My own changes to Viper were pretty conservative, if you want to look at those. I elected not to have it automatically pick things for you, but rather just simply move actions onto your main combos when you're meant to use them (like, 1/2 become Twinfang/blood or Hunter's/Swiftskin's Coil) because I found it more fun to do so while still saving a ton of hotbar space. These are the main ones I did for Viper: image

And it looks like this in action:

https://github.com/MKhayle/XIVComboExpanded/assets/869037/c8f1b51d-073c-46fe-95e2-277a37f7b7a5

Obviously you don't have to use the same solution, but I figured I'd at least show an example of what I thought worked.

MKhayle commented 3 months ago

I haven't even played the job yet so I am absolutely unaware of whatever is going on there but I'll take a look at it, thanks!

looking at the video made me absolutely confused because all of those icons look the same. i'm not ready

Grammernatzi commented 3 months ago

Yeah, it's a job that took me a while to properly understand as well because it throws so much at you. I worked my way up it one action at a time to help.

MKhayle commented 3 months ago

I made a dawntrail branch for now, I'll probably try leveling up things at some point while gauge stuff is solved in clientstructs/dalamud

MKhayle commented 3 months ago

sleep time, see you tomorrow, hope gauges will be updated

kaedys commented 3 months ago

Honestly, I wouldn't have even moved Twinblood/fang and Serpent Tail onto the main combos when active. The weaving of those two is pretty fine, imo, and merging them into the main combo feels like it simplifies things too much. Be like if Ricochet and Gauss Round were merged into Heat Blast for MCH, so you're just spamming one button for all 3 abilities. Granted, the line where combos are "too much" is pretty hazy, but imo the weaving is one of the few actual complexities of Viper, and it's not even that complex.

I just don't want to have three oGCD buttons that all do separate things. Either one button for all of the oGCDs, or make both Twinblood/fang also have all of the stuff that normally goes to Serpent Tail, so you only need Twinblood/fang on your bar. Basically, same idea as the existing combo for Ricochet and Gauss Round to both include each other, so you only really need one of the two on your bar.

Moving the Coil/Den onto the main combo is certainly an good one, though.

As for understanding Viper, I found that just playing it for a couple did a lot. While the Actions and Traits window is worthless for figuring out the rotation, imo, the ability ant highlights on the hotbar do a great job of guiding you through how it's played. I also found this flowchart/cheat sheet to be immensely helpful.

Grammernatzi commented 3 months ago

I don't know how single button spam is less simple than just moving the buttons but still requiring you to press them in order, but it works either way. They're both valid.

kaedys commented 3 months ago

Ya, that's why the second suggestion, where Serpent Tail was merged into Twinfang/blood, but Twinfang/blood otherwise work the same as they currently do. That preserves the 1-2 and 2-1 weaves after Den/Coil, but means you don't need three buttons allocated to different flavors of dynamically-enabled oGCD weaves.

That said, Viper does have a lot of stuff enabled or triggered by things that are neither visible gauge or buff (or traditional combo action), so ya, that's going to be interesting to code.

Grammernatzi commented 3 months ago

I mean, I'm confused, because the solution I gave still has those 1-2 weaves. It doesn't auto pick. Not saying it needs to be what you go with here, I'm just pointing out I didn't do that.

kaedys commented 3 months ago

The description you initially gave wasn't super clear, at least to me. It said that Twinfang/blood were merged into the combo buttons and Coil/Den, but didn't make it clear that, as I'm now assuming, each one is merged exclusively into one of the two (ie. something like Twinblood merges down to Hunter's Coil and Steel Fangs, and Twinfang to Swiftskin's Coil and Dread Fangs), so you still have to 1-2 it. I initially read that description as including the same auto-determination logic I'd proposed and also merging it into a single button, so instead of the Dread combo being, for example, 1 (Dread) -> 2 (Hunter) -> 4 (Twin1) -> 5 (Twin2) -> 3 (Swiftskin) -> 5 (Twin2) -> 4 (Twin1), like it is baseline, or 1 -> 2 -> 4 -> 4 -> 3 -> 4 -> 4, which was my first suggestion, it would just be 1 -> 2 -> 2/3 -> 2/3 -> 3 -> 2/3 -> 2/3, ie. hit Dread, hit Hunter/Swiftskin 3 times, then hit the other one 3 times. But it sounds like it would instead be 1 -> 2 -> 2 -> 3 -> 3 -> 3 -> 2. Which...tbh actually isn't that different.

HatedUser commented 3 months ago

Would like to see:

I'm not interested in seeing all the oGCDs being combined.

kaedys commented 3 months ago

Honestly, if you're going to combine Twinfang and Twinblood, I don't see why you draw the line between them and Serpent Tail. But for me, the key element is having at most 2 oGCD buttons. I have them on my mouse side buttons to better support weaving (key -> mouse -> key is quite a bit easier to do accurately and reliably in a short timespan than key -> other key -> third key), but I only really have two keys available for that, as others are already allocated for spells that are either shared across jobs, or fulfill similar roles across jobs. So 3 oGCD buttons is really quite annoying for me. I don't really care whether it's the Twins that get combined, Serpent Tail that gets merged into the Twins, or all of them onto a single oGCD button. As long as it's only at most 2 oGCD buttons, it'll solve my particular issue with them. However, I do think merging Serpent tail down into the Twins (anything that would be on Serpent is also on both of the Twin buttons) and keeping the Twins separate is a superior setup, imo.

Is merging Hunter's Coil and Swiftskin's Coil really a good idea? Those are both positional abilities, with different positionals from each other. There are distinct reasons to cast the pair in one order or the other, if you're already in one of the positions after Dreadwinder. I've never thought it a good idea to merge buttons with different positionals on them. If, for example, it's coded up to cast Swiftskin before Hunters, and you happen to be in the flank spot when you cast Dreadwinder, you're just throwing away all that extra potency because the combo used the rear one first.

HatedUser commented 3 months ago

I don't like Fang/Blood/Tail as 1 because they don't have a natural interaction. It would feel bad for me if they were combined. It would be like putting Onslaught on Upheaval (maybe people already do that?). Fang/Blood are used in the Dreadwinder/Pit of Dread combos. Tail is used with the Steel/Dread "combos".

"Is merging Hunter's Coil and Swiftskin's Coil really a good idea?" True North exists if you are out of position. I figure since Hunter's is 3rd Gen and Swiftskin's is 4th Gen, it would be combined so you do Hunter's then Swiftskin's. No idea if that is the optimal DPS option tho. Considering it's optional, if someone is the kind of person who would enjoy those two being combined, I'd assume you'd be prepared to use TN or already be on the Flank by the time you cast Dreadwinder. At least for me, it seems like a great option to have.

I base my setup entirely on feel. As an example, I never put Primal Rend on IR because I don't like the feel of a weaponskill and ability being combined.

MKhayle commented 3 months ago

jsyk, VPR probably won't have any combos on the initial release, but I'm okay discussing about it on a later date I'd like to fix what's already existing first and will release it, play the god damn game a bit myself, and then add missing combos

kaedys commented 3 months ago

I don't like Fang/Blood/Tail as 1 because they don't have a natural interaction. It would feel bad for me if they were combined. It would be like putting Onslaught on Upheaval (maybe people already do that?). Fang/Blood are used in the Dreadwinder/Pit of Dread combos. Tail is used with the Steel/Dread "combos".

Except that Onslaught and Upheaval serve very very different roles. The 3 oGCDs are all just different flavors of "various oGCDs pop up on this button when they are available". The Twins are somewhat decent with that, since they support the 1-2 or 2-1 interaction, depending on which ability they are following, but Serpent being separate from them just feels like 100% button bloat with zero purpose. The Twins are never usable when Serpent is, and vice versa. Mechanically, there's zilch for difference between having Serpent as a separate oGCD button, or having those same abilities pop up on the Twins instead, but from an ergonomic and hotbar space perspective, it's just frustrating bloat with no benefit, imo.

I'd also like to point out that combos are optional, so a combo "feeling bad" to you doesn't really mean it shouldn't be added. You can simply elect not to use it, especially if an alternative were available that did fulfill your ask.

Edit: to be clear, imo, combos aren't necessarily about combining related or interacting buttons. They are about combining buttons that serve the same role. The 1-2-3 combo sequence for most jobs is that way. Each ability in the combo serves the exact same role, really. Mechanically, there's little difference between 1-1-1-repeat and 1-2-3-repeat. Comet and Holy on Pictomancer is a great example. They're both instacast cleaves that cost the same paint resource and serve the exact same rotational role. Literally the only differences between them are the potency, the animation, and the GCD length. And not only that, but they are strictly mutually exclusive. Holy is never available if Comet is available, and vice versa. There's zero reason for them to be separate buttons on the bar. It adds no depth, no decision-making, not even an opportunity for mistake. It's just straight purposeless button bloat.

The Twins actually involve decision-making. You want to use them in a specific order, and that order changes depending on which ability they are following. But Serpent Tail isn't like that. The abilities on Serpent Tail fulfill the same mechanical role as the ones on Twins, without even the 1-2 sequencing. They're both effectively just generic fill-in-the-blank oGCD follow-ups. The only real difference is that Serpent is single-weaved and the Twins are always double-weaved in one of their two sequences. Usage of Serpent is effectively indistinguishable from using just one of the two Twin followups. That's why, imo, there's no reason for them to be different buttons. Any buttons the fulfill the same function, and don't add any actual depth to the rotation by being separate, should, imo, be combined.

"Is merging Hunter's Coil and Swiftskin's Coil really a good idea?" True North exists if you are out of position. I figure since Hunter's is 3rd Gen and Swiftskin's is 4th Gen, it would be combined so you do Hunter's then Swiftskin's. No idea if that is the optimal DPS option tho. Considering it's optional, if someone is the kind of person who would enjoy those two being combined, I'd assume you'd be prepared to use TN or already be on the Flank by the time you cast Dreadwinder. At least for me, it seems like a great option to have.

Yes, True North exists, but that only works if you know which one is coming. Granted, the combo to combine them would likely always use a specific one of the two first, but it seems an ill idea to lock yourself into always having to do flank-the-rear for that sequence, or using True North. That said, I'm objecting to this one more on ideological grounds. It wouldn't be the first time this mod combined buttons like that, iirc. It just seems a bad idea to me to lock yourself into only having one of the two positional sequences available just to save a single button. I don't think combining the two really goes beyond the mark of what the mod is intended to do, though, even if I think combining the two in a fixed sequence is tactically unwise.

MKhayle commented 3 months ago

I am actually hesitating whether I should just delete BLM from this or not but I guess I'll just nap and come back refreshed enough to read tooltips

HatedUser commented 3 months ago

Yes, True North exists, but that only works if you know which one is coming. Granted, the combo to combine them would likely always use a specific one of the two first, but it seems an ill idea to lock yourself into always having to do flank-the-rear for that sequence, or using True North. That said, I'm objecting to this one more on ideological grounds. It wouldn't be the first time this mod combined buttons like that, iirc. It just seems a bad idea to me to lock yourself into only having one of the two positional sequences available just to save a single button. I don't think combining the two really goes beyond the mark of what the mod is intended to do, though, even if I think combining the two in a fixed sequence is tactically unwise.

Dragoon already behaves similarly and popping TN after Dreadwinder makes decision making irrelevant. IMO, it's an obvious optional option. It's only a bad idea if you don't like rigidity. My way makes it so you never really have to think about it. If flank is always first, you'll know to be on the flank first. It will become muscle memory which I value over the freedom to choose flank or rear first.

Still think the oGCDs being one button is bad, just like I think putting Generation and Legacy on reawaken is bad. It should just be 1-4 Generation

kaedys commented 3 months ago

Dreadwinder is on a 40s CD, while TN is 45s. You won't have TN available for every Dreadwinder, unless you intentionally let them drift. And DRG working that way is actually no longer the case in DT anyway, since the 5th combo hit is no longer a positional. Still, while I think it tactically unwise, it's not out of the realm of what this mod does. Nor is compressing the oGCD buttons, for that matter. Both of them are, imo, valid combos for this mod to have. Not everyone has to agree with or want to use a combo for it to be worth adding.

kaedys commented 3 months ago

@MKhayle RE the BLM changes, versus the changes you added for it:

MKhayle commented 3 months ago

yeah, I went a bit speedy because of the public dalamud beta being released taking care of that now, and it'll be pushed later on the repo

MKhayle commented 3 months ago

I'll just deal with the removals tho, so I'll change Fire 2 Feature later on

kaedys commented 3 months ago

The change for that one is actually going to be super easy, because it's a much simpler conditional now. If in Astral Fire, use Flare. And then later a conditional for Flare Star if at 6 Astral Soul, but that'll require a gauge update.

But for now, you can just change the BlackFire2 class bottom conditional from this:

            if (level >= BLM.Levels.Flare && gauge.InAstralFire)
            {
                // Lv 50 rotation without Umbral Hearts
                if (LocalPlayer?.CurrentMp < BLM.MpCosts.Fire2 + BLM.MpCosts.Flare)
                    return BLM.Flare;

                // Standard AoE rotation Fire2 until 1 Umbral Heart, followed by 2 Flare
                if (gauge.UmbralHearts == 1 || (gauge.UmbralHearts == 0 && HasEffect(BLM.Buffs.EnhancedFlare)))
                    return BLM.Flare;

                if (IsEnabled(CustomComboPreset.BlackFire2TriplecastOption))
                {
                    int triplecasts = FindEffect(BLM.Buffs.Triplecast)?.StackCount ?? 0;

                    // (Umbral Ice) Fire2 -> Triplecast -> Fire2 -> Swiftcast -> Flare -> Flare -> Manafont -> Flare
                    if (gauge.UmbralHearts > 0 && triplecasts == 2)
                        return BLM.Flare;

                    if (triplecasts == 1)
                        return BLM.Flare;
                }
            }

To this:

            if (level >= BLM.Levels.Flare && gauge.InAstralFire)
            {
                return BLM.Flare;
            }

Yes, it's that simple now 🤣.

MKhayle commented 3 months ago

I think I'll keep the (Between the) Ley Lines combo in case people with muscle memory want to just use Retrace instead of Between the Lines as the new button (the baked-in combo can be disabled), so that's a good option to keep imo

kaedys commented 3 months ago

Actually, ya, that's a good call, since the game does baseline support removing that auto-conversion.