Crushing Leap doesn't deal damage

gisaku33 commented 2 years ago

Describe the bug

The Crushing Leap spell from the Crushing Leap mutation doesn't deal damage when either aiming at an enemy or past one, instead you land in front of them with no other effect.

Steps to reproduce

Make a character with the Crushing Leap mutation.
Cast the Crushing Leap ability, either aimed directly on or past an enemy.
Wish that frogs had tear ducts so you could cry.

Expected behavior

I expected the Crushing Leap to Crush when Leaping.

Screenshots

Versions and configuration

OS: Windows
- OS Version: 10.0 2009
Game Version: 8b7a449 [64-bit]
Graphics Version: Tiles
Game Language: System language []
Mods loaded: [ Dark Days Ahead [dda], Disable NPC Needs [no_npc_food], No Fungal Growth [no_fungal_growth], Bionic Professions [package_bionic_professions], Blaze Industries [blazeindustries], C.R.I.T Expansion Mod [crt_expansion], Magiclysm [magiclysm], Mythical Martial Arts [MMA], Stats Through Kills [stats_through_kills], SpeedyDex [speedydex] ]

Additional context

No response

GuardianDll commented 2 years ago

To fix it, someone need: first - add aoe_min and aoe_max to spell (5 is the smallest valid number that works in my tests, anything bigger would be too big) second - change how dash effect interact, when you target some critter - right now it jumps to this monster, but deal no damage (jumping to point before the target works fine, which is odd)

GuardianDll commented 2 years ago

Repost it here from #60704, so it wont be missed:

Just tested it, and leap works even worse than i thought instead of dealing damage for targets after the jump (i dont really understand how AoE field interacts with leap, but it seems really smart to work in that way) it deals damage to the target before the jump. its hard to explain, but if our character jump on a distance of 1, and aoe is 6, it means it can jump on distance 1, and deal damage to 5 (6 aoe minus 1) targets. But if it tries to jump on distance 6, it wont be able to deal any damage at all and if there would be an aoe 2, it means it can jump on distance 1 and attack 1 monster ahead (2 aoe minus 1), or jump on distance 6 and deal no damage at all tldr: it doesnt work. id prefer to close this pr at all, and tomorrow ill redo the damage leap spells to be two action spell - first is the leap, and second is subspell, that will deal some damage around the character after the jump.

Short answer: leap effect is awful and must be burned in fire long answer: leap spell has some extremely weird behaviour, which prevent using it in any damage dealing process. i cant even desribe the bug itself, but short demo may explain it for a bit https://imgur.com/a/ShtFrsY

andrei8l had posted this quick and dirty dif in devcord for some ideas as to how to fix this.

index 091a9f2464..240b9e1d8d 100644
--- a/src/magic.cpp
+++ b/src/magic.cpp
@@ -1528,6 +1528,7 @@ void spell::cast_spell_effect( Creature &source, const tripoint &target ) const

 void spell::cast_all_effects( Creature &source, const tripoint &target ) const
 {
+    tripoint_abs_ms const target_abs = get_map().getglobal( target );
     if( has_flag( spell_flag::WONDER ) ) {
         const auto iter = type->additional_spells.begin();
         for( int num_spells = std::abs( damage() ); num_spells > 0; num_spells-- ) {
@@ -1543,26 +1544,29 @@ void spell::cast_all_effects( Creature &source, const tripoint &target ) const
             // if a message is added to the casting spell, it will be sent as well.
             source.add_msg_if_player( sp.message() );

+            tripoint const target_local = get_map().getlocal( target_abs );
             if( sp.has_flag( spell_flag::RANDOM_TARGET ) ) {
                 if( const cata::optional<tripoint> new_target = sp.random_valid_target( source,
-                        _self ? source.pos() : target ) ) {
+                        _self ? source.pos() : target_local ) ) {
                     sp.cast_all_effects( source, *new_target );
                 }
             } else {
                 if( _self ) {
                     sp.cast_all_effects( source, source.pos() );
                 } else {
-                    sp.cast_all_effects( source, target );
+                    sp.cast_all_effects( source, target_local );
                 }
             }
         }
     } else {
         if( has_flag( spell_flag::EXTRA_EFFECTS_FIRST ) ) {
             cast_extra_spell_effects( source, target );
-            cast_spell_effect( source, target );
+            tripoint const target_local = get_map().getlocal( target_abs );
+            cast_spell_effect( source, target_local );
         } else {
             cast_spell_effect( source, target );
-            cast_extra_spell_effects( source, target );
+            tripoint const target_local = get_map().getlocal( target_abs );
+            cast_extra_spell_effects( source, target_local );
         }
     }
 }

upd: i compiled said code, but it doesn't resolve the bug in any way

I-am-Erk commented 1 year ago

I am moving this from release blockers as it appears to be a fairly complex fix, and while we definitely want it fixed, I don't think we can let it delay stable further.

however, absolutely fix it if you think you can, this is a fairly significant bug.

CleverRaven / Cataclysm-DDA