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 checks to remove meta arrays when creating and undoing guides #81011

Merged
merged 1 commit into from
Aug 28, 2023

Conversation

21dhruvp
Copy link
Contributor

@21dhruvp 21dhruvp commented Aug 26, 2023

Metadata arrays are now correctly removed after executing the steps from #80947 for the horizontal guide, vertical guide, and in the case of both being used.

@AThousandShips
Copy link
Member

This was a solution already opened:

Though this adds a further case

@AThousandShips
Copy link
Member

Please do not use merges to update your branch but rebase instead, see here

Please squash your commits into one, see here

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.

When pushing you should do git push -f to avoid the merge commits you create

editor/plugins/canvas_item_editor_plugin.cpp Outdated Show resolved Hide resolved
editor/plugins/canvas_item_editor_plugin.cpp Outdated Show resolved Hide resolved
@21dhruvp
Copy link
Contributor Author

Thanks a ton for the help! This is my first open source contribution, so I don't really have much experience. I squashed and force-pushed the commits, so please let me know if there is anything else I need to do.

@AThousandShips
Copy link
Member

You need to squash again, you have three commits

Added checks to remove meta arrays when creating and undoing guides

Update editor/plugins/canvas_item_editor_plugin.cpp

Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>

Update editor/plugins/canvas_item_editor_plugin.cpp

Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
@21dhruvp
Copy link
Contributor Author

Alright, I did it again.

@AThousandShips AThousandShips requested a review from a team August 26, 2023 18:08
Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

Looks good.

On a side note, it might be worth adding some remove_guide() method to clean this up. That's for another PR though.

@21dhruvp
Copy link
Contributor Author

I certainly thought about it, but decided against it since I didn't want to change the code too radically for the time being.
Thank you!

@akien-mga akien-mga merged commit 8f07644 into godotengine:master Aug 28, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

@21dhruvp 21dhruvp deleted the remove-meta-array branch August 30, 2023 17:14
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.

Undoing guide creation makes empty array in scene file
4 participants