From cdd6a56bb0ccdabb5f6c52b1a85601431cd5afa3 Mon Sep 17 00:00:00 2001
From: gammafunk <gammafunk@gmail.com>
Date: Sat, 5 Oct 2013 20:22:38 -0500
Subject: [PATCH 1/2] Some cleanups and simplifications for jump-attack code.

Remove the allow_harmful argument from ::has_additional_landing_sites() and make
_check_moveto_dangerous back into a static function now that we don't need to
call it directly.  The allow_harmful argument allowed direction_chooser to only
find a default jump-attack target with landing sites that are all non-harmful
and within-conduct, but we have proper prompting to the player for such sites
now.  This is consistent with the targetting behaviour for other
spells/abilities.

Also cleanup some comments and simplify related code based on previous fixes.
---
 crawl-ref/source/directn.cc | 14 +++-----------
 crawl-ref/source/player.cc  | 30 ++++++++++++++----------------
 crawl-ref/source/player.h   |  2 --
 crawl-ref/source/target.cc  | 34 ++++++++++------------------------
 crawl-ref/source/target.h   |  4 ++--
 5 files changed, 29 insertions(+), 55 deletions(-)

diff --git a/crawl-ref/source/directn.cc b/crawl-ref/source/directn.cc
index 2c686ba..3b55243 100644
--- a/crawl-ref/source/directn.cc
+++ b/crawl-ref/source/directn.cc
@@ -1142,15 +1142,9 @@ const coord_def& direction_chooser::target() const
 
 void direction_chooser::set_target(const coord_def& new_target)
 {
-    set<coord_def>::const_iterator site;
 
     if (restricts == DIR_JUMP)
-    {
-        if (hitfunc->has_additional_sites(new_target, true))
-            valid_jump = true;
-        else
-            valid_jump = false;
-    }
+        valid_jump = hitfunc->has_additional_sites(new_target);
     moves.target = new_target;
 }
 
@@ -1321,10 +1315,8 @@ bool direction_chooser::select(bool allow_out_of_range, bool endpoint)
     const monster* mons = monster_at(target());
 
     if (restricts == DIR_JUMP && !valid_jump)
-    {
-        mpr("There is no safe place to jump near there.");
         return false;
-    }
+
     if (!allow_out_of_range && !in_range(target()))
     {
         mpr(hitfunc? hitfunc->why_not : "That is beyond the maximum range.",
@@ -2503,7 +2495,7 @@ static bool _find_jump_attack_mons(const coord_def& where, int mode, bool need_p
     if (where == you.pos())
         return false;
 
-    return hitfunc->has_additional_sites(where, false);
+    return hitfunc->has_additional_sites(where);
 }
 
 
diff --git a/crawl-ref/source/player.cc b/crawl-ref/source/player.cc
index 19eff38..40987e6 100644
--- a/crawl-ref/source/player.cc
+++ b/crawl-ref/source/player.cc
@@ -227,8 +227,9 @@ bool check_moveto_trap(const coord_def& p, const string &move_verb,
     return true;
 }
 
-bool check_moveto_dangerous(const coord_def& p, const string& msg, bool cling,
-                            bool do_prompt)
+static bool _check_moveto_dangerous(const coord_def& p, const string& msg,
+                                    bool cling = true)
+
 {
     if (you.can_swim() && feat_is_water(env.grid(p))
         || you.airborne() || cling && you.can_cling_to(p)
@@ -237,20 +238,17 @@ bool check_moveto_dangerous(const coord_def& p, const string& msg, bool cling,
         return true;
     }
 
-    if (do_prompt)
+    if (msg != "")
+        mpr(msg.c_str());
+    else if (you.species == SP_MERFOLK && feat_is_water(env.grid(p)))
+        mpr("You cannot swim in your current form.");
+    else if (you.species == SP_LAVA_ORC && feat_is_lava(env.grid(p))
+             && is_feat_dangerous(env.grid(p)))
     {
-        if (msg != "")
-            mpr(msg.c_str());
-        else if (you.species == SP_MERFOLK && feat_is_water(env.grid(p)))
-            mpr("You cannot swim in your current form.");
-        else if (you.species == SP_LAVA_ORC && feat_is_lava(env.grid(p))
-                 && is_feat_dangerous(env.grid(p)))
-        {
-            mpr("You cannot enter lava in your current form.");
-        }
-        else
-            canned_msg(MSG_UNTHINKING_ACT);
+        mpr("You cannot enter lava in your current form.");
     }
+    else
+        canned_msg(MSG_UNTHINKING_ACT);
     return false;
 }
 
@@ -260,9 +258,9 @@ bool check_moveto_terrain(const coord_def& p, const string &move_verb,
     if (you.is_wall_clinging()
         && (move_verb == "blink" || move_verb == "passwall"))
     {
-        return check_moveto_dangerous(p, msg, false);
+        return _check_moveto_dangerous(p, msg, false);
     }
-    else if (!check_moveto_dangerous(p, msg))
+    else if (!_check_moveto_dangerous(p, msg))
         return false;
 
     if (!need_expiration_warning() && need_expiration_warning(p)
diff --git a/crawl-ref/source/player.h b/crawl-ref/source/player.h
index 6766ac1..62e6ac6 100644
--- a/crawl-ref/source/player.h
+++ b/crawl-ref/source/player.h
@@ -797,8 +797,6 @@ bool check_moveto(const coord_def& p, const string &move_verb = "step",
                   const string &msg = "");
 bool check_moveto_terrain(const coord_def& p, const string &move_verb,
                           const string &msg = "", bool *prompted = nullptr);
-bool check_moveto_dangerous(const coord_def& p, const string& msg = "",
-                            bool cling = true, bool do_prompt = true);
 bool check_moveto_cloud(const coord_def& p, const string &move_verb = "step",
                         bool *prompted = nullptr);
 bool check_moveto_exclusion(const coord_def& p,
diff --git a/crawl-ref/source/target.cc b/crawl-ref/source/target.cc
index 14e8c37..9dbd6df 100644
--- a/crawl-ref/source/target.cc
+++ b/crawl-ref/source/target.cc
@@ -53,7 +53,7 @@ bool targetter::anyone_there(coord_def loc)
     return actor_at(loc);
 }
 
-bool targetter::has_additional_sites(coord_def loc, bool allow_harmful)
+bool targetter::has_additional_sites(coord_def loc)
 {
     return false;
 }
@@ -911,7 +911,7 @@ bool targetter_jump::valid_aim(coord_def a)
         return notify_fail("There's something in the way.");
     else if (!find_ray(agent->pos(), a, ray, opc_solid_see))
         return notify_fail("There's something in the way.");
-    else if (!has_additional_sites(a, true))
+    else if (!has_additional_sites(a))
     {
         switch (no_landing_reason)
         {
@@ -983,7 +983,7 @@ bool targetter_jump::valid_landing(coord_def a, bool check_invis)
         if (act && (!check_invis || agent->can_see(act)))
         {
 
-            // Can't jump over airborn enemies nor gient enemies not in deep
+            // Can't jump over airborn enemies nor giant enemies not in deep
             // water or lava.
             if (act->airborne())
             {
@@ -1013,12 +1013,11 @@ aff_type targetter_jump::is_affected(coord_def loc)
     return aff;
 }
 
-// Handle setting the aim for jump, which is the landing position of the jump.
-// If something unsee either occupies the aim positiion or blocks the jump path,
-// indicate that with jump_is_blocked.
+// If something unseen either occupies the aim position or blocks the jump path,
+// indicate that with jump_is_blocked, but still return true so long there is at
+// least one valid landing position from the player's perspective.
 bool targetter_jump::set_aim(coord_def a)
 {
-    ray_def ray;
     set<coord_def>::const_iterator site;
 
     if (a == origin)
@@ -1028,8 +1027,8 @@ bool targetter_jump::set_aim(coord_def a)
 
     jump_is_blocked = false;
 
-    // Find our set of landing sites, choose one at random, and see if it's
-    // actually blocked.
+    // Find our set of landing sites, choose one at random to be the destination
+    // and see if it's actually blocked.
     set_additional_sites(aim);
     if (additional_sites.size())
     {
@@ -1083,21 +1082,8 @@ void targetter_jump::get_additional_sites(coord_def a)
 }
 
 // See if we can find at least one valid landing position for the given monster.
-// Base on allow_harmful we might care to exclude sites harmful to the player.
-bool targetter_jump::has_additional_sites(coord_def a, bool allow_harmful)
+bool targetter_jump::has_additional_sites(coord_def a)
 {
-    set<coord_def>::const_iterator site;
-
     get_additional_sites(a);
-    if (allow_harmful || !agent->is_player())
-        return temp_sites.size();
-    // Find a valid jump_attack position in the adjacent squares, chosing the
-    // valid position closest to the player.
-    for (site = temp_sites.begin();
-         site != temp_sites.end(); site++)
-    {
-        if (check_moveto_dangerous(*site, "jump", "", false))
-            return true;
-    }
-    return false;
+    return temp_sites.size();
 }
diff --git a/crawl-ref/source/target.h b/crawl-ref/source/target.h
index 17b8acc..489fee3 100644
--- a/crawl-ref/source/target.h
+++ b/crawl-ref/source/target.h
@@ -31,7 +31,7 @@ public:
     virtual bool can_affect_walls();
 
     virtual aff_type is_affected(coord_def loc) = 0;
-    virtual bool has_additional_sites(coord_def a, bool allow_harmful);
+    virtual bool has_additional_sites(coord_def a);
 protected:
     bool anyone_there(coord_def loc);
 };
@@ -209,7 +209,7 @@ public:
     bool set_aim(coord_def a);
     bool jump_is_blocked;
     aff_type is_affected(coord_def loc);
-    bool has_additional_sites(coord_def a, bool allow_harmful);
+    bool has_additional_sites(coord_def a);
     set<coord_def> additional_sites;
     coord_def landing_site;
 private:
-- 
1.7.11.5


From 0cc2cae2174cb30d189ddbc17af1152752067d51 Mon Sep 17 00:00:00 2001
From: gammafunk <gammafunk@gmail.com>
Date: Sat, 5 Oct 2013 21:17:59 -0500
Subject: [PATCH 2/2] Fix jump-attack to not allow forcing targets outside the
 attack range.

Also make shift+dir movement move the cursor instead of trying to
jump-attack in a direction.  This commit also removes a forced redraw
dating from early versions of the ability.
---
 crawl-ref/source/directn.cc | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/crawl-ref/source/directn.cc b/crawl-ref/source/directn.cc
index 3b55243..bbac530 100644
--- a/crawl-ref/source/directn.cc
+++ b/crawl-ref/source/directn.cc
@@ -375,15 +375,13 @@ void direction_chooser::print_key_hints() const
             prompt += hint_string;
             break;
         case DIR_TARGET:
+        case DIR_JUMP:
             prompt += ", Dir - move target cursor";
             prompt += hint_string;
             break;
         case DIR_DIR:
         case DIR_TARGET_OBJECT:
             break;
-        case DIR_JUMP:
-            prompt += hint_string;
-            break;
         }
     }
 
@@ -1142,7 +1140,6 @@ const coord_def& direction_chooser::target() const
 
 void direction_chooser::set_target(const coord_def& new_target)
 {
-
     if (restricts == DIR_JUMP)
         valid_jump = hitfunc->has_additional_sites(new_target);
     moves.target = new_target;
@@ -1317,7 +1314,7 @@ bool direction_chooser::select(bool allow_out_of_range, bool endpoint)
     if (restricts == DIR_JUMP && !valid_jump)
         return false;
 
-    if (!allow_out_of_range && !in_range(target()))
+    if ((restricts == DIR_JUMP || !allow_out_of_range) && !in_range(target()))
     {
         mpr(hitfunc? hitfunc->why_not : "That is beyond the maximum range.",
             MSGCH_EXAMINE_FILTER);
@@ -1587,7 +1584,7 @@ void direction_chooser::reinitialize_move_flags()
 // Returns true if we've completed targetting.
 bool direction_chooser::select_compass_direction(const coord_def& delta)
 {
-    if (restricts != DIR_TARGET)
+    if (restricts != DIR_TARGET && restricts != DIR_JUMP)
     {
         // A direction is allowed, and we've selected it.
         moves.delta    = delta;
@@ -2021,7 +2018,7 @@ bool direction_chooser::do_main_loop()
     }
 
     // Redraw whatever is necessary.
-    if (restricts == DIR_JUMP || old_target != target())
+    if (old_target != target())
     {
         have_beam = show_beam && find_ray(you.pos(), target(), beam,
                                           opc_solid_see, BDS_DEFAULT);
-- 
1.7.11.5