Fix Assassination Rogue Finishers and add Cold Blood#2215
Merged
kadeshar merged 9 commits intoMar 20, 2026
Conversation
Master update from Test-staging: Fix ObjectAccessor retrieval, optimize EquipActions, and implement RaidBossHelpers
This reverts commit c86032f.
Master update from Test staging
Update master from Test staging and Core Update
Test staging to master
kadeshar
reviewed
Mar 16, 2026
kadeshar
approved these changes
Mar 17, 2026
SmashingQuasar
pushed a commit
to ShatteredDawn/mod-playerbots
that referenced
this pull request
Apr 3, 2026
…2215) <!-- Thank you for contributing to mod-playerbots, please make sure that you... 1. Submit your PR to the test-staging branch, not master. 2. Read the guidelines below before submitting. 3. Don't delete parts of this template. DESIGN PHILOSOPHY: We prioritize STABILITY, PERFORMANCE, AND PREDICTABILITY over behavioral realism. Every action and decision executes PER BOT AND PER TRIGGER. Small increases in logic complexity scale poorly across thousands of bots and negatively affect all. We prioritize a stable system over a smarter one. Bots don't need to behave perfectly; believable behavior is the goal, not human simulation. Default behavior must be cheap in processing; expensive behavior must be opt-in. Before submitting, make sure your changes aligns with these principles. --> <!-- Describe what this change does and why it is needed --> **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). <!-- If your PR is very minimal (comment typo, wrong ID reference, etc), and it is very obvious it will not have any impact on performance, you may skip these question. If necessary, a maintainer may ask you for them later. --> <!-- Please answer the following: --> - 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. <!-- - Step-by-step instructions to test the change. - Any required setup (e.g. multiple players, number of bots, specific configuration). - Expected behavior and how to verify it. --> 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. <!-- As a generic test, before and after measure of pmon (playerbot pmon tick) can help you here. --> - 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**) <!-- Bot messages have to be translatable, but you don't need to do the translations here. You only need to make sure the message is in a translatable format, and list in the table the message_key and the default English message. Search for GetBotTextOrDefault in the codebase for examples. --> Does this change add bot messages to translate? - [x] No - [ ] Yes (**list messages in the table**) | Message key | Default message | | --------------- | ------------------ | | | | | | | <!-- AI assistance is allowed, but all submitted code must be fully understood, reviewed, and owned by the contributor. We expect contributors to be honest about what they do and do not understand. --> Was AI assistance used while working on this change? - [ ] No - [x] Yes (**explain below**) <!-- If yes, please specify: - Purpose of usage (e.g. brainstorming, refactoring, documentation, code generation). - Which parts of the change were influenced or generated, and whether it was thoroughly reviewed. --> 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). - [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). <!-- Anything else that's helpful to review or test your pull request. --> 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 <hermensb@gmail.com> Co-authored-by: Revision <tkn963@gmail.com> Co-authored-by: kadeshar <kadeshar@gmail.com>
SmashingQuasar
added a commit
to ShatteredDawn/mod-playerbots
that referenced
this pull request
Apr 3, 2026
…2215) (#55) <!-- Thank you for contributing to mod-playerbots, please make sure that you... 1. Submit your PR to the test-staging branch, not master. 2. Read the guidelines below before submitting. 3. Don't delete parts of this template. DESIGN PHILOSOPHY: We prioritize STABILITY, PERFORMANCE, AND PREDICTABILITY over behavioral realism. Every action and decision executes PER BOT AND PER TRIGGER. Small increases in logic complexity scale poorly across thousands of bots and negatively affect all. We prioritize a stable system over a smarter one. Bots don't need to behave perfectly; believable behavior is the goal, not human simulation. Default behavior must be cheap in processing; expensive behavior must be opt-in. Before submitting, make sure your changes aligns with these principles. --> <!-- Describe what this change does and why it is needed --> **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). <!-- If your PR is very minimal (comment typo, wrong ID reference, etc), and it is very obvious it will not have any impact on performance, you may skip these question. If necessary, a maintainer may ask you for them later. --> <!-- Please answer the following: --> - 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. <!-- - Step-by-step instructions to test the change. - Any required setup (e.g. multiple players, number of bots, specific configuration). - Expected behavior and how to verify it. --> 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. <!-- As a generic test, before and after measure of pmon (playerbot pmon tick) can help you here. --> - 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**) <!-- Bot messages have to be translatable, but you don't need to do the translations here. You only need to make sure the message is in a translatable format, and list in the table the message_key and the default English message. Search for GetBotTextOrDefault in the codebase for examples. --> Does this change add bot messages to translate? - [x] No - [ ] Yes (**list messages in the table**) | Message key | Default message | | --------------- | ------------------ | | | | | | | <!-- AI assistance is allowed, but all submitted code must be fully understood, reviewed, and owned by the contributor. We expect contributors to be honest about what they do and do not understand. --> Was AI assistance used while working on this change? - [ ] No - [x] Yes (**explain below**) <!-- If yes, please specify: - Purpose of usage (e.g. brainstorming, refactoring, documentation, code generation). - Which parts of the change were influenced or generated, and whether it was thoroughly reviewed. --> 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). - [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). <!-- Anything else that's helpful to review or test your pull request. --> 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: Crow <pengchengw@me.com> Co-authored-by: Keleborn <22352763+Celandriel@users.noreply.github.com> Co-authored-by: bash <hermensb@gmail.com> Co-authored-by: Revision <tkn963@gmail.com> Co-authored-by: kadeshar <kadeshar@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
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:
Impact Assessment
Does this change increase per-bot/per-tick processing or risk scaling poorly with thousands of bots?
Does this change modify default bot behavior?
Default behavior for Assassination Rogues was broken, as explained above.
Messages to Translate
Does this change add bot messages to translate?
AI Assistance
Was AI assistance used while working on this change?
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
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?