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

Add compatibility rename of (v)align properties of Label #82894

Merged

Conversation

Quimisagi
Copy link
Contributor

Fixes #74493

Now when converting a project from 3 to 4, it is also renamed the align and valign properties to horizontal_align and vertical_align.

@AThousandShips
Copy link
Member

AThousandShips commented Oct 6, 2023

This will break SpinBox and CheckBox which renames align -> alignment

Copy link
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

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

This needs to be done in a dedicated conversion in Label as it will break other cases otherwise, see the code for TextureRect for how to do this

@Quimisagi
Copy link
Contributor Author

This needs to be done in a dedicated conversion in Label as it will break other cases otherwise, see the code for TextureRect for how to do this

Do you mean the TextureRect::_set(const StringName &p_name, const Variant &p_value) function?
Would it change the names of the properties in .tscn files?

@AThousandShips
Copy link
Member

It would handle old properties stored with the old name and convert them to the new property

@Quimisagi Quimisagi requested a review from a team as a code owner October 7, 2023 22:20
@Quimisagi
Copy link
Contributor Author

Quimisagi commented Oct 7, 2023

Ok, I created another _set() function in Label based on the function on TextureRect but I wonder how exactly does it work?
I mean, when that _set() function is called?

@AThousandShips
Copy link
Member

It's called in many cases but here it is relevant when a scene file is loaded

@akien-mga
Copy link
Member

Looks good! Could you squash the commits? See PR workflow for instructions.

scene/gui/label.cpp Outdated Show resolved Hide resolved
@AThousandShips AThousandShips changed the title Rename of align properties of Label Add compatibility rename of (v)align properties of Label Oct 11, 2023
@AThousandShips
Copy link
Member

If you could also update your commitmit message after squashing as the current message implies that the property was renamed, the title of the PR for example

@Quimisagi Quimisagi force-pushed the label-align-values-discarded branch from dc64e4c to 0be873b Compare October 31, 2023 19:42
@Quimisagi
Copy link
Contributor Author

Done!
Can someone please approve it?

@Quimisagi Quimisagi requested a review from akien-mga October 31, 2023 19:44
@AThousandShips
Copy link
Member

AThousandShips commented Oct 31, 2023

I've already approved it? No need for further approval over the cleanup

Edit: Sorry over confusion if it was the workflow, no rush

@akien-mga akien-mga modified the milestones: 4.x, 4.3 Oct 31, 2023
@akien-mga akien-mga merged commit 8c48e99 into godotengine:master Jan 4, 2024
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@Reneator
Copy link

Reneator commented Jan 4, 2024

Thanks a lot!

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.

Godot 3 to 4 Conversion: scene Label align and valign values get discarded
4 participants