Editor Crashes on Invalid Data in RequiredComponent Attributes, Error Handling Needed

LaughingLeader commented 6 years ago

Summary

Not sure entirely what's triggering this, but at some point when rebuilding my plugins, the Duality Editor decides to continually crash.

How to reproduce [Bugs]

So as far as I know, these are the steps I took:

Writing an editor and core plugin.
Adding a custom component from the core plugin to my project view.
Rebuilding the plugins a few times on changing/adding things.
In trying to figure out why my editor plugin's importer class wasn't working, Adam pointed out that it wasn't public (doh!). Switched it to public and rebuilt.
Duality Editor crashes on each launch.

Extra:

Removing that public line from the importer class doesn't fix the crashes. As far as I'm aware, that had nothing to do with the actual crash - It seemed to just be the trigger for corrupting whatever's crashing the editor.

Workaround [Bugs]

Create a new, fresh project.
Copy code over from previous project.
Rebuild and the editor works as it should.

Attachments

logfile_editor.txt
(Crashing Project) BaseGame.zip

Note: I had to delete the contents of Source/Packages to get the zip below the file size limit. The packages were the standard Duality stuff:

AdamsLair.DockPanelSuite.2.8.1.0 AdamsLair.Duality.2.12.9 AdamsLair.Duality.Backend.DefaultOpenTK.2.0.19 AdamsLair.Duality.Backend.DotNetFramework.2.1.1 AdamsLair.Duality.Docs.2.0.1 AdamsLair.Duality.Editor.2.9.1 AdamsLair.Duality.Editor.Backend.DefaultOpenTK.2.0.5 AdamsLair.Duality.Editor.Plugins.Base.2.2.7 AdamsLair.Duality.Editor.Plugins.CamView.2.5.1 AdamsLair.Duality.Editor.Plugins.HelpAdvisor.2.0.4 AdamsLair.Duality.Editor.Plugins.LogView.2.1.8 AdamsLair.Duality.Editor.Plugins.ObjectInspector.2.0.5 AdamsLair.Duality.Editor.Plugins.PackageManagerFrontend.2.1.1 AdamsLair.Duality.Editor.Plugins.ProjectView.2.0.11 AdamsLair.Duality.Editor.Plugins.SceneView.2.0.12 AdamsLair.Duality.Launcher.2.3.3 AdamsLair.Duality.Plugins.Compatibility.2.0.2 AdamsLair.Duality.Primitives.2.0.4 AdamsLair.FarseerDuality.4.1.4 AdamsLair.NVorbis.0.7.5 AdamsLair.OpenTK.1.1.8 AdamsLair.OpenTK.GLControl.1.1.8 AdamsLair.TreeViewAdv.1.7.6.0 AdamsLair.WinForms.1.1.12 AdamsLair.WinForms.PopupControl.1.0.0.0 IconCache Microsoft.Web.Xdt.2.1.1 NuGet.Core.2.8.3

ilexp commented 6 years ago

First quick look at the callstack turns up this line as the one throwing the exception, which could happen when there is a Component type in the constraint graph that is not part of the list of Component types to be ordered.

Immediate ToDo

Investigate GatherConstraints in order to find out how such a type could end up in the graph.
Also investigate whether the project reproducing this has any RequiredComponent or execution order related attributes referring to types that are (a) not Components or (b) not public / exported types, and if that turns out to be the cause...
...deploy some countermeasures in code to detect and warn about this in the future, while also not crashing.
Extend existing unit tests to include a regression case for this.

LaughingLeader commented 6 years ago

After further testing it appears it was my [RequiredComponent] calls on my SpriteSheetInterpreter.cs class. Removing those fixes the crashes.

ilexp commented 6 years ago

Ah, that makes sense. This seems to be the class in question:

    [RequiredComponent(typeof(Pixmap), typeof(JsonData))]
    public class SpriteSheetInterpreter : Component
    {
        // ...
    }

The RequiredComponent attribute should only be used between component types, and it's also using the wrong constructor here (required / create default distinction) where multiple attributes would have been intended, like so:

[RequiredComponent(typeof(Foo))]
[RequiredComponent(typeof(Bar))]

Regardless, the editor of course shouldn't crash. Instead, there should be a warning about unintended usage and a proper error handling when something like this shows up in a plugin.

LaughingLeader commented 6 years ago

Yep! Sorry about that. I realized when going back through, after separating the required attributes, that those types aren't even components (like what's mentioned in the attribute name itself).

My misunderstanding carried over from requiring a RigidBody on another component - My brain went, "This class needs these two things, so I should do it this way, right?" Without realizing that attribute is for components only, not resources.

ilexp commented 6 years ago

Fixed, and release in progress. Package update should be out there in about 15 minutes.

AdamsLair / duality