Importer version system for files imported in older Godot versions

[aaronfranke]

Fixes #83429, caused by a behavior change from #80270 (the old behavior is wrong, but we must keep it for compat), and also fixes the behavior change from #81264.

I think the easiest way to explain how this works is by giving examples. Comprehensive test project: Block.zip All .glb files in this project are the exact same, just with different file names and .import files.

The "4.2" folder has .import files generated with this PR.

The "4.1" folder has .import files generated in Godot 4.1. The expected behavior is that they import the same way as they did in 4.1, when the scene name (usually file name) took priority over the node name.

The "NoVersion" folder has .import files without a importer_version= field. The expected behavior is that they import with the old 4.1 behavior.

The NoImportFile folder does not have .import files for the .glb files. This is the case when a user brings in a new model, or wants to reset the settings by deleting the .import files. This should generate new files with the importer version set to 2, and then import the same as the files in the 4.2 folder.

The importer version is read by EditorFileSystem, then passed to ResourceImporter, then for scenes this is passed to EditorSceneFormatImporter, then for GLTF this is passed to GLTFDocument.

If desired, we could expose the methods to set the importer version to allow users to manually import files with an older importer version (ex: when using GLTFDocument at runtime), but the use case here is not entirely clear (modifying a file after import, when we care about preserving the exact same data, is mostly relevant in the editor).

The default value of the importer version is RESOURCE_IMPORTER_VERSION which is set to 2 in this PR (it can be incremented later), and this value is also used when generating a new .import file.

[YuriSizov]

caused by a behavior change from #80270 (the old behavior is wrong, but we must keep it for compat).

We should also preserve compat for the Camera node, #81264.

[YuriSizov]

I can confirm the desired behavior with the provided test project. Here's the diff that is generating by loading it with the current master:

diff --git a/4.1/AnotherName.glb.import b/4.1/AnotherName.glb.import
index b406a90..5d5624e 100644
--- a/4.1/AnotherName.glb.import
+++ b/4.1/AnotherName.glb.import
@@ -22,6 +22,7 @@ meshes/generate_lods=true
 meshes/create_shadow_meshes=true
 meshes/light_baking=1
 meshes/lightmap_texel_size=0.2
+meshes/force_disable_compression=false
 skins/use_named_skins=true
 animation/import=true
 animation/fps=30
diff --git a/4.1/Block.glb.import b/4.1/Block.glb.import
index 8407bd9..58fe1e6 100644
--- a/4.1/Block.glb.import
+++ b/4.1/Block.glb.import
@@ -22,6 +22,7 @@ meshes/generate_lods=true
 meshes/create_shadow_meshes=true
 meshes/light_baking=1
 meshes/lightmap_texel_size=0.2
+meshes/force_disable_compression=false
 skins/use_named_skins=true
 animation/import=true
 animation/fps=30
diff --git a/4.2/AnotherName.glb.import b/4.2/AnotherName.glb.import
index 3447673..f7e1645 100644
--- a/4.2/AnotherName.glb.import
+++ b/4.2/AnotherName.glb.import
@@ -1,15 +1,15 @@
 [remap]
 
 importer="scene"
-importer_version=2
+importer_version=1
 type="PackedScene"
 uid="uid://275venqh4408"
-path="res://.godot/imported/AnotherName.glb-5175963f54ad9b115131acbc71b59274.scn"
+path="res://.godot/imported/AnotherName.glb-6fbb2a4c9d761b7b0ce8aafc06672cb8.scn"
 
 [deps]
 
-source_file="res://block/4.2/AnotherName.glb"
-dest_files=["res://.godot/imported/AnotherName.glb-5175963f54ad9b115131acbc71b59274.scn"]
+source_file="res://4.2/AnotherName.glb"
+dest_files=["res://.godot/imported/AnotherName.glb-6fbb2a4c9d761b7b0ce8aafc06672cb8.scn"]
 
 [params]
 
diff --git a/4.2/Block.glb.import b/4.2/Block.glb.import
index 016607d..6c52eb3 100644
--- a/4.2/Block.glb.import
+++ b/4.2/Block.glb.import
@@ -1,15 +1,15 @@
 [remap]
 
 importer="scene"
-importer_version=2
+importer_version=1
 type="PackedScene"
 uid="uid://ss7w6wyp8ygb"
-path="res://.godot/imported/Block.glb-4042f690a7a3ec13bc6979a71e0e5788.scn"
+path="res://.godot/imported/Block.glb-31f7f18bdc8ac6cb1702c9598be2ffe9.scn"
 
 [deps]
 
-source_file="res://block/4.2/Block.glb"
-dest_files=["res://.godot/imported/Block.glb-4042f690a7a3ec13bc6979a71e0e5788.scn"]
+source_file="res://4.2/Block.glb"
+dest_files=["res://.godot/imported/Block.glb-31f7f18bdc8ac6cb1702c9598be2ffe9.scn"]
 
 [params]
 
diff --git a/4.2/expected.png.import b/4.2/expected.png.import
index 49a0898..a9e625d 100644
--- a/4.2/expected.png.import
+++ b/4.2/expected.png.import
@@ -1,7 +1,6 @@
 [remap]
 
 importer="texture"
-importer_version=2
 type="CompressedTexture2D"
 uid="uid://c4impncd7l7v4"
 path="res://.godot/imported/expected.png-7229408d46009b91c60c9a69247fd1cd.ctex"
diff --git a/NoImportFile/expected.png.import b/NoImportFile/expected.png.import
index 4601b86..13f92f6 100644
--- a/NoImportFile/expected.png.import
+++ b/NoImportFile/expected.png.import
@@ -1,7 +1,6 @@
 [remap]
 
 importer="texture"
-importer_version=2
 type="CompressedTexture2D"
 uid="uid://dccljbc0xhd3x"
 path="res://.godot/imported/expected.png-c355fe22188fd1d35ad2a0ef08699014.ctex"
diff --git a/NoVersion/AnotherName.glb.import b/NoVersion/AnotherName.glb.import
index de8facb..89d5ca6 100644
--- a/NoVersion/AnotherName.glb.import
+++ b/NoVersion/AnotherName.glb.import
@@ -1,6 +1,7 @@
 [remap]
 
 importer="scene"
+importer_version=1
 type="PackedScene"
 uid="uid://dc2a2pm2a36l3"
 path="res://.godot/imported/AnotherName.glb-de2a94a6dc054dfcf634b193e0b3066b.scn"
@@ -21,6 +22,7 @@ meshes/generate_lods=true
 meshes/create_shadow_meshes=true
 meshes/light_baking=1
 meshes/lightmap_texel_size=0.2
+meshes/force_disable_compression=false
 skins/use_named_skins=true
 animation/import=true
 animation/fps=30
diff --git a/NoVersion/Block.glb.import b/NoVersion/Block.glb.import
index 609ad94..ddb46ec 100644
--- a/NoVersion/Block.glb.import
+++ b/NoVersion/Block.glb.import
@@ -1,6 +1,7 @@
 [remap]
 
 importer="scene"
+importer_version=1
 type="PackedScene"
 uid="uid://cpjmsi82x2rws"
 path="res://.godot/imported/Block.glb-be263d8933a084b0b2450178012d0586.scn"
@@ -21,6 +22,7 @@ meshes/generate_lods=true
 meshes/create_shadow_meshes=true
 meshes/light_baking=1
 meshes/lightmap_texel_size=0.2
+meshes/force_disable_compression=false
 skins/use_named_skins=true
 animation/import=true
 animation/fps=30
diff --git a/NoVersion/expected.png.import b/NoVersion/expected.png.import
index f66790d..5578a8b 100644
--- a/NoVersion/expected.png.import
+++ b/NoVersion/expected.png.import
@@ -1,7 +1,6 @@
 [remap]
 
 importer="texture"
-importer_version=2
 type="CompressedTexture2D"
 uid="uid://ba71xa50ty0gc"
 path="res://.godot/imported/expected.png-ed2d28179ab42f03d6cfc6a8d3d0fdf5.ctex"
diff --git a/project.godot b/project.godot
index 6317761..e46cf7d 100644
--- a/project.godot
+++ b/project.godot
@@ -11,7 +11,7 @@ config_version=5
 [application]
 
 config/name="Block"
-config/features=PackedStringArray("GL Compatibility")
+config/features=PackedStringArray("4.2", "GL Compatibility")
 config/icon="res://icon.svg"
 
 [rendering]

And here's the diff from this PR:

diff --git a/4.1/AnotherName.glb.import b/4.1/AnotherName.glb.import
index b406a90..5d5624e 100644
--- a/4.1/AnotherName.glb.import
+++ b/4.1/AnotherName.glb.import
@@ -22,6 +22,7 @@ meshes/generate_lods=true
 meshes/create_shadow_meshes=true
 meshes/light_baking=1
 meshes/lightmap_texel_size=0.2
+meshes/force_disable_compression=false
 skins/use_named_skins=true
 animation/import=true
 animation/fps=30
diff --git a/4.1/Block.glb.import b/4.1/Block.glb.import
index 8407bd9..58fe1e6 100644
--- a/4.1/Block.glb.import
+++ b/4.1/Block.glb.import
@@ -22,6 +22,7 @@ meshes/generate_lods=true
 meshes/create_shadow_meshes=true
 meshes/light_baking=1
 meshes/lightmap_texel_size=0.2
+meshes/force_disable_compression=false
 skins/use_named_skins=true
 animation/import=true
 animation/fps=30
diff --git a/4.1/expected.png.import b/4.1/expected.png.import
index c23eb67..65955e1 100644
--- a/4.1/expected.png.import
+++ b/4.1/expected.png.import
@@ -1,6 +1,7 @@
 [remap]
 
 importer="texture"
+importer_version=0
 type="CompressedTexture2D"
 uid="uid://d1cvgqegapopq"
 path="res://.godot/imported/expected.png-6b9144af47c58c765521266c357983e2.ctex"
diff --git a/4.2/AnotherName.glb.import b/4.2/AnotherName.glb.import
index 3447673..9aa2809 100644
--- a/4.2/AnotherName.glb.import
+++ b/4.2/AnotherName.glb.import
@@ -4,12 +4,12 @@ importer="scene"
 importer_version=2
 type="PackedScene"
 uid="uid://275venqh4408"
-path="res://.godot/imported/AnotherName.glb-5175963f54ad9b115131acbc71b59274.scn"
+path="res://.godot/imported/AnotherName.glb-6fbb2a4c9d761b7b0ce8aafc06672cb8.scn"
 
 [deps]
 
-source_file="res://block/4.2/AnotherName.glb"
-dest_files=["res://.godot/imported/AnotherName.glb-5175963f54ad9b115131acbc71b59274.scn"]
+source_file="res://4.2/AnotherName.glb"
+dest_files=["res://.godot/imported/AnotherName.glb-6fbb2a4c9d761b7b0ce8aafc06672cb8.scn"]
 
 [params]
 
diff --git a/4.2/Block.glb.import b/4.2/Block.glb.import
index 016607d..86fc100 100644
--- a/4.2/Block.glb.import
+++ b/4.2/Block.glb.import
@@ -4,12 +4,12 @@ importer="scene"
 importer_version=2
 type="PackedScene"
 uid="uid://ss7w6wyp8ygb"
-path="res://.godot/imported/Block.glb-4042f690a7a3ec13bc6979a71e0e5788.scn"
+path="res://.godot/imported/Block.glb-31f7f18bdc8ac6cb1702c9598be2ffe9.scn"
 
 [deps]
 
-source_file="res://block/4.2/Block.glb"
-dest_files=["res://.godot/imported/Block.glb-4042f690a7a3ec13bc6979a71e0e5788.scn"]
+source_file="res://4.2/Block.glb"
+dest_files=["res://.godot/imported/Block.glb-31f7f18bdc8ac6cb1702c9598be2ffe9.scn"]
 
 [params]
 
diff --git a/4.2/expected.png.import b/4.2/expected.png.import
index 49a0898..5fce9c5 100644
--- a/4.2/expected.png.import
+++ b/4.2/expected.png.import
@@ -1,7 +1,7 @@
 [remap]
 
 importer="texture"
-importer_version=2
+importer_version=0
 type="CompressedTexture2D"
 uid="uid://c4impncd7l7v4"
 path="res://.godot/imported/expected.png-7229408d46009b91c60c9a69247fd1cd.ctex"
diff --git a/NoImportFile/expected.png.import b/NoImportFile/expected.png.import
index 4601b86..0239cc2 100644
--- a/NoImportFile/expected.png.import
+++ b/NoImportFile/expected.png.import
@@ -1,7 +1,7 @@
 [remap]
 
 importer="texture"
-importer_version=2
+importer_version=0
 type="CompressedTexture2D"
 uid="uid://dccljbc0xhd3x"
 path="res://.godot/imported/expected.png-c355fe22188fd1d35ad2a0ef08699014.ctex"
diff --git a/NoVersion/AnotherName.glb.import b/NoVersion/AnotherName.glb.import
index de8facb..e0c4d8b 100644
--- a/NoVersion/AnotherName.glb.import
+++ b/NoVersion/AnotherName.glb.import
@@ -1,6 +1,7 @@
 [remap]
 
 importer="scene"
+importer_version=0
 type="PackedScene"
 uid="uid://dc2a2pm2a36l3"
 path="res://.godot/imported/AnotherName.glb-de2a94a6dc054dfcf634b193e0b3066b.scn"
@@ -21,6 +22,7 @@ meshes/generate_lods=true
 meshes/create_shadow_meshes=true
 meshes/light_baking=1
 meshes/lightmap_texel_size=0.2
+meshes/force_disable_compression=false
 skins/use_named_skins=true
 animation/import=true
 animation/fps=30
diff --git a/NoVersion/Block.glb.import b/NoVersion/Block.glb.import
index 609ad94..5d0dd7e 100644
--- a/NoVersion/Block.glb.import
+++ b/NoVersion/Block.glb.import
@@ -1,6 +1,7 @@
 [remap]
 
 importer="scene"
+importer_version=0
 type="PackedScene"
 uid="uid://cpjmsi82x2rws"
 path="res://.godot/imported/Block.glb-be263d8933a084b0b2450178012d0586.scn"
@@ -21,6 +22,7 @@ meshes/generate_lods=true
 meshes/create_shadow_meshes=true
 meshes/light_baking=1
 meshes/lightmap_texel_size=0.2
+meshes/force_disable_compression=false
 skins/use_named_skins=true
 animation/import=true
 animation/fps=30
diff --git a/NoVersion/expected.png.import b/NoVersion/expected.png.import
index f66790d..c56a26f 100644
--- a/NoVersion/expected.png.import
+++ b/NoVersion/expected.png.import
@@ -1,7 +1,7 @@
 [remap]
 
 importer="texture"
-importer_version=2
+importer_version=0
 type="CompressedTexture2D"
 uid="uid://ba71xa50ty0gc"
 path="res://.godot/imported/expected.png-ed2d28179ab42f03d6cfc6a8d3d0fdf5.ctex"
diff --git a/icon.svg.import b/icon.svg.import
index e51df6b..9ae8bde 100644
--- a/icon.svg.import
+++ b/icon.svg.import
@@ -1,6 +1,7 @@
 [remap]
 
 importer="texture"
+importer_version=0
 type="CompressedTexture2D"
 uid="uid://cyxnr1w8fdacs"
 path="res://.godot/imported/icon.svg-218a8f2b3041327d8a5756f3a245f83b.ctex"
diff --git a/project.godot b/project.godot
index 6317761..e46cf7d 100644
--- a/project.godot
+++ b/project.godot
@@ -11,7 +11,7 @@ config_version=5
 [application]
 
 config/name="Block"
-config/features=PackedStringArray("GL Compatibility")
+config/features=PackedStringArray("4.2", "GL Compatibility")
 config/icon="res://icon.svg"
 
 [rendering]

I haven't looked at the code yet, but what I've noticed is that the master version sets the importer to version 1, whereas this PR sets it to either 2 or 0. This happens to all files too, not just scenes. I would expect the old version to be preserved as 1.

[aaronfranke]

I would expect the old version to be preserved as 1.

Ok, done. But note that in Godot 4.1.x, it wrote importer_version=1 to scenes, and left other files at version 0 (and therefore did not write them), so I figured I should preserve the version number for non-scene files as 0.

[YuriSizov]

Ok, done. But note that in Godot 4.1.x, it wrote importer_version=1 to scenes, and left other files at version 0 (and therefore did not write them), so I figured I should preserve the version number for non-scene files as 0.

Right, and this ties into another point.

I'm not completely sold on the idea that we should version all importers as one. This means every release of Godot that changes any one importer will cause every single imported resource to update this field. Now, we don't auto-increment this version for pre-existing resources right now. But this still means that a texture imported in 4.1 and texture imported in 4.2 will have a different importer version assigned to them, despite having no difference in behavior.

The old system, which wasn't really used outside of importing scenes and OBJ files allowed us to version importers individually. I think it would be better to keep it this way. It shouldn't require a lot of changes to your implementation (which seems good to me otherwise).

The new methods would be virtual. In ResourceImporter the default value would be set to 0 (and we could restore the old logic that skipped writing to .import when value was 0). In EditorFileSystem instead of

importer_version = RESOURCE_IMPORTER_VERSION;

we can set it to -1, and avoid overriding the default importer version if it's -1 (basically making -1 mean AUTO):

if (importer_version >= 0) {
    importer->set_importer_version(importer_version);
}

And RESOURCE_IMPORTER_VERSION would be replaced by a dedicated version constants for both scene and OBJ importers. Scene would be set to 2 and OBJ would be set to 1, to preserve existing versioning.

---

Do you see any downsides with this?

[YuriSizov]

Note that we ignore .inc files (except .compat.inc) in some CI scripts. So this file should probably be a header file (and with guards).

[aaronfranke]

I'm not completely sold on the idea that we should version all importers as one. This means every release of Godot that changes any one importer will cause every single imported resource to update this field.

Well, it will only change for new resources. Old resources will still have the original importer version stored in the file.

I don't really see a problem with versioning the importers together. If nothing changes in the PNG importer between versions 1 and 2, then there's not really a problem. If something changes in version 3 we can just have a check for if (importer_version < 3) {.

I think it's useful information to be able to tell what Godot version a resource came from. We could make it importer_version="4.1" but that would not keep compatibility with the existing number.

Having extra numbers is harmless, having inconsistent numbers may be problematic.

we can set it to -1, and avoid overriding the default importer version if it's -1 (basically making -1 mean AUTO):

No, it can't be auto. That would defeat the whole point. We want it to be pinned to the initial version used to import the asset, to avoid the importer spontaneously changing from under the user.

[YuriSizov]

Having extra numbers is harmless, having inconsistent numbers may be problematic.

How is that problematic if versioning is a feature of each individual importer? All the version information can be tracked in the same exact file as the importer itself is defined, as comments and constants. It works gracefully with custom modules and GDExtensions too this way. Lumping everything together is inconsistent, because you signal unrelated changes in tools which remain the same between versions.

It creates inconsistencies between otherwise identical imports in version N and version N+1.

No, it can't be auto. That would defeat the whole point. We want it to be pinned to the initial version used to import the asset, to avoid the importer spontaneously changing from under the user.

Auto here means "Whatever is the current version of that importer". It's the same logic that you already have implemented, just with different versions for different importers instead of one monolithic version for everything.

[akien-mga]

Great work here, thanks for working on solving this!

I tend to agree with Yuri about making this importer specific. It seems weird to me that we're versioning unrelated importers together, and bumping the version number for all of them if any has a change that requires compatibility handling code.

Most importantly, this makes it unsuitable for thirdparty importers to use, as they can't ask us to bump the importer version for them to handle a change in their importer's compatibility. Everyone should have the possibility to break compatibility in their importer without having to get this reflected upstream.

[aaronfranke]

@akien-mga Makes sense, that's an important use case. I will update the PR later today.

[YuriSizov]

This works correctly, thanks for making the changes.

I still think we should not store version 0, same as we ignored it before. It's going to be added to all resources eventually, but doesn't convey any new information compared to not having it at all.

But if others are okay with this, I won't argue.

Diff with the test project and the latest version of this PR

diff --git a/4.1/AnotherName.glb.import b/4.1/AnotherName.glb.import
index b406a90..5d5624e 100644
--- a/4.1/AnotherName.glb.import
+++ b/4.1/AnotherName.glb.import
@@ -22,6 +22,7 @@ meshes/generate_lods=true
 meshes/create_shadow_meshes=true
 meshes/light_baking=1
 meshes/lightmap_texel_size=0.2
+meshes/force_disable_compression=false
 skins/use_named_skins=true
 animation/import=true
 animation/fps=30
diff --git a/4.1/Block.glb.import b/4.1/Block.glb.import
index 8407bd9..58fe1e6 100644
--- a/4.1/Block.glb.import
+++ b/4.1/Block.glb.import
@@ -22,6 +22,7 @@ meshes/generate_lods=true
 meshes/create_shadow_meshes=true
 meshes/light_baking=1
 meshes/lightmap_texel_size=0.2
+meshes/force_disable_compression=false
 skins/use_named_skins=true
 animation/import=true
 animation/fps=30
diff --git a/4.1/expected.png.import b/4.1/expected.png.import
index c23eb67..65955e1 100644
--- a/4.1/expected.png.import
+++ b/4.1/expected.png.import
@@ -1,6 +1,7 @@
 [remap]
 
 importer="texture"
+importer_version=0
 type="CompressedTexture2D"
 uid="uid://d1cvgqegapopq"
 path="res://.godot/imported/expected.png-6b9144af47c58c765521266c357983e2.ctex"
diff --git a/4.2/AnotherName.glb.import b/4.2/AnotherName.glb.import
index 3447673..9aa2809 100644
--- a/4.2/AnotherName.glb.import
+++ b/4.2/AnotherName.glb.import
@@ -4,12 +4,12 @@ importer="scene"
 importer_version=2
 type="PackedScene"
 uid="uid://275venqh4408"
-path="res://.godot/imported/AnotherName.glb-5175963f54ad9b115131acbc71b59274.scn"
+path="res://.godot/imported/AnotherName.glb-6fbb2a4c9d761b7b0ce8aafc06672cb8.scn"
 
 [deps]
 
-source_file="res://block/4.2/AnotherName.glb"
-dest_files=["res://.godot/imported/AnotherName.glb-5175963f54ad9b115131acbc71b59274.scn"]
+source_file="res://4.2/AnotherName.glb"
+dest_files=["res://.godot/imported/AnotherName.glb-6fbb2a4c9d761b7b0ce8aafc06672cb8.scn"]
 
 [params]
 
diff --git a/4.2/Block.glb.import b/4.2/Block.glb.import
index 016607d..86fc100 100644
--- a/4.2/Block.glb.import
+++ b/4.2/Block.glb.import
@@ -4,12 +4,12 @@ importer="scene"
 importer_version=2
 type="PackedScene"
 uid="uid://ss7w6wyp8ygb"
-path="res://.godot/imported/Block.glb-4042f690a7a3ec13bc6979a71e0e5788.scn"
+path="res://.godot/imported/Block.glb-31f7f18bdc8ac6cb1702c9598be2ffe9.scn"
 
 [deps]
 
-source_file="res://block/4.2/Block.glb"
-dest_files=["res://.godot/imported/Block.glb-4042f690a7a3ec13bc6979a71e0e5788.scn"]
+source_file="res://4.2/Block.glb"
+dest_files=["res://.godot/imported/Block.glb-31f7f18bdc8ac6cb1702c9598be2ffe9.scn"]
 
 [params]
 
diff --git a/4.2/expected.png.import b/4.2/expected.png.import
index 49a0898..5fce9c5 100644
--- a/4.2/expected.png.import
+++ b/4.2/expected.png.import
@@ -1,7 +1,7 @@
 [remap]
 
 importer="texture"
-importer_version=2
+importer_version=0
 type="CompressedTexture2D"
 uid="uid://c4impncd7l7v4"
 path="res://.godot/imported/expected.png-7229408d46009b91c60c9a69247fd1cd.ctex"
diff --git a/NoImportFile/expected.png.import b/NoImportFile/expected.png.import
index 4601b86..0239cc2 100644
--- a/NoImportFile/expected.png.import
+++ b/NoImportFile/expected.png.import
@@ -1,7 +1,7 @@
 [remap]
 
 importer="texture"
-importer_version=2
+importer_version=0
 type="CompressedTexture2D"
 uid="uid://dccljbc0xhd3x"
 path="res://.godot/imported/expected.png-c355fe22188fd1d35ad2a0ef08699014.ctex"
diff --git a/NoImportFile/test_no_import_file.tscn b/NoImportFile/test_no_import_file.tscn
index db02669..7d8acd1 100644
--- a/NoImportFile/test_no_import_file.tscn
+++ b/NoImportFile/test_no_import_file.tscn
@@ -1,7 +1,7 @@
 [gd_scene load_steps=3 format=3 uid="uid://dgfou466achtg"]
 
-[ext_resource type="PackedScene" uid="uid://nbmwsb5wurqv" path="res://NoImportFile/AnotherName.glb" id="1_fujdl"]
-[ext_resource type="PackedScene" uid="uid://d1k7rol406bym" path="res://NoImportFile/Block.glb" id="2_1mskm"]
+[ext_resource type="PackedScene" uid="uid://bd4gmmk3ts5o0" path="res://NoImportFile/AnotherName.glb" id="1_fujdl"]
+[ext_resource type="PackedScene" uid="uid://dvtqf1qi5vssw" path="res://NoImportFile/Block.glb" id="2_1mskm"]
 
 [node name="TestNoImportFile" type="Node3D"]
 
diff --git a/NoVersion/AnotherName.glb.import b/NoVersion/AnotherName.glb.import
index de8facb..e0c4d8b 100644
--- a/NoVersion/AnotherName.glb.import
+++ b/NoVersion/AnotherName.glb.import
@@ -1,6 +1,7 @@
 [remap]
 
 importer="scene"
+importer_version=0
 type="PackedScene"
 uid="uid://dc2a2pm2a36l3"
 path="res://.godot/imported/AnotherName.glb-de2a94a6dc054dfcf634b193e0b3066b.scn"
@@ -21,6 +22,7 @@ meshes/generate_lods=true
 meshes/create_shadow_meshes=true
 meshes/light_baking=1
 meshes/lightmap_texel_size=0.2
+meshes/force_disable_compression=false
 skins/use_named_skins=true
 animation/import=true
 animation/fps=30
diff --git a/NoVersion/Block.glb.import b/NoVersion/Block.glb.import
index 609ad94..5d0dd7e 100644
--- a/NoVersion/Block.glb.import
+++ b/NoVersion/Block.glb.import
@@ -1,6 +1,7 @@
 [remap]
 
 importer="scene"
+importer_version=0
 type="PackedScene"
 uid="uid://cpjmsi82x2rws"
 path="res://.godot/imported/Block.glb-be263d8933a084b0b2450178012d0586.scn"
@@ -21,6 +22,7 @@ meshes/generate_lods=true
 meshes/create_shadow_meshes=true
 meshes/light_baking=1
 meshes/lightmap_texel_size=0.2
+meshes/force_disable_compression=false
 skins/use_named_skins=true
 animation/import=true
 animation/fps=30
diff --git a/NoVersion/expected.png.import b/NoVersion/expected.png.import
index f66790d..c56a26f 100644
--- a/NoVersion/expected.png.import
+++ b/NoVersion/expected.png.import
@@ -1,7 +1,7 @@
 [remap]
 
 importer="texture"
-importer_version=2
+importer_version=0
 type="CompressedTexture2D"
 uid="uid://ba71xa50ty0gc"
 path="res://.godot/imported/expected.png-ed2d28179ab42f03d6cfc6a8d3d0fdf5.ctex"
diff --git a/icon.svg.import b/icon.svg.import
index e51df6b..9ae8bde 100644
--- a/icon.svg.import
+++ b/icon.svg.import
@@ -1,6 +1,7 @@
 [remap]
 
 importer="texture"
+importer_version=0
 type="CompressedTexture2D"
 uid="uid://cyxnr1w8fdacs"
 path="res://.godot/imported/icon.svg-218a8f2b3041327d8a5756f3a245f83b.ctex"
diff --git a/project.godot b/project.godot
index 6317761..e46cf7d 100644
--- a/project.godot
+++ b/project.godot
@@ -11,7 +11,7 @@ config_version=5
 [application]
 
 config/name="Block"
-config/features=PackedStringArray("GL Compatibility")
+config/features=PackedStringArray("4.2", "GL Compatibility")
 config/icon="res://icon.svg"
 
 [rendering]

[akien-mga]

It seems a bit weird to me that we need to split the ResourceImporterScene version number to a separate file, to use in GLTFDocument.

I'm assuming you don't want to include core/io/resource_importer_scene.h in a Resource file, which makes sense. Resource is the result of ResourceImporterScene, it shouldn't know about it.

But by the same rationale, I wonder why GLTFDocument as a Resource class is the one fetching this information. Why can't this be handled in EditorSceneFormatImporterGLTF which has access to this information by inheritance? If for architectural reasons the code can't be in the importer and needs to be in GLTFDocument, I would suggest that EditorSceneFormatImporterGLTF should pass that information to the resource it creates. Wouldn't that make more sense?

But more importantly, as I pointed out earlier when advocating for making the versioning importer-specific, this versioning should be kept specific to GLTF, not the whole ResourceImporterScene IMO. GLTF is a module, and should be treated like we'd treat thirdparty code, as encapsulated as possible.

I still think we should not store version 0, same as we ignored it before. It's going to be added to all resources eventually, but doesn't convey any new information compared to not having it at all.

I'd tend to agree, if we don't need the information stored, it's probably best not to modify all .import files unnecessarily.

But also, is the default 0 or 1? I see we're setting the scene version to 2, and it's the first time we do versioning, so it seems to infer the previous/no-info version was 1.

[aaronfranke]

I would suggest that EditorSceneFormatImporterGLTF should pass that information to the resource it creates. Wouldn't that make more sense?

It already does pass this information, but this is not enough because this is not the only way to use GLTFDocument. If user code contains GLTFDocument.new(), we want to ensure that is using the latest version by default.

But also, is the default 0 or 1? I see we're setting the scene version to 2, and it's the first time we do versioning, so it seems to infer the previous/no-info version was 1.

In both Godot 4.0 and 4.1, ResourceImporterScene and ResourceImporterOBJ used version 1, which was explicitly written. In all other resource importers, the version was 0, which was not written. So for every case a version was not written, that version was 0. The no-info version is version 0.

I'd tend to agree, if we don't need the information stored, it's probably best not to modify all .import files unnecessarily.

I think it's worth always writing this information explicitly. Having it missing before was causing @reduz to have concerns about this system in the first place. If you insist I will remove this (and close #84062) but I will do so in protest.

But more importantly, as I pointed out earlier when advocating for making the versioning importer-specific, this versioning should be kept specific to GLTF, not the whole ResourceImporterScene IMO.

Ok, done. In order to do this I had to refactor some of the logic so that getting the scene importer is its own function, and I also had to add a file extension argument to get_latest_importer_version so that the scene importer can distinguish glTF files, FBX files, Blend files, etc. Now only glTF knows about 2. Also, if you are confused about the terminology of an importer having importers, I have clarified this terminology in another PR #82988 by renaming some of the methods (which will have to be rebased on this PR).

[YuriSizov]

But also, is the default 0 or 1?

@akien-mga Just to reiterate, 0 basically means that the importer has not opted in to this versioning mechanism. Which most importers did not.

but I will do so in protest.

@aaronfranke If you don't think this is a good approach, let's discuss it further.

[lyuma]

Since this is importer-specific, I don't understand why this is a special case with a special property. In particular, I want to be able to see and modify the importer version from the inspector, since it has a real effect on the imported scene (and the effect will only become more prevalent over time).

With the PR as is, the only way to reset the importer version is to change the "import as" to AnimationLibrary and back (which requires restarting Godot twice) or deleting the .import file / hand-editing it.

Seeing as the PR is specifically for GLTF behavior, I would prefer if the importer version is a standard property such as gltf/importer_version or gltf/importer_compat_version or gltf/node_name_compatibility_version or whatever bike shed. This would drastically reduce the amount of code needed for the PR, while making it easier for the user to view and change the compatibility version.

Note that the discussion about defaults has been dealt with before, during a previous change which added a "use legacy names" flag in 3.x - I don't know how much of this code changed in Godot 4, but my understanding is there's a different codepath for assigning defaults for existing .import files (which would be 0) and for writing completely new .import files which have never been imported before (which would be the latest version, such as 2)

[aaronfranke]

Alternative PR: #84271, which implements a glTF-specific compatibility system as @lyuma suggests. Some code has to be in EditorFileSystem to disambiguate the case of a file without a version from a missing file that should use the default version. That PR includes a different test project from this PR.

[YuriSizov]

@lyuma The idea for this was to quickly address the issue. We would provide a one way upgrade option in the import dialog as a follow up, but this is not critical for the release.

[reduz]

I much prefer the approach in #84271, I left some comments there on how I would clean it up a bit.

[YuriSizov]

Superseded by #84271.