simulationcraft / simc

Simulationcraft engine/GUI
GNU General Public License v3.0
1.41k stars 695 forks source link

Add a "flying" target buff and a raid event to activate it #1078

Closed navv1234 closed 9 years ago

navv1234 commented 9 years ago

Originally reported on Google Code with ID 1079

What steps will reproduce the problem?
Simming a spec with ground aoe abilities (UH DK, Ret pally).

What is the expected output? What do you see instead?
Ground AOE effects are unable to hit Ultraxion in the actual encounter.

What version of the product are you using? On what operating system?
svn 10778, 7 64 bit

Please provide any additional information below.
Not sure if its worth the trouble but it does cause some inaccuracy in certain specs.
 Impact is negligible for ret but fairly significant for UH.  Hunter traps also fall
under this rule I believe.

Reported by Garrett.R.Green on 2012-01-04 22:16:56

navv1234 commented 9 years ago
The Ultraxion fight style is just a set of raid events and other settings that setup
the sim to mimic the fight. With that said, there's no flag or anything to disable
certain effects from affecting a certain mob and it's not worth the effort or the additional
complexity to add it.

Ultimately the sim is only as good as the inputs and settings that are used, which
in this case means updating your action list to account for this specific boss.

If another developer feels differently, feel free to reopen this.

Reported by swbusche on 2012-01-04 22:32:41

navv1234 commented 9 years ago
Yeah I honestly just spaced on editing the action list to achieve the desired effect,
thanks.

Reported by Garrett.R.Green on 2012-01-04 23:16:39

navv1234 commented 9 years ago
Here's a pretty simple diff that will have the desired effect. 

Index: sc_paladin.cpp
===================================================================
--- sc_paladin.cpp  (revision 10803)
+++ sc_paladin.cpp  (working copy)
@@ -3252,7 +3252,10 @@
       action_list_str += "/judgement,if=set_bonus.tier13_2pc_melee&buff.zealotry.up&holy_power<3";
       action_list_str += "/wait,sec=0.1,if=cooldown.crusader_strike.remains<0.2&cooldown.crusader_strike.remains>0";
       action_list_str += "/holy_wrath";
-      action_list_str += "/consecration,if=mana>16000";  // Consecration is expensive,
only use if we have plenty of mana
+      if ( ! util_t::str_compare_ci( sim -> fight_style, "Ultraxion" ) )
+      {
+        action_list_str += "/consecration,if=mana>16000";  // Consecration is expensive,
only use if we have plenty of mana
+      }
       action_list_str += "/divine_plea";
     }
     break;
Index: sc_death_knight.cpp
===================================================================
--- sc_death_knight.cpp (revision 10803)
+++ sc_death_knight.cpp (working copy)
@@ -4600,7 +4600,10 @@
         action_list_str += "/summon_gargoyle,time<=60";
         action_list_str += "/summon_gargoyle,if=buff.bloodlust.react|buff.unholy_frenzy.react";
       }
-      action_list_str += "/death_and_decay,if=unholy=2&runic_power<110";
+      if ( ! util_t::str_compare_ci( sim -> fight_style, "Ultraxion" ) )
+      {
+        action_list_str += "/death_and_decay,if=unholy=2&runic_power<110";
+      }
       action_list_str += "/scourge_strike,if=unholy=2&runic_power<110";
       action_list_str += "/festering_strike,if=blood=2&frost=2&runic_power<110";
       action_list_str += "/death_coil,if=runic_power>90";

Please include this as our Death Knights are grumbling too. :-)

Reported by coleb2 on 2012-01-07 21:55:50

navv1234 commented 9 years ago
Oops, death_and_decay is actually in the sim twice. This should handle it. 

diff --git a/engine/sc_death_knight.cpp b/engine/sc_death_knight.cpp
index 2e6a813..a0f53db 100644
--- a/engine/sc_death_knight.cpp
+++ b/engine/sc_death_knight.cpp
@@ -4533,13 +4533,19 @@ void death_knight_t::init_actions()
         action_list_str += "/summon_gargoyle,time<=60";
         action_list_str += "/summon_gargoyle,if=buff.bloodlust.react|buff.unholy_frenzy.react";
       }
-      action_list_str += "/death_and_decay,if=unholy=2&runic_power<110";
+      if ( ! util_t::str_compare_ci( sim -> fight_style, "Ultraxion" ) )
+      {
+        action_list_str += "/death_and_decay,if=unholy=2&runic_power<110";
+      }
       action_list_str += "/scourge_strike,if=unholy=2&runic_power<110";
       action_list_str += "/festering_strike,if=blood=2&frost=2&runic_power<110";
       action_list_str += "/death_coil,if=runic_power>90";
       if ( talents.sudden_doom -> rank() )
         action_list_str += "/death_coil,if=buff.sudden_doom.react";
-      action_list_str += "/death_and_decay";
+      if ( ! util_t::str_compare_ci( sim -> fight_style, "Ultraxion" ) )
+      {
+        action_list_str += "/death_and_decay";
+      }
       action_list_str += "/scourge_strike";
       action_list_str += "/festering_strike";
       action_list_str += "/death_coil";
diff --git a/engine/sc_paladin.cpp b/engine/sc_paladin.cpp
index 5ffc916..e1bdc26 100644
--- a/engine/sc_paladin.cpp
+++ b/engine/sc_paladin.cpp
@@ -3252,7 +3252,10 @@ void paladin_t::init_actions()
       action_list_str += "/judgement,if=set_bonus.tier13_2pc_melee&buff.zealotry.up&holy_power<3";
       action_list_str += "/wait,sec=0.1,if=cooldown.crusader_strike.remains<0.2&cooldown.crusader_strike.remains>0";
       action_list_str += "/holy_wrath";
-      action_list_str += "/consecration,if=mana>16000";  // Consecration is expensive,
only use if we have plenty of mana
+      if ( ! util_t::str_compare_ci( sim -> fight_style, "Ultraxion" ) )
+      {
+        action_list_str += "/consecration,if=mana>16000";  // Consecration is expensive,
only use if we have plenty of mana
+      }
       action_list_str += "/divine_plea";
     }
     break;

Reported by coleb2 on 2012-01-08 00:34:46

navv1234 commented 9 years ago
Sadly, your patch will only work with the TCI and only if they define fight_style before
importing. It won't work in the GUI.

It is possible to get it to work in the GUI, but it requires reworking of how parse_fight_style()
is implemented currently. Also, it should be done correctly to allow for a check in
Hand of Guldan to not cast the debuff, since that also doesn't trigger on Ultraxion.
That sounds easy enough, but it means no string comparisons, so other variables will
need to be created/set.

You or another developer are welcome to make the changes in order to get this to work,
but I don't see the point, when it only requires a little thought from the user to
ensure the action list they're using is correct.

Reported by swbusche on 2012-01-08 01:55:23

navv1234 commented 9 years ago
I think we should solve this issue, but I don't think the posted patch is the correct
way to do it. There are other effects that do not work on Ultraxion but which aren't
simple spells triggered by the user, but rather side effects of other spells which
the user is still going to use. An example is the ground effect left behind by Hand
of Gul'dan - this effect applies a debuff to the target which is never applied to Ultraxion.
A warlock fighting Ultraxion will obviously still cast HoG, but his pet will not gain
the benefit of the debuff.

The proper solution was discussed in IRC the other day, and involves a "flying" buff
or debuff on the target, overriding assess_damage or similar methods for the relevant
spells to check for saidebuff, as well as providing access to this debuff through the
action expression system. Finally, some way of specifying that a target is flying needs
to be added - it could be a simple target-level boolean option, but the better solution
is probably a raid event, because that would let us simulate encounters where the target
is only flying part of the time, like Atramedes.

Reported by jon@valvatne.com on 2012-01-08 07:14:38

navv1234 commented 9 years ago
First step is implemented in r10804 and accomplishes the same thing the patch posted
above does.

Next steps are to ensure the ground effect spells don't do damage to a flying target
if they were to be cast for some reason, and also to enure the HoG debuff is not applied
to flying targets.

Reported by jon@valvatne.com on 2012-01-08 09:43:31

navv1234 commented 9 years ago
This issue was closed by revision r10805.

Reported by jon@valvatne.com on 2012-01-08 10:22:25