Skip to content

Conversation

@zbee
Copy link
Member

@zbee zbee commented Aug 23, 2025

  • Upgrade Occult Crescent
    • Removed ShouldUsePhantomActions
      (this caused Phantom logic to run twice when a Phantom action was desired)
    • Upgraded BestPhantomAction to a TryGet-style method, with a ref for the action ID to go out, and the return being a bool for whether an action was found
      (without this ShouldUsePhantomActions was required, which caused all Phantom logic to run twice)
    • Reorganized all Phantom Jobs' regions into their own methods
      (may as well, it only makes it harder to search for it if not in the file)
    • Reorganized those methods, and the order in which they are called, to match the CCP
      (couldn't believe this wasn't already the case)
  • Add ContentSpecificActions
    • This calls OccultCrescent.TryGetPhantomAction(), and outs the returned action ID, and has a bool for if there is one that should be executed
    • This has placeholders for Variant/Bozja/etc (other content-specific logic), so ContentSpecificActions.TryGet() can be called a single time in all jobs, and Variant/Bozja/etc will not need added to every jobs' code.

@Tartarga re:#765 we can move the condensed logic for Variant actions into a format similar to Occult Crescent, then call that logic in ContentSpecificActions - then we can fully remove all Variant code from every job, and we don't have to add any more content-specific logic to any job ever again :D
I would appreciate if (once this is merged) you re-oriented #765 to use this new way, as the end-goal.

zbee added 3 commits August 22, 2025 22:22
…nd organize the individual phantom jobs according to the CCP.

This removes OC's previous `ShouldUsePhantomActions` method, and upgrades the `BestPhantomAction` method to a `TryGet` method. Firstly to prevent all of the Phantom logic from running twice when an OC action is desired, and secondly to streamline the code needed within each job.
This also replaces the Phantom Jobs' `region`s with actual separate (`private`) methods for each Job, and organizes them all in the order in which they appear in the CCP file.
…ll such action methods into.

 This is being done so more code doesn't need added to every single job, for every type of content.
@zbee zbee requested a review from Tartarga August 23, 2025 04:32
@zbee zbee added Optimization This is solely for increasing performance New Feature This adds a new option or feature labels Aug 23, 2025
@semanticdiff-com
Copy link

semanticdiff-com bot commented Aug 23, 2025

Copy link
Contributor

@edewen edewen left a comment

Choose a reason for hiding this comment

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

Built and tested a couple classes on dummy. new method works fine

Tartarga added a commit to Tartarga/WrathCombo that referenced this pull request Aug 24, 2025
@zbee zbee mentioned this pull request Aug 24, 2025
Copy link
Contributor

@Tartarga Tartarga left a comment

Choose a reason for hiding this comment

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

Code looks good to me

@Limiana Limiana merged commit 3468aca into PunishXIV:main Aug 25, 2025
@zbee zbee deleted the content/occult branch September 2, 2025 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

New Feature This adds a new option or feature Optimization This is solely for increasing performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants