attickdoor / XIVComboPlugin

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

Updated (nearly) everything for Dawntrail #304

Closed Enriath closed 1 month ago

Enriath commented 1 month ago

(Oh gee, yet another PR for updating to 7.0?)

I wanted some more practise with C# tooling, and also was missing XIVCombo, so here's basically everything updated!

The only things I've not touched was Warrior's Inner Release (and given glen-lee's PR, it's a much more complicated issue), Black Mage's Leylines (similar to Warrior, it's a more complicated issue imo), and Gunbreaker (mostly because I don't have access to the class as a trial player, and it was confusing me too much to try and figure it out in abstract).

I've tested every change to make sure it works, either by myself, or with a high level friend.

I wasn't too sure about submitting after I saw just how many different PRs there were, but I felt like most of them covered only a few jobs each. I've also organised my commits such that it'll be a lot easier to pull in a part that another PR missed; I hope that's useful.

Alluneve commented 1 month ago

@Enriath

Suggestion: Instead of removing the play-functionality from AST, try to do it the other way around:

this can be done by default by returning the draw action, but in-case of the cards drawn balance/spear, arrow/bole or spire/ewer, returning the play1/2/3 action

Enriath commented 1 month ago

That's a really interesting idea @Alluneve, and I actually quite like it. I wasn't too sure initially, but after trying it out I feel it really keeps the original Draw/Play combo's idea. I wasn't planning on adding anything "new" for this, just fixing up old options so they work with the new additions, and while I think this is juuust a bit of a stretch I want to say this counts as fixing up old options :P

Thank you for the suggestion. I'll add it as its own commit, while keeping the existing AST: Remove Play commit (2681772ad6e8de556f04df9eab18b68bccfa7dbc), and attick can choose whether to keep or drop this new one.

attickdoor commented 1 month ago

These look pretty good, I think this one is likely to be merged. The AST draw/play change is kinda silly IMO, but I think this is probably the best that can be done given the circumstances.

attickdoor commented 1 month ago

While these additions are good, are there removals to be done as well? They don't have to be done in this PR but that's something I'm going to spend a hot minute looking into.

Enriath commented 1 month ago

These look pretty good, I think this one is likely to be merged. The AST draw/play change is kinda silly IMO, but I think this is probably the best that can be done given the circumstances.

Thank you! And I agree, the AST one is pretty silly, but after playing with it for a bit it does feel like the old swap. Could drop it and move that to its own PR if you'd like.

While these additions are good, are there removals to be done as well? They don't have to be done in this PR but that's something I'm going to spend a hot minute looking into.

So I had a look through every action again, in case I missed something, and here's everything I could find:

I would also like to note that I am unable to check Gunbreaker, Dancer, or Reaper, as I do not have access to them. I'll see if my high level friend is able to give me a peek at the actions list for them, see if there are any that immediately stand out (since SE's job guide site doesn't show stuff like action changes)

Alluneve commented 1 month ago

@Enriath as a scholar main I can help with what Fey blessing is supposed to do, even though I find it rather silly and do not use it (I use barely any portion of this plugin to be honest)

Scholar has a couple of conflicting skills, with dissipation, seraphism and summon seraph all locking the scholar out of some of their toolkit when the button is pressed.

Fey Blessing is a small AOE heal cast by the fairy on a minute cooldown, starting from level 76. Fey Blessing cannot be cast while the fairy is not there (dissipation) and it cannot be cast if the fairy is currently seraph (with summon seraph). so the previous Combo setting made the inaccessible Fey blessing act as a second button of Consolation.

Keitenxx commented 1 month ago

Sorry for the question, i'm pretty a noob when it comes to github but how do i download this?

Alluneve commented 1 month ago

Sorry for the question, i'm pretty a noob when it comes to github but how do i download this?

honestly: just wait until Dalamud itself has a testing version available.

to give an actual answer to your question, you need to install a C# IDE (Visual Studio 2022) and download its dependencies to get the tools to compile a C# project into a usable dll, this usable dll you need to manually load into dalamud to activate a dev plugin.

than this dll you compiled yourself, if you do not know how to code, might do sketchy stuff to your PC (you do not know and cannot read the codebase). a plugin you do not know and blindly install can do a lot of things if the developer has malicious intent like delete your ffxiv character, hijack your account, install a keylogger or virus on your PC, or steal saved credit card information, ....

this is also the reason that dalamud has a giant fucking warning when adding custom plugin repositories:

We cannot take any responsibility for any custom plugins and repositories. if someone told you to copy/paste something here, it's very possible that you are being scammed or taken advantage of. Plugins have full control over your PC, like any other program, and may cause harm or crashes. They can delete your character, steal your FC or discord account, burn down your house. Please make absolutely sure that you only install plugins from developers you trust.

and that the dalamud server and the people who spend their free time helping the development of dalamud and its curated included plugin list that all plugins in there are vetted to not do anything sketchy,

the good thing about open-source stuff like this is that you can see for yourself what this does, in detail, but that requires you being able to read what it does and have some development background.

if you are interested in becoming a developer and learning more about how plugins work, I recommend first heading to a beginner C# course. Microsoft themselves have one: https://learn.microsoft.com/en-us/shows/csharp-fundamentals-for-absolute-beginners/

H-Mill commented 1 month ago

@Enriath It is not introduced in this pull request, but you should probably remove the unused namespace imports in MNK.cs and XIVComboConfiguration.cs

Xurtan commented 1 month ago

Sorry for the question, i'm pretty a noob when it comes to github but how do i download this?

honestly: just wait until Dalamud itself has a testing version available.

Out of curiosity do we have any idea on an ETA for this? Debating if I should go through the hassle of getting it another way or not.

Alluneve commented 1 month ago

Sorry for the question, i'm pretty a noob when it comes to github but how do i download this?

honestly: just wait until Dalamud itself has a testing version available.

Out of curiosity do we have any idea on an ETA for this? Debating if I should go through the hassle of getting it another way or not.

XIVLauncher and its plugins are entirely a hobby project. we do this because it's fun to us, and not because we get paid for it. The owner and developer of XIVCombo (Attick) are currently indisposed because of moving homes.

Getting this through another way is kind-off disrespecting the sovereignty of the developer to work on their hobby project at their own pace. Speaking for myself, I find it extremely disrespectful to have an army of people hammering on me to "go fix X plugin". Especially if they actively go out of their way to look over my shoulder (GitHub pages, branches,...) to see if they can run off with half-finished work.

Please take a long look in the mirror and take a seat. From what Attick has mentioned before, they do not mind and have no issues with people helping out to get all the work done to get a testing/release version available. so if you have the intention to help as a developer, go ahead.

if you are the 400th voice in a crowd of: "When plogon back waaaaaah", please remain quiet.

while this might be slightly hostile worded, it is not a personal attack, but a general statement for everyone who falls into the vocal camp of: "when plogon back" and "if I should go through the hassle of getting it another way"

attickdoor commented 1 month ago

Let's get this fella in.