From 6eb2e2a6e3592d120e4dab91553d66147003c317 Mon Sep 17 00:00:00 2001
From: jean-pierre charras <jp.charras@wanadoo.fr>
Date: Tue, 30 Mar 2021 11:38:03 +0200
Subject: [PATCH] Fp editor: do not change item UUIDs when loading a footprint
 from library. Fixes #8066 https://gitlab.com/kicad/code/kicad/issues/8066

---
 common/fp_lib_table.cpp                 | 12 +++++++-----
 include/fp_lib_table.h                  | 13 +++++++++++--
 pcbnew/io_mgr.h                         |  7 ++++++-
 pcbnew/load_select_footprint.cpp        |  6 +++++-
 pcbnew/pad.cpp                          |  4 ++++
 pcbnew/plugin.cpp                       |  8 +++++---
 pcbnew/plugins/eagle/eagle_plugin.cpp   |  1 +
 pcbnew/plugins/eagle/eagle_plugin.h     |  1 +
 pcbnew/plugins/geda/gpcb_plugin.cpp     |  1 +
 pcbnew/plugins/geda/gpcb_plugin.h       |  4 +++-
 pcbnew/plugins/kicad/kicad_plugin.cpp   | 12 ++++++++++--
 pcbnew/plugins/kicad/kicad_plugin.h     |  1 +
 pcbnew/plugins/legacy/legacy_plugin.cpp |  1 +
 pcbnew/plugins/legacy/legacy_plugin.h   |  4 +++-
 14 files changed, 59 insertions(+), 16 deletions(-)

diff --git a/common/fp_lib_table.cpp b/common/fp_lib_table.cpp
index b5fab4b30f..3a82a4213d 100644
--- a/common/fp_lib_table.cpp
+++ b/common/fp_lib_table.cpp
@@ -365,13 +365,14 @@ bool FP_LIB_TABLE::FootprintExists( const wxString& aNickname, const wxString& a
 }
 
 
-FOOTPRINT* FP_LIB_TABLE::FootprintLoad( const wxString& aNickname, const wxString& aFootprintName )
+FOOTPRINT* FP_LIB_TABLE::FootprintLoad( const wxString& aNickname,
+                                        const wxString& aFootprintName, bool aKeepUUID )
 {
     const FP_LIB_TABLE_ROW* row = FindRow( aNickname, true );
     wxASSERT( (PLUGIN*) row->plugin );
 
     FOOTPRINT* ret = row->plugin->FootprintLoad( row->GetFullURI( true ), aFootprintName,
-                                                 row->GetProperties() );
+                                                 aKeepUUID, row->GetProperties() );
 
     setLibNickname( ret, row->GetNickName(), aFootprintName );
 
@@ -439,14 +440,15 @@ void FP_LIB_TABLE::FootprintLibCreate( const wxString& aNickname )
 }
 
 
-FOOTPRINT* FP_LIB_TABLE::FootprintLoadWithOptionalNickname( const LIB_ID& aFootprintId )
+FOOTPRINT* FP_LIB_TABLE::FootprintLoadWithOptionalNickname( const LIB_ID& aFootprintId,
+                                                            bool aKeepUUID )
 {
     wxString   nickname = aFootprintId.GetLibNickname();
     wxString   fpname   = aFootprintId.GetLibItemName();
 
     if( nickname.size() )
     {
-        return FootprintLoad( nickname, fpname );
+        return FootprintLoad( nickname, fpname, aKeepUUID );
     }
 
     // nickname is empty, sequentially search (alphabetically) all libs/nicks for first match:
@@ -459,7 +461,7 @@ FOOTPRINT* FP_LIB_TABLE::FootprintLoadWithOptionalNickname( const LIB_ID& aFootp
         {
             // FootprintLoad() returns NULL on not found, does not throw exception
             // unless there's an IO_ERROR.
-            FOOTPRINT* ret = FootprintLoad( nicks[i], fpname );
+            FOOTPRINT* ret = FootprintLoad( nicks[i], fpname, aKeepUUID );
 
             if( ret )
                 return ret;
diff --git a/include/fp_lib_table.h b/include/fp_lib_table.h
index dabfbc5d7a..125141f88d 100644
--- a/include/fp_lib_table.h
+++ b/include/fp_lib_table.h
@@ -168,12 +168,16 @@ public:
      *
      * @param aNickname is a locator for the "library", it is a "name" in #LIB_TABLE_ROW.
      * @param aFootprintName is the name of the footprint to load.
+     * @param aKeepUUID = true to keep initial items UUID, false to set new UUID
+     *                   normally true if loaded in the footprint editor, false
+     *                   if loaded in the board editor. Used only in kicad_plugin
      * @return  the footprint if found caller owns it, else NULL if not found.
      *
      * @throw   IO_ERROR if the library cannot be found or read.  No exception
      *          is thrown in the case where aFootprintName cannot be found.
      */
-    FOOTPRINT* FootprintLoad( const wxString& aNickname, const wxString& aFootprintName );
+    FOOTPRINT* FootprintLoad( const wxString& aNickname, const wxString& aFootprintName,
+                              bool aKeepUUID = false );
 
     /**
      * Indicates whether or not the given footprint already exists in the given library.
@@ -241,13 +245,18 @@ public:
      * Load a footprint having @a aFootprintId with possibly an empty nickname.
      *
      * @param aFootprintId the [nickname] and footprint name of the footprint to load.
+     * @param aKeepUUID = true to keep initial items UUID, false to set new UUID
+     *                   normally true if loaded in the footprint editor, false
+     *                   if loaded in the board editor
+     *                   used only in kicad_plugin
      * @return  the #FOOTPRINT if found caller owns it, else NULL if not found.
      *
      * @throw   IO_ERROR if the library cannot be found or read.  No exception is
      *                   thrown in the case where \a aFootprintName cannot be found.
      * @throw   PARSE_ERROR if @a aFootprintId is not parsed OK.
      */
-    FOOTPRINT* FootprintLoadWithOptionalNickname( const LIB_ID& aFootprintId );
+    FOOTPRINT* FootprintLoadWithOptionalNickname( const LIB_ID& aFootprintId,
+                                                  bool aKeepUUID = false );
 
     /**
      * Load the global footprint library table into \a aTable.
diff --git a/pcbnew/io_mgr.h b/pcbnew/io_mgr.h
index f063949d1b..bc1a20df58 100644
--- a/pcbnew/io_mgr.h
+++ b/pcbnew/io_mgr.h
@@ -395,12 +395,17 @@ public:
      *                    is known to support.  The caller continues to own this object
      *                    (plugin may not delete it), and plugins should expect it to be
      *                    optionally NULL.
+     * @param aKeepUUID = true to keep initial items UUID, false to set new UUID
+     *                   normally true if loaded in the footprint editor, false
+     *                   if loaded in the board editor. Make sense only in kicad_plugin
      * @return the #FOOTPRINT object if found caller owns it, else NULL if not found.
      *
      * @throw   IO_ERROR if the library cannot be found or read.  No exception is thrown in
      *                   the case where \a aFootprintName cannot be found.
      */
-    virtual FOOTPRINT* FootprintLoad( const wxString& aLibraryPath, const wxString& aFootprintName,
+    virtual FOOTPRINT* FootprintLoad( const wxString& aLibraryPath,
+                                      const wxString& aFootprintName,
+                                      bool  aKeepUUID = false,
                                       const PROPERTIES* aProperties = nullptr );
 
     /**
diff --git a/pcbnew/load_select_footprint.cpp b/pcbnew/load_select_footprint.cpp
index c1446b4c49..f28c509dc5 100644
--- a/pcbnew/load_select_footprint.cpp
+++ b/pcbnew/load_select_footprint.cpp
@@ -333,9 +333,13 @@ FOOTPRINT* PCB_BASE_FRAME::loadFootprint( const LIB_ID& aFootprintId )
 
     FOOTPRINT *footprint = nullptr;
 
+    // When loading a footprint from a library in the footprint editor
+    // the items UUIDs must be keep and not reinitialized
+    bool keepUUID = IsType( FRAME_FOOTPRINT_EDITOR );
+
     try
     {
-        footprint = fptbl->FootprintLoadWithOptionalNickname( aFootprintId );
+        footprint = fptbl->FootprintLoadWithOptionalNickname( aFootprintId, keepUUID );
     }
     catch( const IO_ERROR& )
     {
diff --git a/pcbnew/pad.cpp b/pcbnew/pad.cpp
index a3b0f63788..bd488c8556 100644
--- a/pcbnew/pad.cpp
+++ b/pcbnew/pad.cpp
@@ -941,6 +941,10 @@ void PAD::GetMsgPanelInfo( EDA_DRAW_FRAME* aFrame, std::vector<MSG_PANEL_ITEM>&
     aList.emplace_back( wxString::Format( _( "Min Clearance: %s" ),
                                           MessageTextFromValue( units, clearance ) ),
                         wxString::Format( _( "(from %s)" ), source ) );
+#if 0
+    // useful for debug only
+    aList.emplace_back( "UUID", m_Uuid.AsString() );
+#endif
 }
 
 
diff --git a/pcbnew/plugin.cpp b/pcbnew/plugin.cpp
index 6ad947a4da..e2564fbadb 100644
--- a/pcbnew/plugin.cpp
+++ b/pcbnew/plugin.cpp
@@ -84,7 +84,7 @@ const FOOTPRINT* PLUGIN::GetEnumeratedFootprint( const wxString& aLibraryPath,
                                                  const PROPERTIES* aProperties )
 {
     // default implementation
-    return FootprintLoad( aLibraryPath, aFootprintName, aProperties );
+    return FootprintLoad( aLibraryPath, aFootprintName, false, aProperties );
 }
 
 
@@ -92,11 +92,13 @@ bool PLUGIN::FootprintExists( const wxString& aLibraryPath, const wxString& aFoo
                               const PROPERTIES* aProperties )
 {
     // default implementation
-    return FootprintLoad( aLibraryPath, aFootprintName, aProperties ) != nullptr;
+    return FootprintLoad( aLibraryPath, aFootprintName, true, aProperties ) != nullptr;
 }
 
 
-FOOTPRINT* PLUGIN::FootprintLoad( const wxString& aLibraryPath, const wxString& aFootprintName,
+FOOTPRINT* PLUGIN::FootprintLoad( const wxString& aLibraryPath,
+                                  const wxString& aFootprintName,
+                                  bool  aKeepUUID,
                                   const PROPERTIES* aProperties )
 {
     // not pure virtual so that plugins only have to implement subset of the PLUGIN interface.
diff --git a/pcbnew/plugins/eagle/eagle_plugin.cpp b/pcbnew/plugins/eagle/eagle_plugin.cpp
index fd242af247..b5129c62d8 100644
--- a/pcbnew/plugins/eagle/eagle_plugin.cpp
+++ b/pcbnew/plugins/eagle/eagle_plugin.cpp
@@ -2852,6 +2852,7 @@ void EAGLE_PLUGIN::FootprintEnumerate( wxArrayString& aFootprintNames, const wxS
 
 FOOTPRINT* EAGLE_PLUGIN::FootprintLoad( const wxString& aLibraryPath,
                                         const wxString& aFootprintName,
+                                        bool  aKeepUUID,
                                         const PROPERTIES* aProperties )
 {
     init( aProperties );
diff --git a/pcbnew/plugins/eagle/eagle_plugin.h b/pcbnew/plugins/eagle/eagle_plugin.h
index 14263616e1..733521ae09 100644
--- a/pcbnew/plugins/eagle/eagle_plugin.h
+++ b/pcbnew/plugins/eagle/eagle_plugin.h
@@ -141,6 +141,7 @@ public:
                              bool aBestEfforts, const PROPERTIES* aProperties = nullptr) override;
 
     FOOTPRINT* FootprintLoad( const wxString& aLibraryPath, const wxString& aFootprintName,
+                              bool  aKeepUUID = false,
                               const PROPERTIES* aProperties = nullptr ) override;
 
     long long GetLibraryTimestamp( const wxString& aLibraryPath ) const override
diff --git a/pcbnew/plugins/geda/gpcb_plugin.cpp b/pcbnew/plugins/geda/gpcb_plugin.cpp
index 8d264656bc..4b4c4d9fa0 100644
--- a/pcbnew/plugins/geda/gpcb_plugin.cpp
+++ b/pcbnew/plugins/geda/gpcb_plugin.cpp
@@ -931,6 +931,7 @@ const FOOTPRINT* GPCB_PLUGIN::GetEnumeratedFootprint( const wxString& aLibraryPa
 
 FOOTPRINT* GPCB_PLUGIN::FootprintLoad( const wxString& aLibraryPath,
                                        const wxString& aFootprintName,
+                                       bool  aKeepUUID,
                                        const PROPERTIES* aProperties )
 {
     const FOOTPRINT* footprint = getFootprint( aLibraryPath, aFootprintName, aProperties, true );
diff --git a/pcbnew/plugins/geda/gpcb_plugin.h b/pcbnew/plugins/geda/gpcb_plugin.h
index f013261b21..05ed7db086 100644
--- a/pcbnew/plugins/geda/gpcb_plugin.h
+++ b/pcbnew/plugins/geda/gpcb_plugin.h
@@ -57,13 +57,15 @@ public:
     }
 
     void FootprintEnumerate( wxArrayString& aFootprintNames, const wxString& aLibraryPath,
-                             bool aBestEfforts, const PROPERTIES* aProperties = nullptr ) override;
+                             bool aBestEfforts,
+                             const PROPERTIES* aProperties = nullptr ) override;
 
     const FOOTPRINT* GetEnumeratedFootprint( const wxString& aLibraryPath,
                                              const wxString& aFootprintName,
                                              const PROPERTIES* aProperties = nullptr ) override;
 
     FOOTPRINT* FootprintLoad( const wxString& aLibraryPath, const wxString& aFootprintName,
+                              bool  aKeepUUID = false,
                               const PROPERTIES* aProperties = nullptr ) override;
 
     void FootprintDelete( const wxString& aLibraryPath, const wxString& aFootprintName,
diff --git a/pcbnew/plugins/kicad/kicad_plugin.cpp b/pcbnew/plugins/kicad/kicad_plugin.cpp
index a98a556e2d..dbc6b4f0de 100644
--- a/pcbnew/plugins/kicad/kicad_plugin.cpp
+++ b/pcbnew/plugins/kicad/kicad_plugin.cpp
@@ -2301,14 +2301,22 @@ bool PCB_IO::FootprintExists( const wxString& aLibraryPath, const wxString& aFoo
 }
 
 
-FOOTPRINT* PCB_IO::FootprintLoad( const wxString& aLibraryPath, const wxString& aFootprintName,
+FOOTPRINT* PCB_IO::FootprintLoad( const wxString& aLibraryPath,
+                                  const wxString& aFootprintName,
+                                  bool  aKeepUUID,
                                   const PROPERTIES* aProperties )
 {
     const FOOTPRINT* footprint = getFootprint( aLibraryPath, aFootprintName, aProperties, true );
 
     if( footprint )
     {
-        FOOTPRINT* copy = static_cast<FOOTPRINT*>( footprint->Duplicate() );
+        FOOTPRINT* copy;
+
+        if( aKeepUUID )
+            copy = static_cast<FOOTPRINT*>( footprint->Clone() );
+        else
+            copy = static_cast<FOOTPRINT*>( footprint->Duplicate() );
+
         copy->SetParent( nullptr );
         return copy;
     }
diff --git a/pcbnew/plugins/kicad/kicad_plugin.h b/pcbnew/plugins/kicad/kicad_plugin.h
index 6211c3dd1a..017eb9de46 100644
--- a/pcbnew/plugins/kicad/kicad_plugin.h
+++ b/pcbnew/plugins/kicad/kicad_plugin.h
@@ -166,6 +166,7 @@ public:
                           const PROPERTIES* aProperties = nullptr ) override;
 
     FOOTPRINT* FootprintLoad( const wxString& aLibraryPath, const wxString& aFootprintName,
+                              bool  aKeepUUID = false,
                               const PROPERTIES* aProperties = nullptr ) override;
 
     void FootprintSave( const wxString& aLibraryPath, const FOOTPRINT* aFootprint,
diff --git a/pcbnew/plugins/legacy/legacy_plugin.cpp b/pcbnew/plugins/legacy/legacy_plugin.cpp
index 1bace4c7e8..50da43ec65 100644
--- a/pcbnew/plugins/legacy/legacy_plugin.cpp
+++ b/pcbnew/plugins/legacy/legacy_plugin.cpp
@@ -3296,6 +3296,7 @@ void LEGACY_PLUGIN::FootprintEnumerate( wxArrayString& aFootprintNames, const wx
 
 FOOTPRINT* LEGACY_PLUGIN::FootprintLoad( const wxString& aLibraryPath,
                                          const wxString& aFootprintName,
+                                         bool aKeepUUID,
                                          const PROPERTIES* aProperties )
 {
     LOCALE_IO   toggle;     // toggles on, then off, the C locale.
diff --git a/pcbnew/plugins/legacy/legacy_plugin.h b/pcbnew/plugins/legacy/legacy_plugin.h
index 36024e9d2a..f52b09391a 100644
--- a/pcbnew/plugins/legacy/legacy_plugin.h
+++ b/pcbnew/plugins/legacy/legacy_plugin.h
@@ -80,9 +80,11 @@ public:
                  const PROPERTIES* aProperties = nullptr, PROJECT* aProject = nullptr ) override;
 
     void FootprintEnumerate( wxArrayString& aFootprintNames, const wxString& aLibraryPath,
-                             bool aBestEfforts, const PROPERTIES* aProperties = nullptr ) override;
+                             bool aBestEfforts,
+                             const PROPERTIES* aProperties = nullptr ) override;
 
     FOOTPRINT* FootprintLoad( const wxString& aLibraryPath, const wxString& aFootprintName,
+                              bool  aKeepUUID = false,
                               const PROPERTIES* aProperties = nullptr ) override;
 
     bool FootprintLibDelete( const wxString& aLibraryPath,