Remove ID from property fields

ID was not maintained or used other than to ensure unique fields.
Instead of saving, we assign the known IDs to specific field names and
sequentially create new IDs on load

Fixes https://gitlab.com/kicad/code/kicad/issues/12390

common/template_fieldnames.cpp

@@ -30,6 +30,7 @@
 
 using namespace TFIELD_T;
 
+// N.B. Do not change these values without transitioning the file format
 #define REFERENCE_CANONICAL "Reference"
 #define VALUE_CANONICAL "Value"
 #define FOOTPRINT_CANONICAL "Footprint"

eeschema/sch_file_versions.h

@@ -46,8 +46,8 @@
 //#define SEXPR_SYMBOL_LIB_FILE_VERSION  20220126   // Text boxes.
 //#define SEXPR_SYMBOL_LIB_FILE_VERSION  20220328   // Text box start/end -> at/size.
 //#define SEXPR_SYMBOL_LIB_FILE_VERSION  20220331   // Text colors.
-#define SEXPR_SYMBOL_LIB_FILE_VERSION  20220914     // Symbol unit display names.
-
+//#define SEXPR_SYMBOL_LIB_FILE_VERSION  20220914   // Symbol unit display names.
+#define SEXPR_SYMBOL_LIB_FILE_VERSION  20220914     // Don't save property ID
 
 /**
  * Schematic file version.
@@ -87,4 +87,5 @@
 //#define SEXPR_SCHEMATIC_FILE_VERSION 20220822  // Hyperlinks in text objects
 //#define SEXPR_SCHEMATIC_FILE_VERSION 20220903  // Field name visibility
 //#define SEXPR_SCHEMATIC_FILE_VERSION 20220904  // Do not autoplace field option
-#define SEXPR_SCHEMATIC_FILE_VERSION 20220914    // Add support for DNP
+//#define SEXPR_SCHEMATIC_FILE_VERSION 20220914  // Add support for DNP
+#define SEXPR_SCHEMATIC_FILE_VERSION 20220929    // Don't save property ID

eeschema/sch_plugins/kicad/sch_sexpr_lib_plugin_cache.cpp

@@ -398,10 +398,9 @@ void SCH_SEXPR_PLUGIN_CACHE::saveField( LIB_FIELD* aField, OUTPUTFORMATTER& aFor
     if( aField->GetId() >= 0 && aField->GetId() < MANDATORY_FIELDS )
         fieldName = TEMPLATE_FIELDNAME::GetDefaultFieldName( aField->GetId(), false );
 
-    aFormatter.Print( aNestLevel, "(property %s %s (id %d) (at %s %s %g)",
+    aFormatter.Print( aNestLevel, "(property %s %s (at %s %s %g)",
                       aFormatter.Quotew( fieldName ).c_str(),
                       aFormatter.Quotew( aField->GetText() ).c_str(),
-                      aField->GetId(),
                       EDA_UNIT_UTILS::FormatInternalUnits( schIUScale, aField->GetPosition().x ).c_str(),
                       EDA_UNIT_UTILS::FormatInternalUnits( schIUScale, aField->GetPosition().y ).c_str(),
                       aField->GetTextAngle().AsDegrees() );

eeschema/sch_plugins/kicad/sch_sexpr_parser.cpp

@@ -770,6 +770,18 @@ LIB_FIELD* SCH_SEXPR_PARSER::parseProperty( std::unique_ptr<LIB_SYMBOL>& aSymbol
     }
 
     field->SetName( name );
+
+    // Correctly set the ID based on canonical (untranslated) field name
+    // If ID is stored in the file (old versions), it will overwrite this
+    for( int ii = 0; ii < MANDATORY_FIELDS; ++ii )
+    {
+        if( !name.CmpNoCase( TEMPLATE_FIELDNAME::GetDefaultFieldName( ii ) ) )
+        {
+            field->SetId( ii );
+            break;
+        }
+    }
+
     token = NextTok();
 
     if( !IsSymbol( token ) )
@@ -1900,6 +1912,31 @@ SCH_FIELD* SCH_SEXPR_PARSER::parseSchField( SCH_ITEM* aParent )
     field->SetText( value );
     field->SetVisible( true );
 
+    // Correctly set the ID based on canonical (untranslated) field name
+    // If ID is stored in the file (old versions), it will overwrite this
+    if( aParent->Type() == SCH_SYMBOL_T )
+    {
+        for( int ii = 0; ii < MANDATORY_FIELDS; ++ii )
+        {
+            if( name == TEMPLATE_FIELDNAME::GetDefaultFieldName( ii, false ) )
+            {
+                field->SetId( ii );
+                break;
+            }
+        }
+    }
+    else if( aParent->Type() == SCH_SHEET_T )
+    {
+        for( int ii = 0; ii < SHEET_MANDATORY_FIELDS; ++ii )
+        {
+            if( name == SCH_SHEET::GetDefaultFieldName( ii,  false ) )
+            {
+                field->SetId( ii );
+                break;
+            }
+        }
+    }
+
     for( token = NextTok(); token != T_RIGHT; token = NextTok() )
     {
         if( token != T_LEFT )
@@ -2878,12 +2915,14 @@ SCH_SHEET* SCH_SEXPR_PARSER::parseSheet()
             // complete) solution is to disallow multiple instances of the same id (for all
             // files since the source of the error *might* in fact be hand-edited files).
             //
-            // While no longer used, -1 is still a valid id for user field.  It gets converted
-            // to the next unused number on save.
-            if( fieldIDsRead.count( field->GetId() ) )
-                field->SetId( -1 );
-            else
-                fieldIDsRead.insert( field->GetId() );
+            // While no longer used, -1 is still a valid id for user field.  We convert it to
+            // the first available ID after the mandatory fields
+
+            if( field->GetId() < 0 )
+                field->SetId( SHEET_MANDATORY_FIELDS );
+
+            while( !fieldIDsRead.insert( field->GetId() ).second )
+                field->SetId( field->GetId() + 1 );
 
             fields.emplace_back( *field );
             delete field;

eeschema/sch_plugins/kicad/sch_sexpr_plugin.cpp

@@ -816,7 +816,6 @@ void SCH_SEXPR_PLUGIN::saveField( SCH_FIELD* aField, int aNestLevel )
     wxCHECK_RET( aField != nullptr && m_out != nullptr, "" );
 
     wxString fieldName = aField->GetCanonicalName();
-
     // For some reason (bug in legacy parser?) the field ID for non-mandatory fields is -1 so
     // check for this in order to correctly use the field name.
 
@@ -830,10 +829,9 @@ void SCH_SEXPR_PLUGIN::saveField( SCH_FIELD* aField, int aNestLevel )
         m_nextFreeFieldId = aField->GetId() + 1;
     }
 
-    m_out->Print( aNestLevel, "(property %s %s (id %d) (at %s %s %s)",
+    m_out->Print( aNestLevel, "(property %s %s (at %s %s %s)",
                   m_out->Quotew( fieldName ).c_str(),
                   m_out->Quotew( aField->GetText() ).c_str(),
-                  aField->GetId(),
                   EDA_UNIT_UTILS::FormatInternalUnits( schIUScale, aField->GetPosition().x ).c_str(),
                   EDA_UNIT_UTILS::FormatInternalUnits( schIUScale, aField->GetPosition().y ).c_str(),
                   EDA_UNIT_UTILS::FormatAngle( aField->GetTextAngle() ).c_str() );

eeschema/sch_sheet.cpp

@@ -46,8 +46,9 @@
 #include <pgm_base.h>
 #include <wx/log.h>
 
-#define SHEET_NAME_CANONICAL "Sheet name"
-#define SHEET_FILE_CANONICAL "Sheet file"
+// N.B. Do not change these values without transitioning the file format
+#define SHEET_NAME_CANONICAL "Sheetname"
+#define SHEET_FILE_CANONICAL "Sheetfile"
 #define USER_FIELD_CANONICAL "Field%d"