godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
89.54k stars 20.67k forks source link

Move compatibility handlers to a dedicated Resource conversion system #50691

Open groud opened 3 years ago

groud commented 3 years ago

Godot version

4.0dev

Issue description

From version to version, some resources changed format. While we have ClassDB::add_compatibility_class to help with class renames, we kind of have to keep a lot of old properties in resources to keep compatibility with previous versions.

While most of those properties are surrounded by #ifndef DISABLE_DEPRECATED, those end up cluttering the API (as most of them can be accessed via the set() function). Also, like in the TileSet case, we have no way to operate a conversion depending on several old properties at once, as there is no callback called on the object once a resource is imported.

Consequently, we discussed with @reduz that it would be better to handle conversion from version to version using a dedicated system. That would probably require saving a version number inside the project (or each resource maybe, to support plugins ?) to check if a resource needs an update. The conversion would likely be called after a resource is imported I guess.

Here is a tracker for resources having compatibility properties that could be moved to such a system (some might be missing):

tlobig commented 1 year ago

As for TileMaps and TileSets I can only agree full heartedly. For my own fun and learning experience I am porting a tilemap-heavy part of a GDQuest course from Godot 3 to 4. As of Godot RC2 there is still a huge data loss when importing scenes with tilemaps. In theory matching tile indices should be relatively straight forward, but without a dedicated conversion system it'll be hard to cleanly code this into the engine.

tlobig commented 1 year ago

I started a tilemap/tileset converter for Godot 3 to Godot 4 conversion in python. I may never finish it. But here it is: https://github.com/tlobig/convert_gd3_togd4_tilemaps

naturally-intelligent commented 1 year ago

AnimatedSprite2D SpriteFrames is a big one for me, it loses all data. Oddly, it keeps the data from v3 inside the .tscn, but doesn't convert it into the new format, so it gets ignored.

Example, after conversion:

[gd_scene load_steps=3 format=2]

[ext_resource path="res://icon.png" type="Texture2D" id=1]

[sub_resource type="SpriteFrames" id=1]
animations = [ {
"sprite_frames": [ ExtResource( 1 ), ExtResource( 1 ), ExtResource( 1 ) ],
"loop": true,
"name": "default",
"speed": 5.0
} ]

[node name="Node2D" type="Node2D"]

[node name="AnimatedSprite2D" type="AnimatedSprite2D" parent="."]
sprite_frames = SubResource( 1 )

Should instead be like:

[gd_scene load_steps=3 format=3]

[ext_resource type="Texture2D" path="res://icon.png" id="1"]

[sub_resource type="SpriteFrames" id="1"]
animations = [{
"frames": [{
"duration": 1.0,
"texture": ExtResource("1")
}, {
"duration": 1.0,
"texture": ExtResource("1")
}, {
"duration": 1.0,
"texture": ExtResource("1")
}],
"loop": true,
"name": &"default",
"speed": 5.0
}]

[node name="Node2D" type="Node2D"]

[node name="AnimatedSprite2D" type="AnimatedSprite2D" parent="."]
sprite_frames = SubResource("1")

So the basic data shouldn't be that hard to preserve.

naturally-intelligent commented 1 year ago

Update: manually converting the SpriteFrame data in .tscn files is possible, isn't too hard to do, and is much faster than rebuilding them using the editor. Ideally these wouldn't be done by hand, loading the old frames into JSON and printing them out in the new format should be simple enough.

naturally-intelligent commented 1 year ago

Another update: even easier method that worked for me, is simply replacing the word "sprite_frames" with "frames" in sub_resource type="SpriteFrames"

I just did a search-replace on all .tscn files!

tlobig commented 1 year ago

I looked into it from the perspective of adding it to my converter (which now basically works for tilemaps, if not in all features). The old format is not directly json compatible and you would have to map the ids to new ids containing uuid stuff. it's doable. but don't know if I find the time. Also is preferable to have a proper conversion system inside Godot, of course.

naturally-intelligent commented 1 year ago

Also noticed the Project InputMap keys are lost

dalexeev commented 1 year ago

In the case of SpriteFrames, this is an easy fix. See #73741.

tlobig commented 1 year ago

Adding another detail for the conversion system

Node.pause_mode got replaced by Node.process_mode, but unfortunately: PAUSE_MODE_PROCESS = 2 (process regardless of scene tree pause state) is matched with: PROCESS_MODE_WHEN_PAUSED = 2 instead of the logical equivalent: PROCESS_MODE_ALWAYS = 3

naturally-intelligent commented 1 year ago

Adding another detail for the conversion system

Node.pause_mode got replaced by Node.process_mode, but unfortunately: PAUSE_MODE_PROCESS = 2 (process regardless of scene tree pause state) is matched with: PROCESS_MODE_WHEN_PAUSED = 2 instead of the logical equivalent: PROCESS_MODE_ALWAYS = 3

That one really tripped me up too.

tlobig commented 1 year ago

Parameter of noise textures are lost, I'm not even sure in how far it's possible to get the same parameters from OpenSimplexNoise to FastNoiseLite.

naturally-intelligent commented 1 year ago

Parameter of noise textures are lost, I'm not even sure in how far it's possible to get the same parameters from OpenSimplexNoise to FastNoiseLite.

I'm currently wondering this as well. Let us know if you find anything shareable!

tlobig commented 1 year ago

another one for the bucket - should I start creating issues with a specific label or should be go on like this? Image.create is no a static function yielding the created image, calling it on an instance works and only gives a warning, but also does not really work as the instance is not change and the created image is lost.

@naturally-intelligent

I'm currently wondering this as well. Let us know if you find anything shareable!

So what I found is that FastNoiseLite does kind of contain OpenSimplexNoise. Godot 3 bases the feature on the free (as in free beer) legacy version of OSN, FNL contains OSN2. The parameters mostly yield the same when you use FSL with Simplex or Simplex Smooth and the FBR fractal. Then you will find ocatave and lacunarity directly. Persistance seems to equate with the gain parameter, the only thing I couldn't match 1:1 is period vs. frequency. They work inverse, but I could not figure if it is 1/x or more complicated.

tlobig commented 1 year ago

Editor.is_editor_hint() is now a function and no longer a property (edit: as always I only mention this because the importer does not resolve this)

tlobig commented 1 year ago

Particles, they used to have a trail which was actually the same particle going along the same trajectory n times. There is a new trail which is something completely else and the old visual character of a trail would now need a sub emitter. this is well on the border of what a new conversion system can be expected to achieve or not.

TownEater commented 1 year ago

This seems like the right place to mention, I encountered an issue with the BoxShape3D "extents" value when updating my project from 3 to 4. Within the tscn file, BoxShape resource types are updated to BoxShape3D, and the extents value is changed to a size value. However, an extents value measure from the center of the box to the edge while a size value measures the box edge to edge. Because the name of the value is changed without changing the actual value, all BoxShape resources converted into BoxShape3D resources are done in a way that reduces the size of the shape by half.

Not sure if it would be better to make a new issue, just wanted to document my problems and findings somewhere and this issue seems to cover the problem.