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 groups to node section #5

Merged
merged 3 commits into from Jun 21, 2021
Merged

Conversation

richard-gebbia
Copy link
Contributor

Hi!

First of all thank you so much for making this project! It's so far saved me quite a bit of time on my own Godot project.

However, my project requires that the nodes that I'm generating have groups (this is normally done in Godot by manually assigning a node to a group in the inspector). It looks like groups was on your todo-list based on the code comments, so I tried my best to implement them. Implementing these changes locally has allowed me to export nodes with groups.

I'm relatively new to Python development and I wasn't sure how to test my changes, so please let me know if this is an acceptable change, and if it's not I would appreciate some direction to make it so.

@stevearc
Copy link
Owner

The change looks great! To test it and make sure it's working I'd recommend either adding a new case in test_parser.py or simply adding some groups to this last test case:

(
"""[node name="Label" parent="."]
text = "Hello
"
""",
GDFile(
GDSection(GDSectionHeader("node", name="Label", parent="."), text="Hello\n")
),
),

Tests are currently failing because of lint. It looks like black was updated to have some different formatting preferences. You can apply those changes by running tox -e format.

@richard-gebbia
Copy link
Contributor Author

Thank you for the direction! I've done the formatting and added some test code to cover my additions. Please let me know if there's anything else I need to do.

@stevearc
Copy link
Owner

Thanks for the submission!

@stevearc stevearc merged commit c5c3279 into stevearc:master Jun 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants