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

NullReferenceException when trying to use Reactive Properties with generic types #61

Open
OmiCron07 opened this issue Sep 9, 2018 · 8 comments
Labels
bug editor An issue effecting the optional editors library

Comments

@OmiCron07
Copy link

OmiCron07 commented Sep 9, 2018

NullReferenceException: Object reference not set to an instance of an object
EcsRx.Unity.JSONNode.set_AsVector3 (UnityEngine.Vector3 value) (at Assets/EcsRx/Unity/JSON.cs:949)
EcsRx.Unity.JSONData..ctor (UnityEngine.Vector3 value) (at Assets/EcsRx/Unity/JSON.cs:428)
EcsRx.Unity.Extensions.ComponentExtensions.SerializeComponent (System.Object component) (at Assets/EcsRx/Unity/Extensions/ComponentExtensions.cs:91)
EcsRx.Unity.RegisterAsEntityInspector+c__AnonStorey0.<>m__0 () (at Assets/EcsRx/Unity/Editor/RegisterAsEntityInspector.cs:83)
EcsRx.Unity.Extensions.EditorExtensions.UseVerticalBoxLayout (UnityEditor.Editor editor, System.Action action) (at Assets/EcsRx/Unity/Editor/Extensions/EditorExtensions.cs:17)
EcsRx.Unity.RegisterAsEntityInspector.ComponentListings () (at Assets/EcsRx/Unity/Editor/RegisterAsEntityInspector.cs:51)
EcsRx.Unity.RegisterAsEntityInspector.OnInspectorGUI () (at Assets/EcsRx/Unity/Editor/RegisterAsEntityInspector.cs:134)
UnityEditor.InspectorWindow.DrawEditor (UnityEditor.Editor[] editors, System.Int32 editorIndex, System.Boolean rebuildOptimizedGUIBlock, System.Boolean& showImportedObjectBarNext, UnityEngine.Rect& importedObjectBarRect) (at C:/buildslave/unity/build/Editor/Mono/Inspector/InspectorWindow.cs:1367)
UnityEngine.GUIUtility:ProcessEvent(Int32, IntPtr)

I added RegisterAsEntity script to a MonoBehavior, via the editor, and on this, I added a component with a Vector3ReactiveProperty in it. But I remark your rogue example have the same problem if you open the components with Vector3 as RamdomlyPlacedComponent.

@grofit
Copy link
Member

grofit commented Sep 10, 2018

I think this is because the editor part only knows about basic types and flips out on unknown types, this is somewhat more extensible in the newer editor stuff, problem is to bring that in I need to add a dependency on Json.Net, so will discuss that in channel.

@grofit grofit added the bug label Sep 10, 2018
@OmiCron07
Copy link
Author

Unity editor can serialize Vector3 and Vector3ReactiveProperty fields. I'm wondering if your JSON class is not obsolete since Unity provide built-in JSON serialization now that works with primitive/Unity types?

@grofit
Copy link
Member

grofit commented Sep 11, 2018

Potentially, but the built in JSON serializer in unity is not that great at handling other use cases outside of unity, the JSON file included is something I wanted to remove for a while, as its basically just an open source drop in JSON implementation (which was taken from uFrame, as it was used there, and they took it from the unify wiki).

The newer editor version removes any need for that and also supports serializing to binary (or xml/json) but as I say it introduces a new dependency on a JSON.Net implementation being available, which may or may not be a problem for other devs but its a topic of discussion at the moment, as if everyone is ok with pulling that in I can move forward with the new editor stuff, which lets you make your own fields a bit easier too. As an interim measure it may be possible to remove this custom JSON file and move to use the unity one for now if that works for you, are you able to give that a try? (I think there are only a few lines of JSON serializer code usage in the editor scripts which can just point to unity's JsonUtility or whatever its called).

If that works let me know and I will look at merging a PR on it or just removing the JSON bit and moving to unity's built in one for now.

@grofit
Copy link
Member

grofit commented Sep 11, 2018

I have just had a quick try with the JSON serializer built into unity and it only works with public fields, not properties, so it is not really useable for this scenario, there is the built in DataContractSerializer or alternatively we all bite the bullet and adopt JSON.NET

@OmiCron07
Copy link
Author

I cringe a little when I have to write public fields instead of properties for Unity can serialize them... So I vote for JSON.Net.

@grofit
Copy link
Member

grofit commented Sep 14, 2018

There is a bit of an issue at the moment trying to have a library that is dependent upon JSON.NET from nuget work with unitys version of JSON.NET (I was originally informed that the asset store JSON project would satisfy the dependency as it mimics the same assembly details, but doesnt seem to work, so looking into that atm)

@grofit grofit changed the title NullReferenceException NullReferenceException when trying to use Reactive Properties with generic types Sep 14, 2018
@grofit
Copy link
Member

grofit commented Jan 21, 2019

In the new editor it has moved to use binary serialization which bypasses some of these problems but I am not sure if this would trigger AOT Issues so although the problem to some extent can be mitigated better, but it may make more sense to move the editor stuff into its own plugin so it can be maintained separately and added to without effecting the core and unity layer, so this is something I will look into when I get time.

@grofit
Copy link
Member

grofit commented Oct 31, 2024

The binary serialization stuff was all done as part of an older branch which ended up never making it into main, so currently this issue will still exhibit and I am not really sure how to better handle it as it needs a more complex serializer, maybe Unity has moved on more in this area.

We solved this originally by writing a custom serialization layer which ended up being spawned into its own library https://github.com/grofit/LazyData

If there is an appetite for this we can revisit this as part of #68 but currently I am just making knee jerk decisions based off my perspective of it and could do with more community steering on the issue before I go ahead either way.

@grofit grofit added the editor An issue effecting the optional editors library label Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug editor An issue effecting the optional editors library
Projects
None yet
Development

No branches or pull requests

2 participants