From 01a8d44bce65e23bd6a1d41b4c6d4d90285bb53b Mon Sep 17 00:00:00 2001
From: Jeff Young <jeff@rokeby.ie>
Date: Tue, 1 Jul 2025 11:03:07 -0600
Subject: [PATCH] Further simplify scoring logic.

Do away with special cases for showing.
Do away with resetting the score separately.
All methods are subtractive (logical AND).

Fixes https://gitlab.com/kicad/code/kicad/-/issues/21220
---
 common/lib_tree_model.cpp         | 59 +++++++++++++++----------------
 common/lib_tree_model_adapter.cpp |  2 --
 include/lib_tree_model.h          |  8 -----
 3 files changed, 29 insertions(+), 40 deletions(-)

diff --git a/common/lib_tree_model.cpp b/common/lib_tree_model.cpp
index 7f87ee0770..c2e4040099 100644
--- a/common/lib_tree_model.cpp
+++ b/common/lib_tree_model.cpp
@@ -30,15 +30,6 @@
 
 
 
-void LIB_TREE_NODE::ResetScore()
-{
-    for( std::unique_ptr<LIB_TREE_NODE>& child: m_Children )
-        child->ResetScore();
-
-    m_Score = 0;
-}
-
-
 void LIB_TREE_NODE::AssignIntrinsicRanks( bool presorted )
 {
     std::vector<LIB_TREE_NODE*> sort_buf;
@@ -174,17 +165,14 @@ LIB_TREE_NODE_UNIT::LIB_TREE_NODE_UNIT( LIB_TREE_NODE* aParent, LIB_TREE_ITEM* a
 void LIB_TREE_NODE_UNIT::UpdateScore( const std::vector<std::unique_ptr<EDA_COMBINED_MATCHER>>& aMatchers,
                                       std::function<bool( LIB_TREE_NODE& aNode )>* aFilter )
 {
+    m_Score = 1;
+
     // aMatchers test results are inherited from parent
     if( !aMatchers.empty() )
         m_Score = m_Parent->m_Score;
 
-    // aFilter test is subtractive
     if( aFilter && !(*aFilter)(*this) )
         m_Score = 0;
-
-    // show all nodes if no search/filter/etc. criteria are given
-    if( aMatchers.empty() && ( !aFilter || (*aFilter)(*this) ) )
-        m_Score = 1;
 }
 
 
@@ -246,19 +234,24 @@ void LIB_TREE_NODE_ITEM::Update( LIB_TREE_ITEM* aItem )
 void LIB_TREE_NODE_ITEM::UpdateScore( const std::vector<std::unique_ptr<EDA_COMBINED_MATCHER>>& aMatchers,
                                       std::function<bool( LIB_TREE_NODE& aNode )>* aFilter )
 {
-    m_Score = 0;
+    m_Score = 1;
 
     for( const std::unique_ptr<EDA_COMBINED_MATCHER>& matcher : aMatchers )
-        m_Score += matcher->ScoreTerms( m_SearchTerms );
+    {
+        int score = matcher->ScoreTerms( m_SearchTerms );
+
+        if( score == 0 )
+        {
+            m_Score = 0;
+            break;
+        }
+
+        m_Score += score;
+    }
 
-    // aFilter test is subtractive
     if( aFilter && !(*aFilter)(*this) )
         m_Score = 0;
 
-    // show all nodes if no search/filter/etc. criteria are given
-    if( aMatchers.empty() && ( !aFilter || (*aFilter)(*this) ) )
-        m_Score = 1;
-
     for( std::unique_ptr<LIB_TREE_NODE>& child: m_Children )
         child->UpdateScore( aMatchers, aFilter );
 }
@@ -290,25 +283,31 @@ void LIB_TREE_NODE_LIBRARY::UpdateScore( const std::vector<std::unique_ptr<EDA_C
 {
     if( m_Children.empty() )
     {
+        m_Score = 1;
+
         for( const std::unique_ptr<EDA_COMBINED_MATCHER>& matcher : aMatchers )
-            m_Score += matcher->ScoreTerms( m_SearchTerms );
+        {
+            int score = matcher->ScoreTerms( m_SearchTerms );
+
+            if( score == 0 )
+            {
+                m_Score = 0;
+                break;
+            }
+
+            m_Score += score;
+        }
     }
     else
     {
-        int maxChildScore = 0;
+        m_Score = 0;
 
         for( std::unique_ptr<LIB_TREE_NODE>& child: m_Children )
         {
             child->UpdateScore( aMatchers, aFilter );
-            maxChildScore = std::max( maxChildScore, child->m_Score );
+            m_Score = std::max( m_Score, child->m_Score );
         }
-
-        m_Score = std::max( m_Score, maxChildScore );
     }
-
-    // show all nodes if no search/filter/etc. criteria are given
-    if( m_Children.empty() && aMatchers.empty() && ( !aFilter || (*aFilter)(*this) ) )
-        m_Score = 1;
 }
 
 
diff --git a/common/lib_tree_model_adapter.cpp b/common/lib_tree_model_adapter.cpp
index f10b6c3982..778fdcbede 100644
--- a/common/lib_tree_model_adapter.cpp
+++ b/common/lib_tree_model_adapter.cpp
@@ -286,8 +286,6 @@ void LIB_TREE_MODEL_ADAPTER::UpdateSearchString( const wxString& aSearch, bool a
         Freeze();
         BeforeReset();
 
-        m_tree.ResetScore();
-
         // Don't cause KiCad to hang if someone accidentally pastes the PCB or schematic into
         // the search box.
         constexpr int MAX_TERMS = 100;
diff --git a/include/lib_tree_model.h b/include/lib_tree_model.h
index 15eb51fce7..2387cc498f 100644
--- a/include/lib_tree_model.h
+++ b/include/lib_tree_model.h
@@ -51,7 +51,6 @@
  * Quick summary of methods used to drive this class:
  *
  * - `UpdateScore()` - accumulate scores recursively given a new search token
- * - `ResetScore()` - reset scores recursively for a new search string
  * - `AssignIntrinsicRanks()` - calculate and cache the initial sort order
  * - `SortNodes()` - recursively sort the tree by score
  * - `Compare()` - compare two nodes; used by `SortNodes()`
@@ -84,13 +83,6 @@ public:
     virtual void UpdateScore( const std::vector<std::unique_ptr<EDA_COMBINED_MATCHER>>& aMatchers,
                               std::function<bool( LIB_TREE_NODE& aNode )>* aFilter ) = 0;
 
-    /**
-     * Initialize scores recursively.
-     */
-    virtual void ResetScore();
-
-    virtual void ForceScore( int aScore ) { m_Score = aScore; }
-
     /**
      * Store intrinsic ranks on all children of this node. See m_IntrinsicRank
      * member doc for more information.