BHoM / BHoM_Adapter

GNU Lesser General Public License v3.0
7 stars 6 forks source link

Adapter_oM: ActionConfig to store a ComparisonConfig instead of DiffingConfig #271

Closed alelom closed 3 years ago

alelom commented 3 years ago

Broken rules:

The only property needed by the Adapter is ComparisonConfig. Should be fixed urgently as this should solve the Versioning on all ActionConfigs.

Suggestions to restore compliance:

alelom commented 3 years ago

After our conversation, the scenario where this can actually be an issue is only when the ActionConfig is manually serialized/deserialized (ToString) and/or internalised into a Grasshopper component.

I have checked that all ActionConfigs remain correctly placed on the canvas when a script written in 3.3 is opened in 4.0, see below. I am testing both Auto-created initialisers and Create methods.

Script file: https://burohappold.sharepoint.com/:f:/s/BHoM/Ep0GE9mLnjZNsT9vwoSQH9kBbHiMoQr7dRChPnZ2JoX6hA?e=6mpKlT

Script written and seen in v3.3

![complete script](https://user-images.githubusercontent.com/6352844/102207858-90e05600-3ec6-11eb-9e53-f0f154ae0b58.png)

Same script opened in 4.0

![complete script](https://user-images.githubusercontent.com/6352844/102209860-51673900-3ec9-11eb-9428-626bd2ebcebd.png)

The DiffConfig is upgraded to DiffingConfig in all instances. This means the severity of this issue can be reduced significantly.

@pawelbaran if you could confirm that the same is working for you, I think we don't need to add any custom converter in the Versioning.

pawelbaran commented 3 years ago

Just tested, I confirm the autogenerated constructors created in 3.3 work fine on 4.0, I believe we should be OK without a custom converter 👍

alelom commented 3 years ago

Ok, closing for now then.

alelom commented 3 years ago

Problem: by saving a grasshopper script with ActionConfigs instantiated with auto-created initializers in 4.0, an error appears

image

The strange thing is that this does not happen consistently for all ActionConfigs – some just work: LusasConfig, PullGeometryConfig and PullRepresentationConfig. edit: it's simply because those 3 configs do not inherit from the Base ActionConfig. All Configs inheriting from ActionConfig fail.

See all ActionConfigs here:

![ActionConfiginitializers](https://user-images.githubusercontent.com/6352844/102597509-ad70ce00-4112-11eb-967f-24644b8aaecc.png)
alelom commented 3 years ago

Problem: by saving a grasshopper script with ActionConfigs instantiated with auto-created initializers in 4.0, an error appears

This seems to be a UX problem: