Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Godot 3.x -> Godot 4.0 project converter #51950

Merged
merged 1 commit into from
Jun 15, 2022

Conversation

qarmin
Copy link
Contributor

@qarmin qarmin commented Aug 21, 2021

Implementation of project converter(currently 3.x -> 4.0, but it can be easily upgraded to handle new versions) godotengine/godot-proposals#387

This PR contains #49053, so it would be good to merge that PR before this because it is needed to check if all functions exists even in builtin types.

Most of things are renamed by "whole word" feature and some with simple Regex.

Before conversion I suggest to use this tool - https://github.com/Scony/godot-gdscript-toolkit to unify code look with some big line length(converter doesn't support multi line expressions and won't support, because it would be too complicated) - e.g. find ./ | grep "\.gd$" | xargs gdformat --line-length 400

For me this is the best way to upgrade project:

  1. Run tool to validate conversion godot4 --validation-conversion-3to4 --headless(current directory must contains godot project)
  2. Read output of previous command, validate variables and rename manually names which you don't want to rename automatically by converter
Checking for conversion - 23/138 file - "test/resources/test_assert_setget_test_objects/ControlChildMock.tscn" with size - 5 KB
    Checking file took 54 ms.
		Line (7) margin_bottom -> offset_bottom  -  LINE """ margin_bottom = 46.0 """
		Line (15) margin_bottom -> offset_bottom  -  LINE """ margin_bottom = 14.0 """
		Line (6) margin_right -> offset_right  -  LINE """ margin_right = 130.0 """
		Line (24) margin_top -> offset_top  -  LINE """ margin_top = -14.0 """

  1. Make sure that your game/project works fine in Godot 3
  2. Repeat steps from 1, until you will be sure that you want to rename all listed elements
  3. Save changes to e.g. Git
  4. Run converter godot4 --convert-3to4 --headless
  5. Try to change project to be able to run it in Godot 4
  6. Revert to Godot 3 project
  7. Re-apply changes(of course if possible) which have been done in Godot 4, to speedup later iteration of conversion
  8. Make sure that your game/project works fine in Godot 3
  9. Repeat steps from 6, until there is no more things that can be changed in Godot 3, to simplify conversion and unify codebase with Godot 4 project

Also during converting, this text about current progress is printing to console

Trying to convert	65/138 file - "test/unit/test_stubber.gd" with size - 7 KB
    File was changed, conversion took 131 ms.
Trying to convert	66/138 file - "test/unit/test_utils.gd" with size - 2 KB
    File was not changed, checking took 50 ms.
.....
Converting ended - all files(802), converted files(319), not converted files(483).

Done things:

  • - functions (also in builtin types)
  • - classes
  • - enums/constants
  • - properties
  • - tool to validate things which will be changed
  • - gdscript keywords
  • - signals
  • - shaders(only 3 renames, but I can't find more)
  • - advanced GDScript renames(like connect -> Callable) - only some, but can be easily added
  • - C# (1:1 CamelCase conversion from GDScript functions)
  • - Check if all new functions exists in Godot 4

Things which may be implemented in future

  • - Regex caching(Not sure if possible)
  • - Removing code duplication(for now there are some code duplication between validating and converting tool, which proably could be integrated into one function)
  • - csproj conversion

Problematic things:

  • Cyclic renames - e.g. in Skeleton3D choose_it change to select_this, but Node3D changes select_this to choose_it. So running tool, will renames all select_this functions to choose_it(assuming that this records have bigger index in function_renames array). Depends of the usage, some things may be renamed(this will broke other functions) when class is used more(like Node3D vs HTTPRequest).
  • Class split - Some classes like Geometry were split at two classes Geometry2D and Geometry3D, so all instances of Geometry are renamed to Geometry2D. If proper class is Geometry3D, then this needs to be fixed manually
  • Deferred functions - custom functions renaming(e.g. via regex), works only with proper syntax(arguments, parentheses etc.) so things like call_deffered which use little different syntax, won't be always fully renamed.
  • Properties changes - most properties are named by single or two simple words like alt. Renaming them, would broke a lot of users scripts, because this words are usable often.
  • Tweens - Tweens were completely rewritten(Complete rewrite of Tweens #41794) and now as base they use RefCounted instead Node, so can't be automatically converted. If want to have less amount of problems with converting, don't add Tweens to tree(try to instantitate them where needed). You may also use TweenSequence from Added Tween Sequence class godot-extended-libraries/godot-next#50.
  • Too big files - Files with size greater than 5MB are not checked, because it takes too long. If you have such files, consider saving e.g. resources or objects to different file.(Not sure about this, maybe big files will be re-enabled later)
  • await - all functions which use await keyword must have await word before
  • Windows - Windows are not longer children of Popups/Controls, so this may require additional changes to code/project

Tested projects:

  • Pole Mrowkowe - enum-int compare(GDScript)
  • Gut - editor_hint(TODO), null problem(GDScript), MARGIN_RIGHT(probably TODO), not dictionaty(Project/GDScript), funcref(TODO)
  • Lorien - Constant Initializer(GDScript), enum compare(GDScript), get_datetime(TODO)
  • Jetpaca - looks that Godot4 doesn't allow to use String && bool(GDScript), nill objects(GDScript)
  • Qarminer - Perfect conversion(some code needs to be enabled manually due availability more types in Godot 4 like Vector2i)
  • Pixelorama - broken project.godot(not sure) ,missing popup hide(Window not longer is child of popup - Project), enum int compare(GDScript), window_tite(TODO), Vector2.INF(GDScript), extends BaseButton not works(GDScript), Tween(too complicated), constant initializer
  • Godot Demo projects - get_simple_path(not sure), to_lower not found in StringName(GDScript)
  • Material Maker - Crash in parse_dictionary(GDScript)
  • Godot Next - Cannot add String to StringName(GDScript), Function "keys" has the same name as a previously declared variable(Project)
  • Tanks of Freedom 3D - Cannot import(Godot)
  • TPS Demo - depth_draw_alpha_prepass shader(Godot?), await(TODO),
  • HeightMap plugin - already created variables(Project)

Legend:

  • Project - project problem, not sure but may be probably added check to converter
  • Godot - Godot bug(general)
  • GDScript - GDScript bug
  • TODO - I may this implement in future

How to find functions that needs to be converted

  • The simplest(and hardest at once) method is to find manually each not working function and also find alternative function which works similarly or just is the same function but only with changed name
  • More automated option require to:
    • compile godot from this PR
    • run this script with the latest available Godot 3 version - PrintAllMethods3.zip (it will produce several txt files like classes.txt which contains info about almost all possible functions, classes, enums etc.)
    • run different script with the latest available Godot 4 version - PrintAllMethods4.zip
    • copy produced files from both runs to new folder
    • create project.godot inside this folder
    • run command godot4 --convert-to-godot40 --headless (This will convert all things from .gd files)
    • sort results, remove duplicate lines etc.
sed -i 's/StringName/String/g' functions4.txt
sed -i 's/Vector2i/Vector2/g' functions4.txt
sed -i 's/Vector3i/Vector3/g' functions4.txt

sort classes4.txt | uniq > classes4.txt2;mv classes4.txt2 classes4.txt
sort constants4.txt | uniq > constants4.txt2;mv constants4.txt2 constants4.txt
sort enums4.txt | uniq > enums4.txt2;mv enums4.txt2 enums4.txt
sort functions4.txt | uniq > functions4.txt2;mv functions4.txt2 functions4.txt
sort functions_default4.txt | uniq > functions_default4.txt2;mv functions_default4.txt2 functions_default4.txt
sort properties4.txt | uniq > properties4.txt2;mv properties4.txt2 properties4.txt
sort signals4.txt | uniq > signals4.txt2;mv signals4.txt2 signals4.txt

sort classes3.gd | uniq > classes3.gd2;mv classes3.gd2 classes3.gd
sort constants3.gd | uniq > constants3.gd2;mv constants3.gd2 constants3.gd
sort enums3.gd | uniq > enums3.gd2;mv enums3.gd2 enums3.gd
sort functions3.gd | uniq > functions3.gd2;mv functions3.gd2 functions3.gd
sort functions_default3.gd | uniq > functions_default3.gd2;mv functions_default3.gd2 functions_default3.gd
sort properties3.gd | uniq > properties3.gd2;mv properties3.gd2 properties3.gd
sort signals3.gd | uniq > signals3.gd2;mv signals3.gd2 signals3.gd
  • compare classes3.gd with classes4.txt etc. with Meld apt install meld
  • it is visible now which classes/functions have same name or are already handled by converter

Screenshot from 2022-04-03 15-32-00

@Calinou Calinou added this to the 4.0 milestone Aug 21, 2021
@Calinou Calinou removed the usability label Aug 21, 2021
@qarmin qarmin marked this pull request as ready for review August 22, 2021 12:33
@qarmin qarmin requested review from a team as code owners August 22, 2021 12:33
@qarmin

This comment has been minimized.

@lyuma
Copy link
Contributor

lyuma commented Aug 25, 2021

Hey @qarmin
I went ahead and merged in some of the extra function calls from my conversion scripts and committed them here:
e5803a0
A lot of them are for builtin types; others are renaming functions on classes that were also renamed. Whatever script was used to generate the original list probably missed those.

I think some of the ones that are commented out that are ambiguous are kind of important and probably better to assume the common case. For example, I consider Control.get_icon->get_theme_icon to be worth doing automatically, even though there are some rare cases in the Theme class and the TreeItem class that this would break.

FuncRef -> Callable is another one that needs to be addressed

There are many patterns that would be nice to do if we had a place to add more general purpose regular expressions that don't fit neatly into the functions or methods buckets:

Here are some regexes to handle cases I found to be common in my code, such as Transform.xform or connect(), using python's raw string syntax r"":

	("\\.xform", "*"), # Transform3D
	(r"([\w.]+)\s*\.\s*xform_inv\s*([^)]*)", r"((\2) * (\1))"), # Transform3D

	("pause_mode = 2", "process_mode = "),
	("pause_mode", "process_mode"),
	(r"\b(connect|disconnect|is_connected)(\s*)\((\s*)([^,]*),(\s*)(?!Callable)([^,\s][^,]*),([^,]*)([,)])", r"\1\2(\3\4,\5Callable(\6,\7)\8"),
	(r"\byield\s*\((.*?)\)\s*$|\byield (.*)$", "await \\1\\2"),
	(r"\byield\s*\(\s*(\S[\s\S]*?)(\s*,\s*)*[\"'](.*)[\"']\s*\)\s*$", "await \\1.\\3"),
	(r"\b@?master(\s*)func\b", "@rpc(master)\\1func"),
	(r"\b@?puppet(\s*)func\b", "@rpc(puppet)\\1func"),
	(r"\b@?remote(\s*)func\b", "@rpc(any)\\1func"),
	(r"\b@?remotesync(\s*)func\b", "@rpc(any,sync)\\1func"),
	(r"\bfunc(\s+)_init(\s*)\(([^)]*)\)[^:]*:", r"func\1_init\2(\3):"),
	(r"\bget_node(\s+)\((\s+)\@", r"get_node\1(\2^"),

These replacements would be only for EditorPlugin: adding the missing _ to some of the overridable functions like get_name and so on.

	(r"\bfunc(\s+)edit\b", r'func\1_edit', ['EditorPlugin']),
	(r"\bfunc(\s+)get_name\b", r'func\1_get_plugin_name', ['EditorPlugin']),
	(r"\bfunc(\s+)handles\b", r'func\1_handles', ['EditorPlugin']),
	(r"\bfunc(\s+)make_visible\b", r'func\1_make_visible', ['EditorPlugin']),

Finally some for export->@export syntax and setget->: set=..., get= syntax

EXPORT_PAREN = re.compile(r"^( *)(\bexport *)?(?:\(([^)]*)\))?( *var *[^=:]*)(: [^ =]*)?( *=[\s\S]*)?")
SETGET_PATTERN = re.compile(r"([\s\S]*)\bsetget\s*([^,]*)(?:,\s*)?([\s\S]*)")

My python script here has a bunch of code to handle that. The export one breaks in a few cases, for example if the line contains a comma. Replacing setget seems moderately reliable.

Clearly I can understand if these heuristics and regexes wouldn't make it in, but I just want to get the discussion going on some other legitimately useful changes.

With all of my regexes as well as the function&class name replacements which your change does well, it usually only requires a few small edits and replaces to fix the rest. The biggest one that my regex usually messes up is mismatching parentheses on the connect syntax, when used like if (something.connect("signal", self, "method")==OK)

@qarmin
Copy link
Contributor Author

qarmin commented Aug 25, 2021

This PR should be created in my repository, because for now there is 2 different PR which implements a little different functions(I added new commit yesterday and I will add probably another today).
Changes to functions are mostly OK, but I didn't like mixing function names with different things like

{ "cursor_set_line", "set_caret_line" }, // Good, only renames function
{ "OS\\.get_ticks_msec", "Time.get_ticks_msec" }, // Bad, renames class but only for some functions 

Non casual renames like e.g. (r"\bfunc(\s+)edit\b", r'func\1_edit', ['EditorPlugin']), should be handled in rename_enums, rename_classes etc. functions(I don't like too much regex so it may took a while until I implement this).

@lyuma
Copy link
Contributor

lyuma commented Aug 26, 2021

@qarmin Apologies, I seem to have done a poor job communicating. I only created that draft PR as a way to publish the diff on top of your PR so that you could merge in the changes. Since there are a few things to fix from my commit, I'll post them here and close my draft PR:

These are wrong and should probably be removed:

	{ "invert", "is_empty" }, // Array
	{ "set_enabled_focus_mode", "set_shortcut_context" }, // BaseButton
	{ "get_enabled_focus_mode", "get_shortcut_context" }, // BaseButton

The closest analogy for invert is "reverse" but it no longer returns the array, so it's not a drop-in replacement. I'd perhaps still suggest changing invert to reverse and leave the user to fix the return value error.

Here are more safe functions from my PR that I would like added (I know enabled_focus_mode isn't one-to-one with focus_mode but it internally calls focus_mode in most cases so it should be as close as you can get):

	{ "get_enabled_focus_mode", "get_focus_mode" }, //BaseButton
	{ "set_enabled_focus_mode", "set_focus_mode" }, //BaseButton
	{ "get_editor_viewport", "get_viewport" }, // EditorPlugin
	{ "http_escape", "uri_encode" }, //String
	{ "http_unescape", "uri_decode" }, //String
	{ "percent_decode", "uri_decode" }, //String
	{ "percent_encode", "uri_encode" }, //String
	{ "popup_centered_minsize", "popup_centered_clamped" }, //Window

Properties in patch form:

diff --git a/editor/converter.cpp b/editor/converter.cpp
index 3ae8c6883f74..3fcf17db5500 100644
--- a/editor/converter.cpp
+++ b/editor/converter.cpp
@@ -351,19 +380,27 @@ static const char *properties_renames[][2] = {
 	// {"Skeleton3D","Skeleton"}, // Polygon2D - this would rename also classes
 	// {"rotate","rotates"}, // PathFollow2D - probably function exists with same name
 	{ "Debug Shape3D", "Debug Shape" }, // RayCast3D
+	{ "doubleclick", "double_click" }, // InputEventMouseButton
 	{ "Emission Shape3D", "Emission Shape" }, // ParticlesMaterial
+	{ "enabled_focus_mode", "focus_mode" }, // BaseButton - Removed
+	{ "as_normalmap", "as_normal_map" }, // NoiseTexture
 	{ "caret_moving_by_right_click", "caret_move_on_right_click" }, // TextEdit
-	{ "d", "disatance" }, //WorldMarginShape2D
+	{ "d", "distance" }, //WorldMarginShape2D
 	{ "global_rate_scale", "playback_speed_scale" }, // AudioServer
 	{ "group", "button_group" }, // BaseButton
+	{ "popup_exclusive", "exclusive" }, //Window
 	{ "region_filter_clip", "region_filter_clip_enabled" }, // Sprite2D
 	{ "syntax_highlighting", "syntax_highlighter" }, // TextEdit
+	{ "toplevel", "top_level" }, // Node
 	{ "translation", "position" }, // Node3D - broke GLTFNode
+	{ "zfar", "far" }, // Camera3D
+	{ "znear", "near" }, // Camera3D
 	{ nullptr, nullptr },
 };

new signal:

	{ "about_to_show", "about_to_popup" }, // Window

not sure about these:

	{ "GDScriptFunctionState", "Node3D" },
	{ "GDScriptNativeClass", "Node3D" },

the first one is unlikely. the second one might show up due to GDNative. it's probably best to leave them as is instead of replacing with Node3D.
Finally, RayShape -> RayCast is wrong, but we should wait for #51896 to get it back in the form of SeparationRayShape.

@qarmin
Copy link
Contributor Author

qarmin commented Aug 26, 2021

I changed code according to suggestions.

Now I started to create manual regex for each specific function (like connect -> Callable)

This is how it looks in code

//  assert(speed < 20, str(randi()%10))  ->  assert(speed < 20)    GDScript - GDScript bug
RegEx reg_assert = RegEx("assert\\(([^\n]+),[^\n]+\\)");
file_content = reg_assert.sub(file_content, "assert($1)", true);

so it shouldn't be too hard to create, since I use builtin in Godot regex object.
I use this GDScript file to validate created regex(a2 should be same or similar to a3)

	var a1 = "Color.black" 
	var a2 = "Color.BLACK"
	
	var reg : RegEx = RegEx.new()
	assert(reg.compile("Color.([a-z]+)") == OK)
	var a3 = reg.sub(a1,"Color.[$1\\p{Lu}]",true)
	
	print(a1)
	print(a2)
	print(a3)

@qarmin qarmin force-pushed the bad_godot4_converter branch 2 times, most recently from 882d890 to bb73963 Compare August 30, 2021 10:21
@qarmin
Copy link
Contributor Author

qarmin commented Aug 30, 2021

@lyuma can you add real life examples(or even simplified) about your regex's from above/python script?
I'm quite bad at reading them, because a lot of better I work with this

(connect(A,B,C,D,E,F,G) != OK):  ->  (connect(A,Callable(B,C),D,E,F,G) != OK):

than

RegEx("\\bconnect\\(([^,^\n]+),([^,^\n]+),([^,^\n^\\)]+)");
reg_connect.sub(file_content, "connect($1, Callable($2,$3)", true);

@Calinou
Copy link
Member

Calinou commented Aug 30, 2021

Cyclic renames - e.g. in Skeleton3D choose_it change to select_this, but Node3D changes select_this to choose_it. So running tool, will renames all select_this functions to choose_it(assuming that this records have bigger index in function_renames array). Depends of the usage, some things may be renamed(this will broke other functions) when class is used more(like Node3D vs HTTPRequest) Cyclic renames are validated by tool.

In this example, cyclic renames could be handled by renaming choose_it to something_temporary, renaming select_this to choose_it then renaming something_temporary to select_this. It's like swapping 2 variables using a temporary 3rd variable 🙂

@qarmin
Copy link
Contributor Author

qarmin commented Aug 30, 2021

Adding support for this, probably will require to add new array/function.
I'm not exactly sure if it is worth to do it - it will add some complexity but this problem probably happens only for some(I think max 10) methods.
I think it would be best to standardize these names, to not have such problems.

@qarmin qarmin requested review from a team as code owners September 1, 2021 18:49
@akien-mga
Copy link
Member

This PR contains #49053, so it would be good to merge that PR before this because it is needed to check if all functions exists even in builtin types.

Where is this method used exactly in this PR? I don't see any code that calls ClassDB::get_variant_method_list.

If it's not directly needed right now, it might be easier to merge the rest of the converter as agreeing on how to expose get_variant_method_list was the main point of debate.

@qarmin
Copy link
Contributor Author

qarmin commented Mar 10, 2022

get_method_list_by_type function was used to get list of methods from builtin types.

I expected that this code will find more breaking changes, but looks that not found any(at least yet), so it is not so important as I think that it would be.

I commented this code.

editor/converter.cpp Outdated Show resolved Hide resolved
@duchainer
Copy link

duchainer commented Apr 30, 2022

Not certain this is the best place to put it, but if not I moved that over to the correct issue/proposal/discussion. 😅

If you want to add an Open Source project to test it on and compare with a real life conversion :
I'm currently porting Librerama from Godot 3.4 to Godot4.alpha7 in my own fork.
The first commit of my new branch is the auto-converted stuff from the existing Godot3 to Godot4 converting taking place in alpha7.
The following commits are the manual conversion that I'm currently doing.
I'll edit this post when I'm done converting everything, but I hope this can be useful.

Wish you the best, have a great weekend ^^

@fire
Copy link
Member

fire commented May 1, 2022

What are the blockers on merging this in a timely matter now?

@fire-forge
Copy link
Contributor

Does the converter support theme property renames? Several theme properties were renamed in #60261, and there may be others as well.

@Calinou
Copy link
Member

Calinou commented May 17, 2022

One more shader rename that could be added: TRANSMISSIONSSS_TRANSMITTANCE_COLOR

@akien-mga
Copy link
Member

I pushed an update to improve some names:

  • GodotConverter4 -> ProjectConverter3To4
  • converter.h -> project_converter_3_to_4.h
  • --convert-to-godot40 -> --convert-3to4
  • --validate-convert-to-godot40 -> --validation-conversion-3to4

And fixed build with RegEx module disabled.

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a pretty good start for converting projects. There are more conversions that can be added in future PRs, and still some manual work needed, but it does make it possible to open a fairly big project like Jetpaca in 4.0 and run it (with bugs) and with minor manual script changes.

@akien-mga akien-mga merged commit 4bbe7f0 into godotengine:master Jun 15, 2022
@akien-mga
Copy link
Member

Thanks!

@golddotasksquestions
Copy link

Is this the right place to submit remaining methods which still need to be included in the converter as of Beta12?

@Riteo
Copy link
Contributor

Riteo commented Jan 15, 2023

@golddotasksquestions I think that you should just open a ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.