From f98983e5ad5d4e81955b2356a3bab18362724355 Mon Sep 17 00:00:00 2001
From: John Bytheway <jbytheway@gmail.com>
Date: Tue, 10 Dec 2019 21:34:23 -0500
Subject: [PATCH] Improved cycle finding algorithm

Use a O(V+E) cycle-finding algorithm, and factor it out appropriately to
clarify the code.
---
 src/cata_algo.h           | 80 +++++++++++++++++++++++++++++++++++++++
 src/recipe_dictionary.cpp | 50 ++++++------------------
 2 files changed, 92 insertions(+), 38 deletions(-)

diff --git a/src/cata_algo.h b/src/cata_algo.h
index a93f27ed18cbf..ef284136f9c87 100644
--- a/src/cata_algo.h
+++ b/src/cata_algo.h
@@ -3,6 +3,10 @@
 #define CATA_ALGO_H
 
 #include <algorithm>
+#include <cassert>
+#include <stack>
+#include <unordered_map>
+#include <unordered_set>
 #include <vector>
 
 namespace cata
@@ -41,6 +45,82 @@ void sort_by_rating( Iterator begin, Iterator end, RatingFunction rating_func )
     } );
 }
 
+// Implementation detail of below find_cycles
+// This explores one branch of the given graph depth-first
+template<typename T>
+void find_cycles_impl(
+    const std::unordered_map<T, std::vector<T>> &edges,
+    const T &v,
+    std::unordered_set<T> &visited,
+    std::unordered_map<T, T> &on_current_branch,
+    std::vector<std::vector<T>> &result )
+{
+    bool new_vertex = visited.insert( v ).second;
+
+    if( !new_vertex ) {
+        return;
+    }
+    auto it = edges.find( v );
+    if( it == edges.end() ) {
+        return;
+    }
+
+    for( const T &next_v : it->second ) {
+        if( next_v == v ) {
+            // Trivial self-loop
+            result.push_back( { v } );
+            continue;
+        }
+        auto previous_match = on_current_branch.find( next_v );
+        if( previous_match != on_current_branch.end() ) {
+            // We have looped back to somewhere along the branch we took to
+            // reach this vertex, so reconstruct the loop and save it.
+            std::vector<T> loop;
+            T on_path = v;
+            while( true ) {
+                loop.push_back( on_path );
+                if( on_path == next_v ) {
+                    break;
+                }
+                on_path = on_current_branch[on_path];
+            }
+            std::reverse( loop.begin(), loop.end() );
+            result.push_back( loop );
+        } else {
+            on_current_branch.emplace( next_v, v );
+            find_cycles_impl( edges, next_v, visited, on_current_branch, result );
+            on_current_branch.erase( next_v );
+        }
+    }
+}
+
+// Find and return a list of all cycles in a directed graph.
+// Each T defines a vertex.
+// For a vertex a, edges[a] is a list of all the vertices connected by edges
+// from a.
+// It is acceptable for some vertex keys to be missing from the edges map, if
+// those vertices have no out-edges.
+// Complexity should be O(V+E)
+// Based on https://www.geeksforgeeks.org/detect-cycle-in-a-graph/
+template<typename T>
+std::vector<std::vector<T>> find_cycles( const std::unordered_map<T, std::vector<T>> &edges )
+{
+    std::unordered_set<T> visited;
+    std::unordered_map<T, T> on_current_branch;
+    std::vector<std::vector<T>> result;
+
+    for( const auto &p : edges ) {
+        const T &root = p.first;
+
+        on_current_branch.emplace( root, root );
+        find_cycles_impl( edges, root, visited, on_current_branch, result );
+        on_current_branch.erase( root );
+        assert( on_current_branch.empty() );
+    }
+
+    return result;
+}
+
 } // namespace cata
 
 #endif // CATA_ALGO_H
diff --git a/src/recipe_dictionary.cpp b/src/recipe_dictionary.cpp
index 71539f37f07ef..68efd587926c7 100644
--- a/src/recipe_dictionary.cpp
+++ b/src/recipe_dictionary.cpp
@@ -3,9 +3,9 @@
 #include <algorithm>
 #include <iterator>
 #include <memory>
-#include <stack>
 #include <utility>
 
+#include "cata_algo.h"
 #include "cata_utility.h"
 #include "init.h"
 #include "item.h"
@@ -391,44 +391,18 @@ void recipe_dictionary::find_items_on_loops()
     }
 
     // Now check that graph for loops
-    for( const auto &p : potential_components_of ) {
-        const itype_id &root = p.first;
-        std::unordered_map<itype_id, itype_id> reachable_via;
-        std::stack<itype_id, std::vector<itype_id>> to_check;
-        to_check.push( root );
-        while( !to_check.empty() && !recipe_dict.items_on_loops.count( root ) ) {
-            itype_id next = to_check.top();
-            to_check.pop();
-            auto it = potential_components_of.find( next );
-            if( it == potential_components_of.end() ) {
-                continue;
-            }
-            for( const itype_id &potential_component : it->second ) {
-                if( potential_component == root ) {
-                    std::string error_message =
-                        "loop in comestible recipes detected: " + potential_component;
-                    itype_id on_path = next;
-                    while( true ) {
-                        error_message += " -> " + on_path;
-                        items_on_loops.insert( on_path );
-                        if( on_path == root ) {
-                            break;
-                        }
-                        on_path = reachable_via[on_path];
-                    }
-                    error_message += ".  Such loops can be broken by either removing or altering "
-                                     "recipes or marking one of the items involved with the NUTRIENT_OVERRIDE "
-                                     "flag";
-                    debugmsg( error_message );
-                    break;
-                } else {
-                    bool inserted = reachable_via.emplace( potential_component, next ).second;
-                    if( inserted ) {
-                        to_check.push( potential_component );
-                    }
-                }
-            }
+    std::vector<std::vector<itype_id>> loops = cata::find_cycles( potential_components_of );
+    for( const std::vector<itype_id> &loop : loops ) {
+        std::string error_message =
+            "loop in comestible recipes detected: " + loop.back();
+        for( const itype_id &i : loop ) {
+            error_message += " -> " + i;
+            items_on_loops.insert( i );
         }
+        error_message += ".  Such loops can be broken by either removing or altering "
+                         "recipes or marking one of the items involved with the NUTRIENT_OVERRIDE "
+                         "flag";
+        debugmsg( error_message );
     }
 }