attickdoor / XIVComboPlugin

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

PCT + VPR + GNB #309

Closed Fenyn closed 3 months ago

Fenyn commented 3 months ago

Contains Enriath's changes in #304 as well as some cherry picked additions to PCT, VPR, and GNB.

Total classes modified: AST, BRD, DRG, GNB, MCH, PCT, RDM, SAM, SMN, VPR

For those who are wanting a temporary xivcombo fix while the main repo gets updated, just compile this locally and add the path to the .dll to your Dalamud (Dalamud Settings -> Experimental -> Dev Plugin Locations)

Hope this helps :)

Aywinn commented 3 months ago

Sorry, but how do i compile this project locally ? Even after creating an account i cannot see any pptions and simple importing your project with the browser Link doesnt work.

attickdoor commented 3 months ago

I've merged Enriath's PR. Please rebase off master and update this PR or close/open another.

Fenyn commented 3 months ago

I've merged Enriath's PR. Please rebase off master and update this PR or close/open another.

Cheers, I've rebased off of your updated master. Thanks!

attickdoor commented 3 months ago

Please add .idea/ to your local .gitignore.

attickdoor commented 3 months ago

I'm gonna have to unlock Viper and Picto and take a look at things. This is promising though.

Fenyn commented 3 months ago

Made some small edits to reattach the Steel Muse > Hammer Stamp combo and added the Muse IDs since they were missing. It feels pretty good after a few hours of playtesting now :)

RuinIv commented 3 months ago

Thanks for making these changes, especially for PCT, the way they did the motifs was driving me nuts. Though, I was just messing around with these changes in a few roulettes and wanted to provide some feedback:

Having the motif go into the muse means you can no longer keep track of the cooldowns of the muse until you repaint the motif. It might be a bit more friendly for the player to provide an alternative way to combo the actions that lets you still keep track of the cooldowns on the muse abilities.

  1. Hammer Motif -> Hammer Stamp when Hammer Ready, and leave Steel Muse separate so you can keep track of the cooldown
  2. Living Muse -> Mog of the Ages when either the Moogle or Madeen is drawn, and leave Creature Motif separate so you can keep track of the cooldown
  3. Landscape Motif -> Star Prism when Star Struck, and leave Starry Muse separate so you can keep track of the cooldown

The second suggestion would need to have Mog of the Ages added to the PCT consts, I'm unsure of how to find the int for that, otherwise if this feedback is well received I could open a PR on your fork to make these changes.

Thank you for your consideration in advance!

Fenyn commented 3 months ago

I think that definitely sounds reasonable as an alternate option. Would you add it as a new toggle that detaches the mentioned skills?

I'll add Mog to the constants. I find the IDs by using the SimpleTweaks addon and enabling 'Show ID' in the Tooltip settings. :)

attickdoor commented 3 months ago

Thinking about that one. I'm going to unlock Viper in a hot minute and check through everything, then this one is probably mergeable after.

attickdoor commented 3 months ago

Yeah, there's definitely an awkward question here - How quickly do you repaint a motif after using your last one? If you're holding it off, then you risk missing the ability come off CD. I can see this two-combo thingy working. I'll drop a suggestion for how I envision it.

attickdoor commented 3 months ago

I think those are all the comments required for cleaning it up. We're almost there folks.

skycoop commented 3 months ago

In playing with PCT, I think the there should be four toggles related to Motifs/Muses

Unfortunately, I think this would make the logic pretty gross since you'd need to have a branch for each motif and muse to handle the case where toggle 2/3/4 are on while 1 is off.

I do really like how this is shaping up though. Thanks @Fenyn!

attickdoor commented 3 months ago

I've also realized there's a decent argument against the subtractive pallet combo - the buff has no expiration time. This means that you can pop the buff, not consume it, and then rebuild to 100 gauge for a triple-subtractive burst. Now that I've set up my PCT bars, I can see that there's very little need for extra space, especially with a bit of Motif/Muse consolidation. I'm thinking the combination of these two is a good enough reason to axe the subtractive combo. Thoughts?

Fenyn commented 3 months ago

I've also realized there's a decent argument against the subtractive pallet combo - the buff has no expiration time. This means that you can pop the buff, not consume it, and then rebuild to 100 gauge for a triple-subtractive burst. Now that I've set up my PCT bars, I can see that there's very little need for extra space, especially with a bit of Motif/Muse consolidation. I'm thinking the combination of these two is a good enough reason to axe the subtractive combo. Thoughts?

Once you've used Subtractive Pallet you are locked out from using additive skills until you've consumed the Subtractive buff. There is no way, to my knowledge at least, to build the gauge once the buff has been used.

attickdoor commented 3 months ago

Me in charge of actually playing a job 😎 Sounds good.

attickdoor commented 3 months ago

I think that this qualifies as "good enough to merge". I've been doing some cleanup locally so anything more from this PR will go onto that pile.