Skip to content
New issue

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

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

Already on GitHub? # to your account

Added warning and skip objects with negative scale #367

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

xphlawlessx
Copy link
Contributor

Added a warning and ignore objects with negative scale, as a basic way to deal with Issue #24

Copy link
Collaborator

@Jason0214 Jason0214 left a comment

Choose a reason for hiding this comment

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

Thanks for the change!

@@ -156,6 +156,10 @@ def export_object(self, obj, parent_gd_node):

def should_export_object(self, obj):
"""Checks if a node should be exported:"""
for dimension in obj.scale:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just for your information, looks like scale with all dimensions being negative is valid in Godot (godotengine/godot#26173), but with some limitations.
I am okay with your implementation by skipping any negative scale object.

@@ -156,6 +156,10 @@ def export_object(self, obj, parent_gd_node):

def should_export_object(self, obj):
"""Checks if a node should be exported:"""
for dimension in obj.scale:
if dimension < 0:
logging.info("Negative scale is unsupported, Object '%s' was skipped", obj.name)
Copy link
Collaborator

@Jason0214 Jason0214 Oct 31, 2020

Choose a reason for hiding this comment

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

May be better to use logging.warning(), we have a hook to show warning messages to the GUI.
Btw pep is not happy with this line being too long, wrap at 80 characters will fix it :D
Thanks!

@xphlawlessx
Copy link
Contributor Author

xphlawlessx commented Oct 31, 2020 via email

Sorry it took so long, I'm quite new at using Git.
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants