From cba6af27add9722ea954d6084ecf2ae3212206a4 Mon Sep 17 00:00:00 2001 From: Crow Date: Fri, 20 Mar 2026 14:40:59 -0500 Subject: [PATCH] Fix Assassination Rogue Finishers and add Cold Blood (#2215) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Pull Request Description **Note for reviewers**: The Rogue files are very confusing, so for background, there is DpsRogueStrategy, which is for all Rogues and represented by the “dps” strategy in game, and there is also AssassinationRogueStrategy, which is for Assassination and Subtlety specs and represented by the “melee” strategy in game. So Combat has only the dps strategy, while Assassination and Subtlety have the dps and melee strategies. - The main focus of this PR is to fix an issue with Assassination Rogues that caused them to use Eviscerate instead of Envenom about 1/3 of the time they should have been using Envenom, which was significantly reducing their DPS. See the bottom of this post for an explanation for why this was happening and why the fix works. Well, LMK if you think it's wrong, but this is how I am understanding things, and my back-of-the-envelope math (also below) supports it. - After this PR, Assassination Rogues will use Eviscerate only if they are unable to use Envenom (don't have the ability learned or no Deadly Poison on the target) or if they don’t have Rank 3 in Master Poisoner. - Additionally, Assassination Rogues previously would use Envenom/Eviscerate at 3 or more combo points. This is suboptimal so I created a new “combo points 4 available” trigger that will fire at 4 or 5 combo points only. They will still use the finisher at 3 combo points if the mob is almost dead (via the existing “target with combo points almost dead” trigger). - I then added Cold Blood, which Rogues previously would not use at all. Now there is a ColdBloodAction(), and Cold Blood is used when a Rogue has at least 4 combo points, right before using Envenom (or Eviscerate). I implemented it as a standard BuffTrigger so they’ll just use the ability off cooldown. - While looking at the combo point triggers, I thought it was confusing that the “combo points available” trigger actually meant 5 combo points (presumably because the default parameter for combo points in ComboPointsAvailableTrigger() is 5). I changed the string to “combo points 5 available” so it’s less confusing going forward. This necessitated some changes in the Druid files too. - Next, I cleaned up DpsRogueStrategy a bit. Not a lot to say, just some duplicative or useless logic was removed. There shouldn’t be any impact on gameplay from the changes. - In the process of making the edits in the Druid files, I noticed that the trigger for Tiger’s Fury in OffhealDruidCatStrategy was “low energy,” which does not exist (there is a “light energy available,” but the EnergyAvailable triggers are for when energy is AT LEAST the designated level, not AT MOST the designated level). So I replaced the trigger with the already-existing “tiger’s fury” trigger, which I think is just a generic BuffTrigger so I don’t actually know why it exists (i.e., Druid will use the spell off cooldown). But this particular change is just a quick fix and not intended to be thoughtful (that would be outside the scope of this PR). ## Feature Evaluation - Describe the **minimum logic** required to achieve the intended behavior. - Describe the **processing cost** when this logic executes across many bots. There should be no relevant impact on performance. This PR adds one new action triggered by the standard BuffTrigger. Otherwise, these are just fixes to existing logic. ## How to Test the Changes The easiest way to test is to fight a boss that doesn't tend to result in downtime (since downtime can lead to the loss of deadly poison stacks, in which case Eviscerate will (and should be) used by Assassination Rogues). You can use a damage meter such as Skada to track ability use. You should see: - Assassination Rogues don't use Eviscerate at all, or very few times. - Assassination Rogues use Cold Blood. - Offheal Cat Druids use Tiger's Fury. - Otherwise, Rogue and Cat Druid behavior should remain the same. ## Impact Assessment - Does this change increase per-bot/per-tick processing or risk scaling poorly with thousands of bots? - [x] No, not at all - [ ] Minimal impact (**explain below**) - [ ] Moderate impact (**explain below**) - Does this change modify default bot behavior? - [ ] No - [x] Yes (**explain why**) Default behavior for Assassination Rogues was broken, as explained above. - Does this change add new decision branches or increase maintenance complexity? - [x] No - [ ] Yes (**explain below**) ## Messages to Translate Does this change add bot messages to translate? - [x] No - [ ] Yes (**list messages in the table**) | Message key | Default message | | --------------- | ------------------ | | | | | | | ## AI Assistance Was AI assistance used while working on this change? - [ ] No - [x] Yes (**explain below**) I had Claude help me diagnose the initial issue and help me understand the queue system. And I had it implement the changes that were just busywork (like combo point triggers). ## Final Checklist - [x] Stability is not compromised. - [x] Performance impact is understood, tested, and acceptable. - [x] Added logic complexity is justified and explained. - [x] Documentation updated if needed (Conf comments, WiKi commands). ## Notes for Reviewers The reason for why Assassination Rogues were using Eviscerate so frequently is due to the fact that Envenom and Eviscerate were part of the same TriggerNode. When actions are part of the same TriggerNode, they're processed together, and the actions are queued by priority. When the higher-priority action is executed, the lower-priority action is not cleared--it remains in the queue for expireActionTime from the config, which is 5 seconds by default. Then, as soon as the lower-priority action can be executed (without regard for triggers because it is already triggered, just sitting in the queue), it will execute. This pattern of code works fine ingame if (1) you are actually trying to queue actions, like what I did with Cold Blood -> Envenom, (2) there are other guards like IsUseful() and IsPossible() that keep unwanted actions from executing, or (3) the trigger is just constantly firing so the higher-priority action is always evaluated. But TriggerNode isn't really the right way to implement action priority--that's through ActionNode. AssassinationRogueStrategy had Envenom and Eviscerate in the same TriggerNode, and then the corresponding ActionNode had Rupture as a fallback. Now, I changed it so Eviscerate is instead a fallback in the Envenom ActionNode (and Rupture is removed entirely because Assassination Rogues just shouldn't be using it, except maybe on very high-armor targets that are immune to poison, but that is very niche). ~ I did some back-of-the-envelope math to check this pattern. Say we're in a situation where Deadly Poison is up so ideally the Rogue should use Envenom 100% of the time. Through the old system, what would happen when the trigger fired? - Rogue uses Envenom since it's the higher-priority action. - Due to the Ruthlessness talent, Rogue has a 60% chance of having 1 combo point after the finisher, 40% chance of 0 combo points. If it has 1 combo point, it uses Eviscerate immediately. - If it has 0 combo points, it uses Mutilate. Mutilate grants 2 combo points, unless it crits, in which case it grants 3 due to Seal Fate. If Mutilate doesn't crit, the Rogue has 2 combo points, and it uses Eviscerate. If Mutilate does crit, the Rogue has 3 combo points, and it uses Envenom. - So let's assume Mutilate has a 55% crit chance (very reasonable for a Rogue in entry-level raid gear with raid buffs due to Opportunity giving +20% crit chance to Mutilate). Mutilate hits twice, and if either hit crits, Seal Fate Procs. The chance of at least one crit with two hits at a 55% crit chance is ~80%. That means if Ruthlessness doesn't give a combo point, there is an 80% chance that Envenom will be used and a 20% chance that Eviscerate will be used. - Combine the above, and the result of one trigger firing is you get 1 guaranteed Envenom + 0.6 Eviscerates (Ruthlessness proc path) + 0.32 Envenoms (No Ruthlessness proc but Seal Fate proc path) + 0.08 Eviscerates (No Ruthlessness proc and no Seal Fate proc path) = 1.32 Envenoms to each 0.68 Eviscerates, or a 1.94:1 ratio of Envenoms to Eviscerates. That is basically identical to what I saw in practice of roughly a 2:1 ratio of Envenoms to Eviscerates. - I understand the above is simplistic and it assumes that the Rogue gets a combo point within 5 seconds following using Envenom (very likely) and that there are not two opportunities to use Envenom or Eviscerate in the 5-second queue period after using Envenom (it can happen but is uncommon). That's all at the margins and isn't going to impact the math very much. --------- Co-authored-by: Keleborn <22352763+Celandriel@users.noreply.github.com> Co-authored-by: bash Co-authored-by: Revision Co-authored-by: kadeshar --- src/Ai/Base/TriggerContext.h | 6 ++-- .../Druid/Strategy/CatDpsDruidStrategy.cpp | 2 +- .../Strategy/OffhealDruidCatStrategy.cpp | 4 +-- src/Ai/Class/Rogue/Action/RogueActions.cpp | 2 +- src/Ai/Class/Rogue/Action/RogueActions.h | 6 ++++ src/Ai/Class/Rogue/RogueAiObjectContext.cpp | 2 ++ .../Strategy/AssassinationRogueStrategy.cpp | 11 ++++---- .../Class/Rogue/Strategy/DpsRogueStrategy.cpp | 28 ++----------------- 8 files changed, 24 insertions(+), 37 deletions(-) diff --git a/src/Ai/Base/TriggerContext.h b/src/Ai/Base/TriggerContext.h index ceb7c001c..d4f8ac3b7 100644 --- a/src/Ai/Base/TriggerContext.h +++ b/src/Ai/Base/TriggerContext.h @@ -103,7 +103,8 @@ public: creators["enemy within melee"] = &TriggerContext::enemy_within_melee; creators["party member to heal out of spell range"] = &TriggerContext::party_member_to_heal_out_of_spell_range; - creators["combo points available"] = &TriggerContext::ComboPointsAvailable; + creators["combo points 5 available"] = &TriggerContext::ComboPoints5Available; + creators["combo points 4 available"] = &TriggerContext::ComboPoints4Available; creators["combo points 3 available"] = &TriggerContext::ComboPoints3Available; creators["target with combo points almost dead"] = &TriggerContext::target_with_combo_points_almost_dead; creators["combo points not full"] = &TriggerContext::ComboPointsNotFull; @@ -338,7 +339,8 @@ private: { return new PartyMemberToHealOutOfSpellRangeTrigger(botAI); } - static Trigger* ComboPointsAvailable(PlayerbotAI* botAI) { return new ComboPointsAvailableTrigger(botAI); } + static Trigger* ComboPoints5Available(PlayerbotAI* botAI) { return new ComboPointsAvailableTrigger(botAI, 5); } + static Trigger* ComboPoints4Available(PlayerbotAI* botAI) { return new ComboPointsAvailableTrigger(botAI, 4); } static Trigger* ComboPoints3Available(PlayerbotAI* botAI) { return new ComboPointsAvailableTrigger(botAI, 3); } static Trigger* target_with_combo_points_almost_dead(PlayerbotAI* ai) { diff --git a/src/Ai/Class/Druid/Strategy/CatDpsDruidStrategy.cpp b/src/Ai/Class/Druid/Strategy/CatDpsDruidStrategy.cpp index fda1b5f94..b1a4685b1 100644 --- a/src/Ai/Class/Druid/Strategy/CatDpsDruidStrategy.cpp +++ b/src/Ai/Class/Druid/Strategy/CatDpsDruidStrategy.cpp @@ -228,7 +228,7 @@ void CatDpsDruidStrategy::InitTriggers(std::vector& triggers) ); triggers.push_back( new TriggerNode( - "combo points available", + "combo points 5 available", { NextAction("rip", ACTION_HIGH + 6) } diff --git a/src/Ai/Class/Druid/Strategy/OffhealDruidCatStrategy.cpp b/src/Ai/Class/Druid/Strategy/OffhealDruidCatStrategy.cpp index c472ce8d8..fb7893651 100644 --- a/src/Ai/Class/Druid/Strategy/OffhealDruidCatStrategy.cpp +++ b/src/Ai/Class/Druid/Strategy/OffhealDruidCatStrategy.cpp @@ -176,7 +176,7 @@ void OffhealDruidCatStrategy::InitTriggers(std::vector& triggers) ); triggers.push_back( new TriggerNode( - "combo points available", + "combo points 5 available", { NextAction("rip", ACTION_HIGH + 6) } @@ -257,7 +257,7 @@ void OffhealDruidCatStrategy::InitTriggers(std::vector& triggers) ); triggers.push_back( new TriggerNode( - "low energy", + "tiger's fury", { NextAction("tiger's fury", ACTION_NORMAL + 1) } diff --git a/src/Ai/Class/Rogue/Action/RogueActions.cpp b/src/Ai/Class/Rogue/Action/RogueActions.cpp index b554a8253..46beaf86c 100644 --- a/src/Ai/Class/Rogue/Action/RogueActions.cpp +++ b/src/Ai/Class/Rogue/Action/RogueActions.cpp @@ -61,7 +61,7 @@ bool CastEnvenomAction::isUseful() bool CastEnvenomAction::isPossible() { // alternate to eviscerate if talents unlearned - return botAI->HasAura(58410, bot) /* Master Poisoner */; + return botAI->HasAura(58410, bot) /* Master Poisoner Rank 3 */; } bool CastTricksOfTheTradeOnMainTankAction::isUseful() diff --git a/src/Ai/Class/Rogue/Action/RogueActions.h b/src/Ai/Class/Rogue/Action/RogueActions.h index dd0ad4735..3ae1f8142 100644 --- a/src/Ai/Class/Rogue/Action/RogueActions.h +++ b/src/Ai/Class/Rogue/Action/RogueActions.h @@ -78,6 +78,12 @@ public: CastFeintAction(PlayerbotAI* botAI) : CastBuffSpellAction(botAI, "feint") {} }; +class CastColdBloodAction : public CastBuffSpellAction +{ +public: + CastColdBloodAction(PlayerbotAI* botAI) : CastBuffSpellAction(botAI, "cold blood") {} +}; + class CastDismantleAction : public CastSpellAction { public: diff --git a/src/Ai/Class/Rogue/RogueAiObjectContext.cpp b/src/Ai/Class/Rogue/RogueAiObjectContext.cpp index 8586d93d1..4b4ebab1e 100644 --- a/src/Ai/Class/Rogue/RogueAiObjectContext.cpp +++ b/src/Ai/Class/Rogue/RogueAiObjectContext.cpp @@ -143,6 +143,7 @@ public: creators["use instant poison on off hand"] = &RogueAiObjectContextInternal::use_instant_poison_off_hand; creators["fan of knives"] = &RogueAiObjectContextInternal::fan_of_knives; creators["killing spree"] = &RogueAiObjectContextInternal::killing_spree; + creators["cold blood"] = &RogueAiObjectContextInternal::cold_blood; } private: @@ -184,6 +185,7 @@ private: static Action* use_instant_poison_off_hand(PlayerbotAI* ai) { return new UseInstantPoisonOffHandAction(ai); } static Action* fan_of_knives(PlayerbotAI* ai) { return new FanOfKnivesAction(ai); } static Action* killing_spree(PlayerbotAI* ai) { return new CastKillingSpreeAction(ai); } + static Action* cold_blood(PlayerbotAI* ai) { return new CastColdBloodAction(ai); } }; SharedNamedObjectContextList RogueAiObjectContext::sharedStrategyContexts; diff --git a/src/Ai/Class/Rogue/Strategy/AssassinationRogueStrategy.cpp b/src/Ai/Class/Rogue/Strategy/AssassinationRogueStrategy.cpp index b8563893c..a104fae07 100644 --- a/src/Ai/Class/Rogue/Strategy/AssassinationRogueStrategy.cpp +++ b/src/Ai/Class/Rogue/Strategy/AssassinationRogueStrategy.cpp @@ -29,7 +29,7 @@ private: return new ActionNode( "envenom", /*P*/ {}, - /*A*/ { NextAction("rupture") }, + /*A*/ { NextAction("eviscerate") }, /*C*/ {} ); } @@ -108,10 +108,10 @@ void AssassinationRogueStrategy::InitTriggers(std::vector& trigger triggers.push_back( new TriggerNode( - "combo points 3 available", + "combo points 4 available", { - NextAction("envenom", ACTION_HIGH + 5), - NextAction("eviscerate", ACTION_HIGH + 3) + NextAction("cold blood", ACTION_HIGH + 6), + NextAction("envenom", ACTION_HIGH + 5) } ) ); @@ -120,8 +120,7 @@ void AssassinationRogueStrategy::InitTriggers(std::vector& trigger new TriggerNode( "target with combo points almost dead", { - NextAction("envenom", ACTION_HIGH + 4), - NextAction("eviscerate", ACTION_HIGH + 2) + NextAction("envenom", ACTION_HIGH + 4) } ) ); diff --git a/src/Ai/Class/Rogue/Strategy/DpsRogueStrategy.cpp b/src/Ai/Class/Rogue/Strategy/DpsRogueStrategy.cpp index 22c6a6f83..06aeda57c 100644 --- a/src/Ai/Class/Rogue/Strategy/DpsRogueStrategy.cpp +++ b/src/Ai/Class/Rogue/Strategy/DpsRogueStrategy.cpp @@ -12,36 +12,14 @@ class DpsRogueStrategyActionNodeFactory : public NamedObjectFactory public: DpsRogueStrategyActionNodeFactory() { - creators["mutilate"] = &mutilate; creators["sinister strike"] = &sinister_strike; creators["kick"] = &kick; creators["kidney shot"] = &kidney_shot; creators["backstab"] = &backstab; - creators["melee"] = &melee; creators["rupture"] = &rupture; } private: - static ActionNode* melee([[maybe_unused]] PlayerbotAI* botAI) - { - return new ActionNode( - "melee", - /*P*/ {}, - /*A*/ { - NextAction("mutilate") }, - /*C*/ {} - ); - } - static ActionNode* mutilate([[maybe_unused]] PlayerbotAI* botAI) - { - return new ActionNode( - "mutilate", - /*P*/ {}, - /*A*/ { - NextAction("sinister strike") }, - /*C*/ {} - ); - } static ActionNode* sinister_strike([[maybe_unused]] PlayerbotAI* botAI) { return new ActionNode( @@ -77,7 +55,7 @@ private: "backstab", /*P*/ {}, /*A*/ { - NextAction("mutilate") }, + NextAction("sinister strike") }, /*C*/ {} ); } @@ -140,7 +118,7 @@ void DpsRogueStrategy::InitTriggers(std::vector& triggers) triggers.push_back( new TriggerNode( - "combo points available", + "combo points 5 available", { NextAction("rupture", ACTION_HIGH + 1), NextAction("eviscerate", ACTION_HIGH) @@ -335,7 +313,7 @@ void StealthedRogueStrategy::InitTriggers(std::vector& triggers) { triggers.push_back( new TriggerNode( - "combo points available", + "combo points 5 available", { NextAction("eviscerate", ACTION_HIGH) }