Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Summon Fairy and Fairy Action consolidation #19

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

CostasAK
Copy link

@CostasAK CostasAK commented Aug 13, 2020

Closes #17

To-Do

  • Find skill IDs for both summon actions, whispering dawn and fey illumination.
  • Test skill IDs.
  • Add config entry.
  • Test config flag.
  • Test config actions.
  • Logic to find Fairy in actor table.
  • Only match Fairy if current player is the owner.
  • Test fairy logic.

@CostasAK CostasAK changed the title WIP: Summon Fairy and Fairy Action consolidation Summon Fairy and Fairy Action consolidation Aug 14, 2020
@CostasAK CostasAK marked this pull request as ready for review August 14, 2020 16:18
@CostasAK
Copy link
Author

Didn't change any of the version numbers, since I wasn't 100% on what your scheme is for numbering. ^^

@attickdoor
Copy link
Collaborator

I'll check this out and get to it soon. Busy week or so upcoming.

@CostasAK
Copy link
Author

If there's anything I can do for you to make your process easier, let me know. Like run some more tests perhaps. I made it as similar as I could to how you had already wrote the rest. :) in any case, take your time!

@CostasAK
Copy link
Author

CostasAK commented Aug 15, 2020

Successful Tests

  • Summon Eos or Selene.
  • Summon Seraph.
  • Dissipation.
    • Note: There is a small fraction of a second after Dissipation ends and before your fairy returns where the buttons quickly switches between the Summon actions and the Fairy actions. This happens equally when checking the JobGauge or the Dissipation buff. This could be prevented by adding a timer or some sort of memory specifically for dissipation, but I think it's probably not that much of an issue anyway. Kind of serves as a nice reminder your fairy is back.
  • Have someone else's Eos, Selene or Seraph nearby.
  • Have a party member with Eos, Selene, Seraph or Dissipation.

In all the above cases, the buttons behave exactly as expected.

I added some conditionals before looping through the actor table, because I figured that looping through the actor table in busy areas could become cumbersome in extreme cases. A few cases where looking through the table is unnecessary are checked beforehand.
The returns for the buttons are always reached, regardless of these conditionals (only obeying the Config flag), following the style of all your work.

Copy link
Collaborator

@attickdoor attickdoor left a comment

Choose a reason for hiding this comment

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

There has to be a better way to find the fairy other than iterating the entire actor table. Reading the table is an expensive operation for something that runs multiple times per rendered frame. Everything else looks fine but there's got to be a better way to do this.

@attickdoor
Copy link
Collaborator

Addendum: sorry this took so long to get to, classes have started back up and I've been waist-deep in upgrading TitleEdit in the meantime.

@CostasAK
Copy link
Author

CostasAK commented Sep 3, 2020

Alright, fair enough. I will look for a lower complexity method of finding the fairy. This was why I added some more conditionals, but you're correct that it isn't very future-proof or some machines might not like having to check a potentially large table often.
Just in case: would a timeout be acceptable, or is there already a 'slower' process in your plugin that could house a check like parsing the actortable? Obviously not the cleanest solution still, just checking what could theoretically be acceptable.

@czlr
Copy link

czlr commented Feb 25, 2022

Just to chime in on this, since I was looking into consolidating fairy on SCH myself today
The way that I ended up implementing it was to check if pet skills are usable. If they are and you're SCH (and it's not dissipation), assume fairy is summoned
I don't believe there's another case where pet controls are available otherwise
Also this has the benefit of resolving seraph for free

I haven't checked how pet controls interact with certain CCs (stunned, locked for transitions, etc.). But maybe you can check for this case against a skill that would otherwise always be available (jump or sprint maybe?)

Some snippets:

private static FFXIVClientStructs.FFXIV.Client.Game.ActionManager* actionManager = FFXIVClientStructs.FFXIV.Client.Game.ActionManager.Instance();

// True if usable
// 1 is the skill id for "Away" in the context of pet actions. This was arbitrarily chosen, and could be any other pet action
actionManager->GetActionStatus(ActionType.PetAction, 1) == 0;

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.

Feature: SCH Fairy
3 participants