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

Added more missing properties/methods to the Godot converter #70913

Conversation

Maran23
Copy link
Contributor

@Maran23 Maran23 commented Jan 4, 2023

While converting a bigger project I found some properties/methods which were not yet covered by the converter.

The following properties/methods are converted now:

  • icon_align -> icon_alignment (get + set were already converted)
  • rect_min_size -> custom_minimum_size
  • get_tree().set_input_as_handled() -> get_viewport().set_input_as_handled()
  • _unhandled_key_input(event: InputEventKey) -> _unhandled_key_input(event: InputEvent)

And C# equivalents

@Maran23 Maran23 changed the title Added more missing properties/methods to the converter Added more missing properties/methods to the Godot converter Jan 4, 2023
@akien-mga akien-mga added this to the 4.0 milestone Jan 4, 2023
@akien-mga
Copy link
Member

For the methods/properties, there's also a table for C# replacements which would be good to sync.

@Maran23 Maran23 force-pushed the 4-x-some-more-missing-properties-to-godot-converter branch from 51abd16 to f499218 Compare January 4, 2023 19:00
@Maran23
Copy link
Contributor Author

Maran23 commented Jan 4, 2023

For the methods/properties, there's also a table for C# replacements which would be good to sync.

Done. :)
Note that there was no equivalent of rect_min_size (RectMinSize) to minimum_size (MinimumSize) in the converter for C#.
So I skipped custom_minimum_size for now.

@Maran23
Copy link
Contributor Author

Maran23 commented Jan 4, 2023

For the methods/properties, there's also a table for C# replacements which would be good to sync.

Just had a look in the history. The rect_ conversions were added here: #64396
Looks like the C# functions were just forgotten for the rect_ functions then.
I can create another PR where I add them for C# as well. Is that fine?

@Maran23 Maran23 force-pushed the 4-x-some-more-missing-properties-to-godot-converter branch from f499218 to 21061b3 Compare January 5, 2023 18:32
@acedogblast
Copy link

acedogblast commented Jan 6, 2023

From what I understand of the code there is a problem where if one property spelling exists for multiple nodes they all will be renamed even if they are incompatible. Should it be better to do renaming node properties and functions done on a node to node basis rather than the current global perspective?

@YuriSizov
Copy link
Contributor

@acedogblast It works as it does because we can't be sure with untyped GDScript what type does a property hold. And even for typed, I don't think we do any kind of static analysis.

@acedogblast
Copy link

Then how about the vertical and horizontal alignment for labels. In Godot 3.5 they are referred to as "align" and "valign" in Godot 4.0 they have been renamed to "horizontal_alignment" and "vertical_alignment". In both versions they can only be assigned 4 same possible values. I was not sure if we can add that to the list as "align" may be used in other nodes with a different context.

@YuriSizov
Copy link
Contributor

If the expected result is mostly positive, we can add it. If it has a big negative impact on an average codebase, then we shouldn't add it. It will never be completely safe as users may declare any sort of variable or method, and we can conflict with that. Which is why we have a disclaimer that this convertor operates as a best effort tool.

@Maran23
Copy link
Contributor Author

Maran23 commented Jan 6, 2023

I was not sure if we can add that to the list as "align" may be used in other nodes with a different context.

Yeah, align is used in many nodes. valign might be a feasible though.

@Maran23 Maran23 force-pushed the 4-x-some-more-missing-properties-to-godot-converter branch from 21061b3 to 9e20350 Compare January 6, 2023 18:39
@Maran23 Maran23 force-pushed the 4-x-some-more-missing-properties-to-godot-converter branch from 9e20350 to c25b440 Compare January 20, 2023 12:07
icon_align -> icon_alignment
rect_min_size -> custom_minimum_size
get_tree().set_input_as_handled() -> get_viewport().set_input_as_handled()
_unhandled_key_input(event: InputEventKey) -> _unhandled_key_input(event: InputEvent)
And C# equivalents
@Maran23 Maran23 force-pushed the 4-x-some-more-missing-properties-to-godot-converter branch from c25b440 to e27695e Compare January 20, 2023 22:34
@akien-mga akien-mga merged commit 0b141e1 into godotengine:master Jan 20, 2023
@akien-mga
Copy link
Member

Thanks!

@Maran23 Maran23 deleted the 4-x-some-more-missing-properties-to-godot-converter branch August 4, 2023 18:02
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.

4 participants