RockTomate / Issues

Submit bugs and feature requests.
4 stars 1 forks source link

Step class renaming breaks job execution #8

Open DmitriyYukhanov opened 4 years ago

DmitriyYukhanov commented 4 years ago

Steps to Reproduce

NullReferenceException: Object reference not set to an instance of an object HardCodeLab.RockTomate.Core.Steps.Scope.StopAll () (at Library/PackageCache/com.modumlab.thirdparty.tools.rocktomate@1.0.0/RockTomate/Scripts/Core/Data/Session/Scope.cs:197) HardCodeLab.RockTomate.Core.Data.JobSession.Stop () (at Library/PackageCache/com.modumlab.thirdparty.tools.rocktomate@1.0.0/RockTomate/Scripts/Core/Data/Session/JobSession.cs:431) HardCodeLab.RockTomate.Core.Managers.JobSessionController.StopSession () (at Library/PackageCache/com.modumlab.thirdparty.tools.rocktomate@1.0.0/RockTomate/Scripts/Core/Managers/JobSessionController.cs:226) HardCodeLab.RockTomate.Editor.Windows.JobEditorWindow.StopJob () (at Library/PackageCache/com.modumlab.thirdparty.tools.rocktomate@1.0.0/RockTomate/Scripts/Editor/Windows/JobEditorWindow.cs:134) HardCodeLab.RockTomate.Editor.Windows.JobEditorWindow.OnToolbarGUI () (at Library/PackageCache/com.modumlab.thirdparty.tools.rocktomate@1.0.0/RockTomate/Scripts/Editor/Windows/JobEditorWindow.cs:110) HardCodeLab.RockTomate.Editor.Windows.JobEditorWindow.OnGUI () (at Library/PackageCache/com.modumlab.thirdparty.tools.rocktomate@1.0.0/RockTomate/Scripts/Editor/Windows/JobEditorWindow.cs:198) UnityEditor.HostView.InvokeOnGUI (UnityEngine.Rect onGUIPosition, UnityEngine.Rect viewRect) (at <92f998fbb4ca4d8dab7793d6e003b794>:0) UnityEditor.DockArea.DrawView (UnityEngine.Rect viewRect, UnityEngine.Rect dockAreaRect) (at <92f998fbb4ca4d8dab7793d6e003b794>:0) UnityEditor.DockArea.OldOnGUI () (at <92f998fbb4ca4d8dab7793d6e003b794>:0) UnityEngine.UIElements.IMGUIContainer.DoOnGUI (UnityEngine.Event evt, UnityEngine.Matrix4x4 parentTransform, UnityEngine.Rect clippingRect, System.Boolean isComputingLayout, UnityEngine.Rect layoutSize, System.Action onGUIHandler, System.Boolean canAffectFocus) (at <e40488dcb2e3475fad9ef37a4468eea0>:0) UnityEngine.UIElements.IMGUIContainer.HandleIMGUIEvent (UnityEngine.Event e, UnityEngine.Matrix4x4 worldTransform, UnityEngine.Rect clippingRect, System.Action onGUIHandler, System.Boolean canAffectFocus) (at <e40488dcb2e3475fad9ef37a4468eea0>:0) UnityEngine.UIElements.IMGUIContainer.HandleIMGUIEvent (UnityEngine.Event e, System.Action onGUIHandler, System.Boolean canAffectFocus) (at <e40488dcb2e3475fad9ef37a4468eea0>:0) UnityEngine.UIElements.IMGUIContainer.HandleIMGUIEvent (UnityEngine.Event e, System.Boolean canAffectFocus) (at <e40488dcb2e3475fad9ef37a4468eea0>:0) UnityEngine.UIElements.IMGUIContainer.SendEventToIMGUIRaw (UnityEngine.UIElements.EventBase evt, System.Boolean canAffectFocus, System.Boolean verifyBounds) (at <e40488dcb2e3475fad9ef37a4468eea0>:0) UnityEngine.UIElements.IMGUIContainer.SendEventToIMGUI (UnityEngine.UIElements.EventBase evt, System.Boolean canAffectFocus, System.Boolean verifyBounds) (at <e40488dcb2e3475fad9ef37a4468eea0>:0) UnityEngine.UIElements.IMGUIContainer.HandleEvent (UnityEngine.UIElements.EventBase evt) (at <e40488dcb2e3475fad9ef37a4468eea0>:0) UnityEngine.UIElements.CallbackEventHandler.HandleEventAtTargetPhase (UnityEngine.UIElements.EventBase evt) (at <e40488dcb2e3475fad9ef37a4468eea0>:0) UnityEngine.UIElements.MouseCaptureDispatchingStrategy.DispatchEvent (UnityEngine.UIElements.EventBase evt, UnityEngine.UIElements.IPanel panel) (at <e40488dcb2e3475fad9ef37a4468eea0>:0) UnityEngine.UIElements.EventDispatcher.ApplyDispatchingStrategies (UnityEngine.UIElements.EventBase evt, UnityEngine.UIElements.IPanel panel, System.Boolean imguiEventIsInitiallyUsed) (at <e40488dcb2e3475fad9ef37a4468eea0>:0) UnityEngine.UIElements.EventDispatcher.ProcessEvent (UnityEngine.UIElements.EventBase evt, UnityEngine.UIElements.IPanel panel) (at <e40488dcb2e3475fad9ef37a4468eea0>:0) UnityEngine.UIElements.EventDispatcher.ProcessEventQueue () (at <e40488dcb2e3475fad9ef37a4468eea0>:0) UnityEngine.UIElements.EventDispatcher.OpenGate () (at <e40488dcb2e3475fad9ef37a4468eea0>:0) UnityEngine.UIElements.EventDispatcherGate.Dispose () (at <e40488dcb2e3475fad9ef37a4468eea0>:0) UnityEngine.UIElements.EventDispatcher.ProcessEvent (UnityEngine.UIElements.EventBase evt, UnityEngine.UIElements.IPanel panel) (at <e40488dcb2e3475fad9ef37a4468eea0>:0) UnityEngine.UIElements.EventDispatcher.Dispatch (UnityEngine.UIElements.EventBase evt, UnityEngine.UIElements.IPanel panel, UnityEngine.UIElements.DispatchMode dispatchMode) (at <e40488dcb2e3475fad9ef37a4468eea0>:0) UnityEngine.UIElements.BaseVisualElementPanel.SendEvent (UnityEngine.UIElements.EventBase e, UnityEngine.UIElements.DispatchMode dispatchMode) (at <e40488dcb2e3475fad9ef37a4468eea0>:0) UnityEngine.UIElements.UIElementsUtility.DoDispatch (UnityEngine.UIElements.BaseVisualElementPanel panel) (at <e40488dcb2e3475fad9ef37a4468eea0>:0) UnityEngine.UIElements.UIElementsUtility.UnityEngine.UIElements.IUIElementsUtility.ProcessEvent (System.Int32 instanceID, System.IntPtr nativeEventPtr, System.Boolean& eventHandled) (at <e40488dcb2e3475fad9ef37a4468eea0>:0) UnityEngine.UIElements.UIEventRegistration.ProcessEvent (System.Int32 instanceID, System.IntPtr nativeEventPtr) (at <e40488dcb2e3475fad9ef37a4468eea0>:0) UnityEngine.UIElements.UIEventRegistration+<>c.<.cctor>b__1_2 (System.Int32 i, System.IntPtr ptr) (at <e40488dcb2e3475fad9ef37a4468eea0>:0) UnityEngine.GUIUtility.ProcessEvent (System.Int32 instanceID, System.IntPtr nativeEventPtr, System.Boolean& result) (at <faa31f22dbbe43159226eb3a4f009ff2>:0)

Expected Results Ideally, step just renames in Job, or at least job does not fails to run

Actual Results Renamed step disappears from job list and job fails to run

ElmarTalibzade commented 4 years ago

At the moment, I don't know how to auto-rename a step if its class has been renamed. I need to somehow "find out" the new name of the step, then copy its values over. I am open to suggestions on that.

Least I can do is mark the step has invalid and not cause any errors.

DmitriyYukhanov commented 4 years ago

I didn't inspected deeply how you store references to the step scripts but I believe Unity's serialization system allows to keep references after class renaming since I can see component renaming does not leads to the missing script error in Unity itself.

Are you using Unity's serialization to track references to the steps?

BTW marking step as invalid already looks totally fine to me, though it still will be very painful to lose all configured settings of all renamed step instances =(

ElmarTalibzade commented 4 years ago

Are you using Unity's serialization to track references to the steps?

I am using OdinSerializer for serialization.

BTW marking step as invalid already looks totally fine to me, though it still will be very painful to lose all configured settings of all renamed step instances =(

Thing is, how often do you rename Step class names? If you want to change the display name of the step, you can use a Name attribute in StepDescription instead.

DmitriyYukhanov commented 4 years ago

I am using OdinSerializer for serialization.

Ah, I'm not sure how they handle such case, never worked with Odin serializer before =\

Thing is, how often do you rename Step class names? If you want to change the display name of the step, you can use a Name attribute in StepDescription instead.

Thank you for this tip, but in my case class rename was needed to keep proper architecture of my custom steps (e.g. I created Step1 with some functionality but later I found that I need similar step with some additional features so I needed to rename Step1 to SimpleStep1 \ BaseStep1 etc.).

Also it's a common practice to rename classes during refactoring phase in order to give them more clear and meaningful names if it was not done initially.

I'm afraid without renamed classes reference tracking you may have more similar bug reports in the future since it's a default behaviour in Unity - to keep reference of renamed classes.

ElmarTalibzade commented 4 years ago

I would need to get in touch with Odin Serializer development team to discuss this issue. At the moment, I am not confident that this would be fixed in 1.0.3.