Optimize Creature Iteration 2: Electric Boogaloo

src/creature_tracker.cpp

@@ -7,18 +7,18 @@
 #include "avatar.h"
 #include "cata_assert.h"
 #include "debug.h"
+#include "flood_fill.h"
 #include "map.h"
 #include "mongroup.h"
 #include "monster.h"
 #include "mtype.h"
 #include "npc.h"
 #include "string_formatter.h"
+#include "submap.h"
 #include "type_id.h"
 
 static const efftype_id effect_ridden( "ridden" );
 
-static const mfaction_str_id monfaction_player( "player" );
-
 static const mon_flag_str_id mon_flag_VERMIN( "VERMIN" );
 
 #define dbg(x) DebugLog((x),D_GAME) << __FILE__ << ":" << __LINE__ << ": "
@@ -93,23 +93,9 @@ bool creature_tracker::add( const shared_ptr_fast<monster> &critter_ptr )
 
     monsters_list.emplace_back( critter_ptr );
     monsters_by_location[critter.get_location()] = critter_ptr;
-    add_to_faction_map( critter_ptr );
     return true;
 }
 
-void creature_tracker::add_to_faction_map( const shared_ptr_fast<monster> &critter_ptr )
-{
-    cata_assert( critter_ptr );
-    monster &critter = *critter_ptr;
-
-    // Only 1 faction per mon at the moment.
-    if( critter.friendly == 0 ) {
-        monster_faction_map_[critter.faction][critter_ptr->get_location().z()].insert( critter_ptr );
-    } else {
-        monster_faction_map_[monfaction_player][critter_ptr->get_location().z()].insert( critter_ptr );
-    }
-}
-
 size_t creature_tracker::size() const
 {
     return monsters_list.size();
@@ -185,36 +171,27 @@ void creature_tracker::remove( const monster &critter )
         return;
     }
 
-    for( auto &pair : monster_faction_map_ ) {
-        const int zpos = critter.pos().z;
-        const auto fac_iter = pair.second[zpos].find( *iter );
-        if( fac_iter != pair.second[zpos].end() ) {
-            // Need to do this manually because the shared pointer containing critter is kept valid
-            // within removed_ and so the weak pointer in monster_faction_map_ is also valid.
-            pair.second[zpos].erase( fac_iter );
-            break;
-        }
-    }
     remove_from_location_map( critter );
-    removed_.push_back( *iter );
+    removed_.emplace( iter->get() );
+    removed_this_turn_.emplace( *iter );
     monsters_list.erase( iter );
 }
 
 void creature_tracker::clear()
 {
     monsters_list.clear();
     monsters_by_location.clear();
-    monster_faction_map_.clear();
     removed_.clear();
+    removed_this_turn_.clear();
+    creatures_by_zone_and_faction_.clear();
+    invalidate_reachability_cache();
 }
 
 void creature_tracker::rebuild_cache()
 {
     monsters_by_location.clear();
-    monster_faction_map_.clear();
     for( const shared_ptr_fast<monster> &mon_ptr : monsters_list ) {
         monsters_by_location[mon_ptr->get_location()] = mon_ptr;
-        add_to_faction_map( mon_ptr );
     }
 }
 
@@ -224,6 +201,10 @@ void creature_tracker::swap_positions( monster &first, monster &second )
         return;
     }
 
+    if( first.get_reachable_zone() != second.get_reachable_zone() ) {
+        invalidate_reachability_cache();
+    }
+
     // Either of them may be invalid!
     const auto first_iter = monsters_by_location.find( first.get_location() );
     const auto second_iter = monsters_by_location.find( second.get_location() );
@@ -283,16 +264,16 @@ void creature_tracker::remove_dead()
 {
     // Can't use game::all_monsters() as it would not contain *dead* monsters.
     for( auto iter = monsters_list.begin(); iter != monsters_list.end(); ) {
-        const monster &critter = **iter;
-        if( critter.is_dead() ) {
-            remove_from_location_map( critter );
+        monster *const critter = iter->get();
+        if( critter->is_dead() ) {
+            remove_from_location_map( *critter );
+            removed_.insert( critter );
             iter = monsters_list.erase( iter );
         } else {
             ++iter;
         }
     }
-
-    removed_.clear();
+    removed_this_turn_.clear();
 }
 
 template<typename T>
@@ -341,6 +322,86 @@ T *creature_tracker::creature_at( const tripoint_abs_ms &p, bool allow_hallucina
     return nullptr;
 }
 
+/** This is lazily evaluated on demand. Each creature in a zone is visited
+ * as it flood fills, then the zone number is incremented. At the end all creatures in
+ * the same zone will have the same zone number assigned, which can be used to have creatures in
+ * different zones ignore each other very cheaply.
+ */
+void creature_tracker::flood_fill_zone( const Creature &origin )
+{
+    if( dirty_ ) {
+        creatures_by_zone_and_faction_.clear();
+        removed_.clear();
+        zone_tick_ = zone_tick_ > 0 ? -1 : 1;
+        zone_number_ = 1;
+        dirty_ = false;
+    }
+
+    // This check insures we only flood fill when the target monster has an uninitialized zone,
+    // or if it has a zone from last turn.  In other words it only triggers on
+    // the first monster in a zone each turn. We can detect this because the sign
+    // of the zone numbers changes on every invalidation.
+    int old_zone = origin.get_reachable_zone();
+    // Compare with zone_tick == old_zone && old_zone != 0
+    if( old_zone * zone_tick_ > 0 ) {
+        return;
+    }
+
+    map &map = get_map();
+    ff::flood_fill_visit_10_connected( origin.pos_bub(),
+    [&map]( const tripoint_bub_ms & loc, int direction ) {
+        if( direction == 0 ) {
+            return map.inbounds( loc ) && ( map.is_transparent_wo_fields( loc.raw() ) ||
+                                            map.passable( loc ) );
+        }
+        if( direction == 1 ) {
+            const maptile &up = map.maptile_at( loc );
+            const ter_t &up_ter = up.get_ter_t();
+            if( up_ter.id.is_null() ) {
+                return false;
+            }
+            if( ( ( up_ter.movecost != 0 && up.get_furn_t().movecost >= 0 ) ||
+                  map.is_transparent_wo_fields( loc.raw() ) ) &&
+                ( up_ter.has_flag( ter_furn_flag::TFLAG_NO_FLOOR ) ||
+                  up_ter.has_flag( ter_furn_flag::TFLAG_GOES_DOWN ) ) ) {
+                return true;
+            }
+        }
+        if( direction == -1 ) {
+            const maptile &up = map.maptile_at( loc + tripoint_above );
+            const ter_t &up_ter = up.get_ter_t();
+            if( up_ter.id.is_null() ) {
+                return false;
+            }
+            const maptile &down = map.maptile_at( loc );
+            const ter_t &down_ter = up.get_ter_t();
+            if( down_ter.id.is_null() ) {
+                return false;
+            }
+            if( ( ( down_ter.movecost != 0 && down.get_furn_t().movecost >= 0 ) ||
+                  map.is_transparent_wo_fields( loc.raw() ) ) &&
+                ( up_ter.has_flag( ter_furn_flag::TFLAG_NO_FLOOR ) ||
+                  up_ter.has_flag( ter_furn_flag::TFLAG_GOES_DOWN ) ) ) {
+                return true;
+            }
+        }
+        return false;
+    },
+    [this]( const tripoint_bub_ms & loc ) {
+        Creature *creature = this->creature_at<Creature>( loc );
+        if( creature ) {
+            const int n = zone_number_ * zone_tick_;
+            creatures_by_zone_and_faction_[n][creature->get_monster_faction()].push_back( creature );
+            creature->set_reachable_zone( n );
+        }
+    } );
+    if( zone_number_ == std::numeric_limits<int>::max() ) {
+        zone_number_ = 1;
+    } else {
+        zone_number_++;
+    }
+}
+
 template<typename T>
 const T *creature_tracker::creature_at( const tripoint &p, bool allow_hallucination ) const
 {

src/creature_tracker.h

@@ -11,11 +11,11 @@
 #include <vector>
 
 #include "coordinates.h"
+#include "creature.h"
 #include "memory_fast.h"
 #include "point.h"
 #include "type_id.h"
 
-class Creature;
 class game;
 class JsonArray;
 class JsonOut;
@@ -24,29 +24,6 @@ class npc;
 
 class creature_tracker
 {
-        friend game;
-    private:
-
-        void add_to_faction_map( const shared_ptr_fast<monster> &critter );
-
-        class weak_ptr_comparator
-        {
-            public:
-                bool operator()( const weak_ptr_fast<monster> &lhs,
-                                 const weak_ptr_fast<monster> &rhs ) const {
-                    return lhs.lock().get() < rhs.lock().get();
-                }
-        };
-
-        using MonstersByZ = std::map<int, std::set<weak_ptr_fast<monster>, weak_ptr_comparator>>;
-        std::unordered_map<mfaction_id, MonstersByZ> monster_faction_map_; // NOLINT(cata-serialize)
-
-        /**
-         * Creatures that get removed via @ref remove are stored here until the end of the turn.
-         * This keeps the objects valid and they can still be accessed instead of causing UB.
-         */
-        std::vector<shared_ptr_fast<monster>> removed_; // NOLINT(cata-serialize)
-
     public:
         creature_tracker();
         ~creature_tracker();
@@ -56,6 +33,44 @@ class creature_tracker
          * Dead monsters are ignored and not returned.
          */
         shared_ptr_fast<monster> find( const tripoint_abs_ms &pos ) const;
+
+        /**
+         * Returns the reachable creature matching the given predicate.
+         *  - CreaturePredicateFn: bool(Creature*)
+         * If there is no creature, it returns a `nullptr`.
+         * Dead monsters are ignored and not returned.
+         */
+        template <typename PredicateFn>
+        Creature *find_reachable( const Creature &origin, PredicateFn &&predicate_fn );
+
+        /**
+         * Returns the reachable creature matching the given predicates.
+         *  - FactionPredicateFn: bool(const mfaction_id&)
+         *  - CreaturePredicateFn: bool(Creature*)
+         * If there is no creature, it returns a `nullptr`.
+         * Dead monsters are ignored and not returned.
+         */
+        template <typename FactionPredicateFn, typename CreaturePredicateFn>
+        Creature *find_reachable( const Creature &origin, FactionPredicateFn &&faction_fn,
+                                  CreaturePredicateFn &&creature_fn );
+        /**
+         * Visits all reachable creatures using the given functor.
+         *  - VisitFn: void(Creature*)
+         * Dead monsters are ignored and not visited.
+         */
+        template <typename VisitFn>
+        void for_each_reachable( const Creature &origin, VisitFn &&visit_fn );
+
+        /**
+         * Visits all reachable creatures using the given functor matching the given predicate.
+         *  - FactionPredicateFn: bool(const mfaction_id&)
+         *  - CreatureVisitFn: void(Creature*)
+         * Dead monsters are ignored and not visited.
+         */
+        template <typename FactionPredicateFn, typename CreatureVisitFn>
+        void for_each_reachable( const Creature &origin, FactionPredicateFn &&faction_fn,
+                                 CreatureVisitFn &&creature_fn );
+
         /**
          * Returns a temporary id of the given monster (which must exist in the tracker).
          * The id is valid until monsters are added or removed from the tracker.
@@ -118,20 +133,99 @@ class creature_tracker
         void serialize( JsonOut &jsout ) const;
         void deserialize( const JsonArray &ja );
 
-        const decltype( monster_faction_map_ ) &factions() const {
-            return monster_faction_map_;
+        // This must be called when persistent visibility from terrain or furniture changes
+        // (this excludes vehicles and fields) or when persistent traversability changes,
+        // which means walls and floors.
+        void invalidate_reachability_cache() {
+            dirty_ = true;
         }
 
     private:
+        /** Remove the monsters entry in @ref monsters_by_location */
+        void remove_from_location_map( const monster &critter );
+
+        void flood_fill_zone( const Creature &origin );
+
+        void rebuild_cache();
+
         std::list<shared_ptr_fast<npc>> active_npc; // NOLINT(cata-serialize)
         std::vector<shared_ptr_fast<monster>> monsters_list;
-        void rebuild_cache();
         // NOLINTNEXTLINE(cata-serialize)
         std::unordered_map<tripoint_abs_ms, shared_ptr_fast<monster>> monsters_by_location;
-        /** Remove the monsters entry in @ref monsters_by_location */
-        void remove_from_location_map( const monster &critter );
+
+        /**
+         * Creatures that get removed via @ref remove are stored here until the end of the turn.
+         * This keeps the objects valid and they can still be accessed instead of causing UB.
+         */
+        std::unordered_set<shared_ptr_fast<monster>> removed_this_turn_;  // NOLINT(cata-serialize)
+
+        // Tracks the dirtiness of the visitable zones cache. This must be flipped when
+        // persistent visibility from terrain or furniture changes (this excludes vehicles and fields)
+        // or when persistent traversability changes, which means walls and floors.
+        bool dirty_ = true;  // NOLINT(cata-serialize)
+        int zone_tick_ = 1;  // NOLINT(cata-serialize)
+        int zone_number_ = 0;  // NOLINT(cata-serialize)
+        std::unordered_map<int, std::unordered_map<mfaction_id, std::vector<Creature *>>>
+        creatures_by_zone_and_faction_;  // NOLINT(cata-serialize)
+        std::unordered_set<Creature *> removed_;  // NOLINT(cata-serialize)
+
+        friend game;
 };
 
 creature_tracker &get_creature_tracker();
 
+// Implementation Details
+
+template <typename PredicateFn>
+Creature *creature_tracker::find_reachable( const Creature &origin, PredicateFn &&predicate_fn )
+{
+    return find_reachable( origin, []( const mfaction_id & ) {
+        return true;
+    }, std::forward<PredicateFn>( predicate_fn ) );
+}
+
+template <typename FactionPredicateFn, typename CreaturePredicateFn>
+Creature *creature_tracker::find_reachable( const Creature &origin, FactionPredicateFn &&faction_fn,
+        CreaturePredicateFn &&creature_fn )
+{
+    flood_fill_zone( origin );
+
+    const auto map_iter = creatures_by_zone_and_faction_.find( origin.get_reachable_zone() );
+    if( map_iter != creatures_by_zone_and_faction_.end() ) {
+        for( const auto& [faction, creatures] : map_iter->second ) {
+            if( !faction_fn( faction ) ) {
+                continue;
+            }
+            for( Creature *other : creatures ) {
+                if( removed_.count( other ) == 0 ) {
+                    if( creature_fn( other ) ) {
+                        return other;
+                    }
+                }
+            }
+        }
+    }
+    return nullptr;
+}
+
+template <typename VisitFn>
+void creature_tracker::for_each_reachable( const Creature &origin, VisitFn &&visit_fn )
+{
+    find_reachable( origin, [&visit_fn]( Creature * other ) {
+        visit_fn( other );
+        return false;
+    } );
+}
+
+template <typename FactionPredicateFn, typename CreatureVisitFn>
+void creature_tracker::for_each_reachable( const Creature &origin, FactionPredicateFn &&faction_fn,
+        CreatureVisitFn &&creature_fn )
+{
+    find_reachable( origin, std::forward<FactionPredicateFn>( faction_fn ), [&creature_fn](
+    Creature * other ) {
+        creature_fn( other );
+        return false;
+    } );
+}
+
 #endif // CATA_SRC_CREATURE_TRACKER_H