Cleanup 3-to-4 renames, prevent common words replacements, handle project.godot settings #74040

akien-mga · 2023-02-27T11:18:08Z

A bit late but should help solve common pitfalls.
I'll put some inline comments on some relevant changes.

~~Fixes not 3-to-4 project converter changes instance of "Reference" in a comment to "RefCounted" #66061.~~
Fixes --convert-3to4 converts common simple English words in comments such as: "on" -> "checked", "off" -> "unchecked" #73505.
Fixes 3 to 4 converter alters strings "Room" into "Node3D" #73996.
Fixes 3-to-4 project converter doesn't preserve viewport width and height #66125.
Supersedes Project converter: fix convertion for project.godot file #66251.

akien-mga · 2023-02-27T11:18:57Z

editor/renames_map_3_to_4.cpp

@@ -456,7 +466,6 @@ const char *RenamesMap3To4::gdscript_function_renames[][2] = {
 	{ "post_import", "_post_import" }, // EditorScenePostImport
 	{ "print_stray_nodes", "print_orphan_nodes" }, // Node
 	{ "property_list_changed_notify", "notify_property_list_changed" }, // Object
-	{ "raise", "move_to_front" }, // CanvasItem


Removed as possibly conflicting with comments / user-defined methods.

akien-mga · 2023-02-27T11:19:04Z

editor/renames_map_3_to_4.cpp

 	{ "clamped", "limit_length" }, // Vector2
 	{ "get_rotation_quat", "get_rotation_quaternion" }, // Basis
 	{ "grow_margin", "grow_side" }, // Rect2
-	{ "invert", "reverse" }, // Array -- Give it a check. Be careful, this will be used everywhere.


Too common.

akien-mga · 2023-02-27T11:19:27Z

editor/renames_map_3_to_4.cpp

 	{ "is_abs_path", "is_absolute_path" }, // String
 	{ "is_valid_integer", "is_valid_int" }, // String
 	{ "linear_interpolate", "lerp" }, // Color
 	{ "find_last", "rfind" }, // Array, String
 	{ "to_ascii", "to_ascii_buffer" }, // String
 	{ "to_utf8", "to_utf8_buffer" }, // String
-	{ "to_wchar", "to_utf32_buffer" }, // String -- utf32 or utf16?


Wrong (will be fixed by #73225).

So, shouldn't it be changed to convert to to_wchar_buffer then?

It's not merged yet, because production folks are stingy on their merges :P

Ahh, right right, the vermin of this world get in our way again!

akien-mga · 2023-02-27T11:19:55Z

editor/renames_map_3_to_4.cpp

+	// NOTE: Commented out renames are disabled because deemed not suitable for
+	// the current way the regex-based converter works.
+	// When uncommenting any of those as suitable for conversion, please move it
+	// to the block with other enabled conversions, ordered alphabetically, and
+	// make sure to add it to the C# rename map too.


Added this blurb so we don't have to keep commented out stuff in sync too.

akien-mga · 2023-02-27T11:20:16Z

editor/renames_map_3_to_4.cpp

@@ -617,52 +623,6 @@ const char *RenamesMap3To4::gdscript_function_renames[][2] = {

 // gdscript_function_renames clone with CamelCase
 const char *RenamesMap3To4::csharp_function_renames[][2] = {


For properties I did a full comparative review of GDScript vs C#, but not for this one.

akien-mga · 2023-02-27T11:21:26Z

editor/renames_map_3_to_4.cpp

@@ -1155,6 +1118,7 @@ const char *RenamesMap3To4::gdscript_properties_renames[][2] = {
 	{ "refuse_new_network_connections", "refuse_new_connections" }, // MultiplayerAPI
 	{ "region_filter_clip", "region_filter_clip_enabled" }, // Sprite2D
 	{ "reverb_bus_enable", "reverb_bus_enabled" }, // Area3D
+	{ "scancode", "keycode" }, // InputEventKey


Seems safe enough?

akien-mga · 2023-02-27T11:21:34Z

editor/renames_map_3_to_4.cpp

@@ -1170,49 +1134,29 @@ const char *RenamesMap3To4::gdscript_properties_renames[][2] = {
 	{ "tab_align", "tab_alignment" }, // TabContainer
 	{ "table_hseparation", "table_h_separation" }, // Theme
 	{ "table_vseparation", "table_v_separation" }, // Theme
-	{ "translation", "position" }, // Node3D -- Breaks GLTFNode
+	{ "toplevel", "top_level" }, // Node


Seems safe enough?

akien-mga · 2023-02-27T11:22:29Z

editor/renames_map_3_to_4.cpp

 	{ "ExtraSpacingBottom", "SpacingBottom" }, // Font
 	{ "ExtraSpacingTop", "SpacingTop" }, // Font
 	{ "FocusNeighbourBottom", "FocusNeighborBottom" }, // Control
 	{ "FocusNeighbourLeft", "FocusNeighborLeft" }, // Control
 	{ "FocusNeighbourRight", "FocusNeighborRight" }, // Control
 	{ "FocusNeighbourTop", "FocusNeighborTop" }, // Control
 	{ "FollowViewportEnable", "FollowViewportEnabled" }, // CanvasItem
+	{ "FileIconModulate", "FileIconColor" }, // Theme


I don't know if theme properties are actually capitalized or if they're used with their core snake_case identifier.
Either way, these shouldn't hurt as it will do nothing if they're not capitalized.

akien-mga · 2023-02-27T11:23:41Z

editor/renames_map_3_to_4.cpp

+	// Too common.
+	// { "Area", "Area3D" },
+	// { "Camera", "Camera3D" },
+	// { "World", "World3D" },
+	// { "Particles", "GPUParticles3D" },
+	// { "Reference", "RefCounted" },
+	// { "Tabs", "TabBar" },
+	// { "Shape", "Shape3D" },
+	// { "Path", "Path3D" },


This will lead to more manual work for users, as Reference, Area, Camera, Shape and Path are quite used. But those are also potential clashes with user comments and strings.

akien-mga · 2023-02-27T11:25:31Z

editor/renames_map_3_to_4.cpp

-	// Portal and room occlusion culling was replaced by raster occlusion culling.
-	{ "CullInstance", "Node3D" },
-	{ "RoomGroup", "Node3D" },
-	{ "Room", "Node3D" },
-	{ "RoomManager", "Node3D" },
-	{ "Portal", "Node3D" },


This kind of renames was added as aliases in register_scene_types.cpp to avoid scenes getting corrupted, but it makes no sense to do the same in scripts.

YuriSizov · 2023-02-27T11:48:22Z

Removing Reference is a big downgrade, IMO. There is a decent possibility for conflicts, but it's also a core class in Godot, and can be used quite a lot. Not sure if it's better to avoid converting it and have users update it manually.

akien-mga · 2023-02-27T11:51:57Z

That's fair. What about the others?

	// { "Area", "Area3D" },
	// { "Camera", "Camera3D" },
	// { "Particles", "GPUParticles3D" },
	// { "Path", "Path3D" },
	// { "Reference", "RefCounted" },
	// { "Shape", "Shape3D" },
	// { "Tabs", "TabBar" },
	// { "World", "World3D" },

I think at least "World" can be kept commented out, currently it would replace stuff like using World as node name in a node path ($Game/World/0/StartPosition, etc.).

"Particles" probably doesn't make sense to convert like this because it's not going to be compatible anyway.

The rest might be still worth it even if there's a high risk of doing wrong replacements in e.g. comments, strings and node paths?

YuriSizov

Aside from the two notes above, LGTM. We can give it a go.

The rest might be still worth it even if there's a high risk of doing wrong replacements in e.g. comments, strings and node paths?

I agree that World is too commonplace (I have it in practically every project, and it's never the World node). For the rest, it's a toss. We can go on with a safer option and re-evaluate later. I wouldn't worry too much about comments though.

akien-mga · 2023-02-27T11:57:57Z

That's fair. What about the others?

	// { "Area", "Area3D" },
	// { "Camera", "Camera3D" },
	// { "Particles", "GPUParticles3D" },
	// { "Path", "Path3D" },
	// { "Reference", "RefCounted" },
	// { "Shape", "Shape3D" },
	// { "Tabs", "TabBar" },
	// { "World", "World3D" },

Alright, re-added those aside from "World" (too common) and "Particles" (too common, and incompatible class).

YuriSizov · 2023-02-27T11:58:30Z

Feels good!

Fixes godotengine#73505. Fixes godotengine#73996.

In the ConfigFile format, the first subpath is the category and is not part of the line that the regex would match. Fixes godotengine#66125.

akien-mga · 2023-02-27T12:35:17Z

Added a second commit to fix #66125 too, with a much simpler approach than #66251.

akien-mga added enhancement topic:editor usability labels Feb 27, 2023

akien-mga added this to the 4.0 milestone Feb 27, 2023

akien-mga force-pushed the safer-renames branch from ebf87bb to e307048 Compare February 27, 2023 11:24

akien-mga commented Feb 27, 2023

View reviewed changes

akien-mga requested review from KoBeWi, qarmin and YuriSizov February 27, 2023 11:25

YuriSizov approved these changes Feb 27, 2023

View reviewed changes

akien-mga force-pushed the safer-renames branch from e307048 to 5f5569c Compare February 27, 2023 11:56

akien-mga force-pushed the safer-renames branch 2 times, most recently from 72e2ea4 to b02cbd0 Compare February 27, 2023 12:06

akien-mga added 2 commits February 27, 2023 13:14

Cleanup 3-to-4 renames, prevent common words replacements

6eb25f2

Fixes godotengine#73505. Fixes godotengine#73996.

Add 3-to-4 renames for project settings in project.godot

7cf1ec1

In the ConfigFile format, the first subpath is the category and is not part of the line that the regex would match. Fixes godotengine#66125.

akien-mga force-pushed the safer-renames branch from b02cbd0 to 7cf1ec1 Compare February 27, 2023 12:34

akien-mga mentioned this pull request Feb 27, 2023

Project converter: fix convertion for project.godot file #66251

Closed

YuriSizov approved these changes Feb 27, 2023

View reviewed changes

akien-mga changed the title ~~Cleanup 3-to-4 renames, prevent common words replacements~~ Cleanup 3-to-4 renames, prevent common words replacements, handle project.godot settings Feb 27, 2023

akien-mga merged commit 8208060 into godotengine:master Feb 27, 2023

akien-mga deleted the safer-renames branch February 27, 2023 12:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cleanup 3-to-4 renames, prevent common words replacements, handle project.godot settings #74040

Cleanup 3-to-4 renames, prevent common words replacements, handle project.godot settings #74040

akien-mga commented Feb 27, 2023 •

edited

Loading

akien-mga Feb 27, 2023

akien-mga Feb 27, 2023

akien-mga Feb 27, 2023

YuriSizov Feb 27, 2023

akien-mga Feb 27, 2023 •

edited

Loading

YuriSizov Feb 27, 2023

akien-mga Feb 27, 2023

akien-mga Feb 27, 2023

akien-mga Feb 27, 2023

akien-mga Feb 27, 2023

akien-mga Feb 27, 2023

akien-mga Feb 27, 2023

akien-mga Feb 27, 2023

YuriSizov commented Feb 27, 2023

akien-mga commented Feb 27, 2023

YuriSizov left a comment •

edited

Loading

akien-mga commented Feb 27, 2023

YuriSizov commented Feb 27, 2023

akien-mga commented Feb 27, 2023

		@@ -617,52 +623,6 @@ const char *RenamesMap3To4::gdscript_function_renames[][2] = {

		// gdscript_function_renames clone with CamelCase
		const char *RenamesMap3To4::csharp_function_renames[][2] = {

Cleanup 3-to-4 renames, prevent common words replacements, handle project.godot settings #74040

Cleanup 3-to-4 renames, prevent common words replacements, handle project.godot settings #74040

Conversation

akien-mga commented Feb 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akien-mga Feb 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

YuriSizov commented Feb 27, 2023

akien-mga commented Feb 27, 2023

YuriSizov left a comment • edited Loading

Choose a reason for hiding this comment

akien-mga commented Feb 27, 2023

YuriSizov commented Feb 27, 2023

akien-mga commented Feb 27, 2023

akien-mga commented Feb 27, 2023 •

edited

Loading

akien-mga Feb 27, 2023 •

edited

Loading

YuriSizov left a comment •

edited

Loading