fireclawthefox / DirectGuiDesigner

A Visual Editor for Panda3Ds Direct GUI
BSD 2-Clause "Simplified" License
25 stars 2 forks source link

Crash when entering invalid path to a path property #22

Closed Augustifolia closed 1 year ago

Augustifolia commented 1 year ago

Hi again, I have found a few more bugs.

When entering an invalid path to a path property (for example image or geom) the program crashes. This is because of the line updateElement[updateAttribute] = None, where updateElement and updateAttribute are undefined (this is in the method __createPathProperty). Replacing it with elementInfo.element[definition.internalName] = None fixes that issue.

I also noticed that if you first enter a valid path and then enter an invalid one the image is removed from the first state but still there for all other states. But if you first set the property to something else (for example None) before restoring the old value the old image is set for all states. Something like this (added in the exception clause of the setValue method of the propertyHelper):

                if definition.nullable:
                    elementInfo.element[propName] = None
                elementInfo.element[propName] = oldValue

This feels a bit hacky, but I haven't found a better solution. This also requires the image and geom properties to be marked as nullable, which seems reasonable to me.

fireclawthefox commented 1 year ago

Thanks for the report and detailed description, those are some good points. I think marking image and geom as nullable should be fine but I'll have to have another look into directGui to be sure about that.

Augustifolia commented 1 year ago

No problem, I'm just happy to help.

Sure, I did not find any issues with image and geom being nullable when testing, but it sounds reasonable to check.

fireclawthefox commented 1 year ago

After checking again, I can confirm for DirectFrame and all derived classes that the base properties image, geom and text all can be nullable. If any problems with that arise, it's a bug that should be fixed elsewhere.

Augustifolia commented 1 year ago

Alright, thank you for checking!