speckleworks / SpeckleRhino

Check a brand new Speckle at: https://github.com/specklesystems
https://speckle.works
MIT License
38 stars 16 forks source link

Cannot attach Custom User Data to Speckle Objects. #310

Closed daviddekoning closed 4 years ago

daviddekoning commented 4 years ago

Step 0:

Expected Behaviour

Creating custom user data, then adding that to a Speckle Object should not give an error.

Actual Behaviour

The Create Custom User Data component is producing an ArchivableDictionary, and the Set User Data Speckle Object is trying to cast it to Dictionary.

Affected Projects

I believe that this is limited to GH.

Reproduction Steps & System Config (win, osx, web, etc.)

image

Proposed Solution (if any)

Perhaps this can be fixed by casting to IDictionary instead of Dictionary here: https://github.com/speckleworks/SpeckleRhino/blob/af0fc2b79fd8bcc043ad54390dc2621097efcf2f/SpeckleGrasshopper/UserDataUtils/SetUserDataSpeckleObjectComponent.cs#L61.

(It appears that this behaviour was changed in 9896f027202c4154426b87a5916da9f88dff9f9d).

The workaround is to attach data to object before converting them to Speckle Objects.

daviddekoning commented 4 years ago

btw, I am hoping to have a colleague give the GH client some love shortly. I am flagging this here to see if this is a big issue for anyone else.

teocomi commented 4 years ago

Thanks @daviddekoning ,

This will be greatly improved in 2.0. In the meantime you can use the normal "Set User Data" component. And please make sure your colleague joins the insider program so we can coordinate re GH client development :)

image

daviddekoning commented 4 years ago

The fundamental issue here is the mismatch between this line:

https://github.com/speckleworks/SpeckleRhino/blob/ca0497d714dde9d230a8e0232f2d9885282e0841/SpeckleGrasshopper/UserDataUtils/CreateUserData.cs#L76

and this line:

https://github.com/speckleworks/SpeckleRhino/blob/af0fc2b79fd8bcc043ad54390dc2621097efcf2f/SpeckleGrasshopper/UserDataUtils/SetUserDataSpeckleObjectComponent.cs#L61

There are three possible solutions:

  1. Make CreateUserData output a Dictionary
  2. Make SetUserDataSpeckleObject accept an ArchivableDictionary and convert it to a Dictionary
  3. Update the base SpeckleObject class so that it's properties property is an IDictionary.

1 and 3 touch have a higher chance of a affecting other items.

@psarras any thoughts?

daviddekoning commented 4 years ago

This is fixed in #311.

I followed a slightly modified version of item 2 above. Instead of only accepting an ArchivableDictionary, the component now accepts any class that implements the IDictionary interface, and converts it to a Dictionary.

In addition, if the SpeckleObject to which the data is being attached already has custom user data, the component now adds the new data, instead of throwing out all the old properties and replacing them by the new ones. If a user data key exists in the both the old and the new user data, the new data overrights the old (on a key by key basis).