Skip to content

[DRG] LvL 90 Ultimate sim - #345

Open
wolfcomp wants to merge 8 commits into
xiv-gear-planner:mainfrom
wolfcomp:master
Open

[DRG] LvL 90 Ultimate sim#345
wolfcomp wants to merge 8 commits into
xiv-gear-planner:mainfrom
wolfcomp:master

Conversation

@wolfcomp

@wolfcomp wolfcomp commented Oct 11, 2024

Copy link
Copy Markdown

Has all of DRG ability actions for up to LvL 90 in use for TOP and DSR

Notes

Life of the dragon is just printed as a name as it's a time based buff that is shown through the Gauge and has no icon attached to it

@wolfcomp
wolfcomp marked this pull request as ready for review October 24, 2024 17:20
@wolfcomp wolfcomp changed the title Add some extra sims [DRG] LvL 90 Ultimate sim Oct 24, 2024
Comment thread packages/core/src/sims/melee/drg/drg_top_sim.ts Outdated
Comment thread packages/core/src/sims/melee/drg/drg_top_sim.ts Outdated
@wolfcomp

wolfcomp commented Oct 28, 2024

Copy link
Copy Markdown
Author

If it complains about linting, I have no idea how to get the same linter to run on my machine to actually fix it as this is the result i get

@wolfcomp

Copy link
Copy Markdown
Author

finally got the linter to show the same as the workflow so now this should be happy

name: "Drakesbane",
id: 36952,
attackType: "Weaponskill",
potency: 400,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I know this sim only supports level 90 for now, but if other levels were being added in the future, it'd be great if we used levelModifiers on these abilities to represent their potencies at other levels, so that we only needed to support one list of DRG abilities and not maintain a separate list for every level. For example, from the DRK sim:

export const HardSlash: DrkGcdAbility = {
    type: 'gcd',
    name: "Hard Slash",
    id: 3617,
    potency: 150,
    attackType: "Weaponskill",
    gcd: 2.5,
    appDelay: 0.58,
    cast: 0,
    levelModifiers: [
        {
            minLevel: 84,
            potency: 180,
        },
        {
            minLevel: 94,
            potency: 300,
        },
    ],
};

I noticed this because Drakesbane is 440 potency at level 100 :)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I would say that this is something that should be done if the other sims gets implemented and not tossed to this portion.

},
supportedJobs: ["DRG"],
supportedLevels: [90],
isDefaultSim: false,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this be true?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I would say only the end level sim should be the default which would be in this case 100

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A default sim will show up only for its supported levels, so you should be safe here. If setting this to true makes the sim show up for level 100 gearsets, then there'd be a bug

Comment thread packages/core/src/sims/melee/drg/drg_top_sim.ts Outdated
return new DrgTopSim(exported);
},
supportedJobs: ["DRG"],
supportedLevels: [90],

@VioletHynes VioletHynes Dec 1, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There's probably not a lot of extra work to get this supported for level 100, too, though I understand that is some extra work, and it's fine to not want to address that for now.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If we did this, we wouldn't need to call it drg_top_sim either, it'd just be drg_sheet_sim. If you're interested in an example, DRK supports 70-100.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I would rather not have to maintain an Excel sheet thing in order for this to work. I know that it is something that could be done, and more me just not wanting to rely on other services working for a single sim to work.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Perhaps the nomenclature is confusing here, but "sheet" in this comment doesn't mean an Excel sheet, it means gearsheet. This wouldn't mean needing to support any other service, it's just a naming thing. Most of our sims are called job_sheet_sim etc.

type: 'ogcd',
name: "Nastrond",
id: 7400,
potency: 360,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This (and Nastrond Ready) will need to be updated following the balance patch.

const span = document.createElement('span');
span.textContent = `${firstmindsFocus}`;

const barOuter = document.createElement('div');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: instead of a bar, we could consider using circles to note stacks, like the Monk Sim for Chakras or the GNB sim for carts

dmgIncrease: 0.15,
},
duration: 20,
// No status ID due to being a hidden buff

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since it's hidden, it would be great to show in the UI

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It both is hidden and not hidden. The reason being that the status itself isn't a displayed thing, but it does show it through the Job Gauge with a countdown timer

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Life of the Dragons 15% damage up still has to be shown in the total buffs column and it should be listed with an icon to make it more readable. While there is no direct buff icon for it in game, the status icon for LotD still exists https://arcanedisgea.github.io/xiv-id/?index=Status&query=life+of+the+dragon
We always display it as an active buff in our rotation builders so people are used to seeing it.

apply(cp: DRG90CycleProcessor) {
const numPots = Math.ceil(cp.remainingTime / 360); // Force pots during 2mins
const maxAllowableDelay = cp.remainingTime - ((numPots - 1) * 360);
const potOpener = maxAllowableDelay <= 120;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The pot doesn't seem to be applying in the opener from the image, and the above logic seems like it would need to be in the remainingCycles function somewhere (e.g. useOgcdCooldown) to actually use pots past the opener

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Then most likely another sim is also broken since I took most of the pot logic from there

@RhyTamok

Copy link
Copy Markdown

This rotation is not ready to ship. Judging by the screenshot there are multiple major (and minor) issues that need to be addressed first.

  • Rotation is started with the wrong combo. Always needs to be Chaotic Spring combo first to apply Power Surge.
  • Every combo is broken. The respective second combo actions (Disembowel and Vorpal Thrust) are always replaced by a True Thrust. This also means that Power Surge is never accounted for.
  • Logic around double weaves needs to be adjusted. There should not be any double weaves with Stardiver
  • Logic for Life Surge has to be adjusted. Unless we're talking about the last 0.1% of optimization it should only be used on Heavens' Thrust and Drakesbane. It is used on Chaotic Spring in the opener here. It should also be held for burst windows and not necessarily used on cooldown
  • Geirskoguls 15% damage buff is not accounted for
  • Stardiver is used outside of Life at 42s
  • This isn't clear from the screenshot but Wyrmwind Thrust should also be held for buffs if possible (Always use before next Raiden, hold for any buffs applied before that)

The rotations for level 90 and 100 are virtually identical. You only gain 2 more follow-up skills and a replacement for the second combo skills, so as long as the double weave logic is sounds, it should be viable for both.
I am not sure how skillspeed is handled here but for speeds faster than 2.50 it becomes considerably more complex as you have to account for 9 GCD buff windows and delaying oGCD to not clip while still keeping optimal buff windows.

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.

4 participants