[Item] Dragonwrath, Tarecgosa's Rest #989

InDebt · 2024-08-30T06:39:06Z

This is the implementation PR for Dragonwrath, Tarecgosa's Rest.
Implements #946 for further information.

Note

This is an early draft encouring early feedback and participation. Class specific implementations are highly encouraged as I don't know how much time I have a week to get this finished.

Current status

Open topics

On retail Spells instantiate casts which carry a lot of meta information on the actual circumstances and values used during their instantiation. Like base spell power when cast, attackpower and so on. Most importantly casts know the meta data for their calculations. I.e. The pulse lightning capaciatotor knows the charges it was triggered with or Fulmination the lightning shield stacks it was triggered with. In theory DRT copies casts not spells. Therfore the meta data is the same and the same calculations will be used causing fulimnation and capacitator to use the same aura charges for damage calculations. This is something rather tricky to realize right now in our backend and we have to see if all instances can simply "missuse" the BonusSpellDamage field for such things.

Polynomix · 2024-08-30T09:12:53Z

sim/common/cata/dragonwrath.go

+					return
+				}
+
+				// make sure the same spell impact can only trigger once per timestamp (AoE Impact spells like Arcane Explosion or Mind Sear)


This might prevent multiple Chain Lightning Overloads from being duplicated

You might be right. Trigger/Proc systems are always the hardest to get right even when I was working on p-servers as they heavily depend on how blizz actually implemented stuff.

I suppose we have the following:

TARGET_UNIT_TARGET_ENEMY - proc per target even chain targets

TARGET_UNIT_DEST_AREA_ENEMY and TARGET_UNIT_SRC_AREA_ENEMY - per cast no matter how many targets

Usually those are spells like concecrate, blizzard, arcane explosion or mind sear.

Obviously we don't have access to the target enums for the spells but I guess we need to be able to configure those spells in the class setup. I'll take a look to provide a clean API.

I meant more because of 2 different overload proc. You chain lightning on 3 targets, 2 of those hits procs CL Overload and both of those overloads can be duplicated, they're 2 different spells with same id happening at the same time. As to how the duplicated spell will work (only first target, new chain, 1 hit per target), i guess there is no way to know

I more and more wish we had a differenitation between spells and casts i guess xD

So I refactored spell configuration a bit and added some hopefully nice to use APIs around it. What you described should now be the default case and can be configured to be handled otherwise easily.

* Also adds a custom handler for the script effect of Conflagrate

InDebt · 2024-08-30T12:40:40Z

@1337LutZ Do you have the time and would want to look into how we could display a, perferably spec specific disclaimer for DTR in the UI? Right now I have desigend the staff's implementation that allows us to implement it spec by spec. So the accuracy may very from spec to spec.

1337LutZ · 2024-08-30T14:51:46Z

@1337LutZ Do you have the time and would want to look into how we could display a, perferably spec specific disclaimer for DTR in the UI? Right now I have desigend the staff's implementation that allows us to implement it spec by spec. So the accuracy may very from spec to spec.

Sure thing!
Would this just be coded in the the sim.ts config? (I assume yes)
I'll have time this weekend to look into this.

stn-ms · 2024-08-30T19:32:30Z

sim/druid/balance/dragonwrath.go

+func init() {
+	cata.CreateDTRClassConfig(proto.Spec_SpecBalanceDruid, 0.072).
+		AddSpell(8921, cata.NewDragonwrathSpellConfig().SupressImpact()). // Moonfire
+		AddSpell(93402, cata.NewDragonwrathSpellConfig().SupressImpact()) // Sunfire


Please consider having a spell proc chance override. It seems like there are spells with very low proc chance, like Mushrooms and Typhoon - https://www.mmo-champion.com/threads/967211-FAQ-Dragonwrath-Tarecgosa-s-Rest-and-you-a-Balance-Druid

Right now I assume the 'low' proc chance reported is due to the fact that both typhoon and shrooms only get a single proc rool per 'cast' which hits serveral targets, which is to be expected.

Adding a custom proc chance later on would be very easy. I can just add a proc chance overwrite on the SpellConfig for the class if testing should confirm this.

[UI] Add Item Notices

1337LutZ · 2024-09-30T10:12:46Z

Just merged master into this branch since there were quite some conflicts

* Add proper implementation of flat crit scaling for lava burst abstraction * Add new class support * Add more DTR framework methods

…ature/dtr

1337LutZ · 2024-10-10T10:13:38Z

@InDebt Would it be an idea to for now only enable DTR under the Experimental flag?
That way it allows "hardcore" Sim users to play around with it, whilst the "click-and-go" users will see it as a stat stick when comparing to other classes. I believe otherwise all Class Discords will get a bunch of questions on "Why spec XXX does so much more DPS, this is unfair".

1337LutZ · 2024-10-10T12:10:14Z

sim/mage/blast_wave.go

-			if targetCount > 1 {
-				mage.FlamestrikeBW.Cast(sim, target)
-			}
+			// if targetCount > 1 {


Should this commented code be removed?

Yes. I moved the Improved Flamestrike logic into the talent and commented the previous code during testing. Will remove it, as it seems to work as I want.

InDebt · 2024-10-12T09:59:47Z

@InDebt Would it be an idea to for now only enable DTR under the Experimental flag? That way it allows "hardcore" Sim users to play around with it, whilst the "click-and-go" users will see it as a stat stick when comparing to other classes. I believe otherwise all Class Discords will get a bunch of questions on "Why spec XXX does so much more DPS, this is unfair".

Right now damage gains are close together. Do we even get the 'experimental' flag with the proto request so I could disable / enable class support on the backend? Because otherwise we'd need to thide the staff alltogether behind that flag.

1337LutZ · 2024-10-12T10:50:37Z

@InDebt Would it be an idea to for now only enable DTR under the Experimental flag? That way it allows "hardcore" Sim users to play around with it, whilst the "click-and-go" users will see it as a stat stick when comparing to other classes. I believe otherwise all Class Discords will get a bunch of questions on "Why spec XXX does so much more DPS, this is unfair".

Right now damage gains are close together. Do we even get the 'experimental' flag with the proto request so I could disable / enable class support on the backend? Because otherwise we'd need to thide the staff alltogether behind that flag.

As discussed in Discord let's wait with merging for now 👌

* Reduce latency to process on same sim tick * Update eclipse behaviour for Druids to match live (proc on cast complete, remove on damage done) * Update Wild Mushrooms spell implementation to properly work with DTR * Update some spells to not proc off procs properly * Update Volcano Trinket to count as a Weapon Proc correctly * Remove certain impact supressions that have now been validated * Update proc chances * Add AoE proc chance supression for AoE abilities

* Also fixup possible crash for t13 mages

InDebt · 2024-10-25T09:15:20Z

@1337LutZ and others of course. I think we're in a state where we could give this out for preliminary testing.
I've updated the discliamer for the weapon. Checked I think all the reported interactions and adjusted accordingly after the PTR.

There will certainly be minor things I will inevitably have missed, so I think it's a good point to let others help out find those things 😁 .

Lutz already went and did some math and at least the current gains from DTR seem to be within expectations.

1337LutZ · 2024-10-25T09:24:25Z

@1337LutZ and others of course. I think we're in a state where we could give this out for preliminary testing. I've updated the discliamer for the weapon. Checked I think all the reported interactions and adjusted accordingly after the PTR.

There will certainly be minor things I will inevitably have missed, so I think it's a good point to let others help out find those things 😁 .

Lutz already went and did some math and at least the current gains from DTR seem to be within expectations.

Super awesome, and thanks for all your hard work on making a extensible implementation. ❤️
I've shared some Local builds with DTR implemented with some of the Discord big brains and most see it in line and have already provided feedback for some changes that I relayed to you. 👍

…ature/dtr

[DTR] First draft for implementation

506afd2

github-actions bot added the Priest label Aug 30, 2024

InDebt mentioned this pull request Aug 30, 2024

[Item] Dragonwrath, Tarecgosa's Rest #946

Closed

16 tasks

InDebt added Mage Druid Shaman labels Aug 30, 2024

InDebt self-assigned this Aug 30, 2024

Polynomix reviewed Aug 30, 2024

View reviewed changes

[DTR] Improve spell config API

71dbd42

github-actions bot removed Mage Druid Shaman labels Aug 30, 2024

[DTR] Add sample implemntation for Balance, Affliction, Demo and Destro

1cfc41b

* Also adds a custom handler for the script effect of Conflagrate

github-actions bot added Druid Warlock labels Aug 30, 2024

stn-ms reviewed Aug 30, 2024

View reviewed changes

Add Item Notices

fb02375

1337LutZ mentioned this pull request Sep 1, 2024

[UI] Add Item Notices #991

Merged

kayla-glick and others added 2 commits September 1, 2024 16:23

Merge pull request #991 from wowsims/feature/item-implementation-warning

8052a55

[UI] Add Item Notices

Merge branch 'master' into feature/dtr

7697062

Marcel Ellermann added 3 commits October 1, 2024 15:06

[DTR] Update DTR implementation

32324b7

* Add proper implementation of flat crit scaling for lava burst abstraction * Add new class support * Add more DTR framework methods

Merge branch 'feature/dtr' of https://github.com/wowsims/cata into fe…

70529b7

…ature/dtr

Merge remote-tracking branch 'upstream/master' into feature/dtr

8c41022

github-actions bot added Hunter Mage Shaman Warrior labels Oct 9, 2024

1337LutZ reviewed Oct 10, 2024

View reviewed changes

Merge branch 'master' into feature/dtr

7ed3fb4

Merge branch 'master' into feature/dtr

8ea6a11

Merge branch 'master' into feature/dtr

43a229c

github-actions bot removed the Paladin label Oct 22, 2024

Marcel Ellermann added 3 commits October 23, 2024 12:02

Merge remote-tracking branch 'upstream/master' into feature/dtr

e4edb81

Update tests

553e262

github-actions bot added the Paladin label Oct 24, 2024

Marcel Ellermann added 9 commits October 24, 2024 15:11

[DTR] Fix proc flag and update eclipse handling

62d6cc5

* Also fixup possible crash for t13 mages

[DTR] Update unit tests

fbb9416

[DTR] Instant spells with DoT component should not refresh dot duration

3840ba0

[DTR] Udpate tests

730241b

[DTR] Update flamestrike and add more AoE spells

bfe09af

[DTR] Update tests

47d20e1

[DTR] Fix Warlock immolation aura and shadow mind sear

462162e

[DTR] Update tests

d753fbf

[DTR] Update Item notice

bfb6425

1337LutZ approved these changes Oct 25, 2024

View reviewed changes

1337LutZ added 2 commits October 25, 2024 12:15

Add new DTR gearset & APL for Eclipse lag

e4c7bec

Merge branch 'feature/dtr' of https://github.com/wowsims/cata into fe…

33ae582

…ature/dtr

stn-ms approved these changes Oct 25, 2024

View reviewed changes

[DTR] Adjust shadow proc rate to be more in line with logs

3c1f842

InDebt merged commit c01c223 into master Oct 25, 2024
2 checks passed

InDebt deleted the feature/dtr branch October 25, 2024 10:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Item] Dragonwrath, Tarecgosa's Rest #989

[Item] Dragonwrath, Tarecgosa's Rest #989

InDebt commented Aug 30, 2024 •

edited by 1337LutZ

Loading

Polynomix Aug 30, 2024

InDebt Aug 30, 2024

Polynomix Aug 30, 2024

InDebt Aug 30, 2024

InDebt Aug 30, 2024

InDebt commented Aug 30, 2024

1337LutZ commented Aug 30, 2024

stn-ms Aug 30, 2024

InDebt Aug 30, 2024

1337LutZ commented Sep 30, 2024

1337LutZ commented Oct 10, 2024 •

edited

Loading

1337LutZ Oct 10, 2024

InDebt Oct 12, 2024

InDebt commented Oct 12, 2024

1337LutZ commented Oct 12, 2024

InDebt commented Oct 25, 2024

1337LutZ commented Oct 25, 2024

[Item] Dragonwrath, Tarecgosa's Rest #989

[Item] Dragonwrath, Tarecgosa's Rest #989

Conversation

InDebt commented Aug 30, 2024 • edited by 1337LutZ Loading

Current status

Open topics

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

InDebt commented Aug 30, 2024

1337LutZ commented Aug 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1337LutZ commented Sep 30, 2024

1337LutZ commented Oct 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

InDebt commented Oct 12, 2024

1337LutZ commented Oct 12, 2024

InDebt commented Oct 25, 2024

1337LutZ commented Oct 25, 2024

InDebt commented Aug 30, 2024 •

edited by 1337LutZ

Loading

1337LutZ commented Oct 10, 2024 •

edited

Loading