speckleworks / SpeckleCore

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

Define full set of interfaces for client-side implementations #56

Closed radumg closed 6 years ago

radumg commented 6 years ago

Things like Speckle.RhinoConverter and its Speckle.RevitConverter cousin should implement the same ISpeckleConverter interface. This came out of issue 4 on SpeckleRevit repo where I realised there wasn't a base class or interface defined for what these converters should implement as a minimum.

Same goes for the clients themselves, for example the Rhino client defines an interface for itself in the client repo (filename doesn't match contents btw @didimitrie @fraguada ) but it doesn't look like Speckle.Core defines or has any opinions on what a client should implement.

Provisional list of interfaces required :

I might be off-base here, keen to hear what others think ?

fraguada commented 6 years ago

@radumg this seems like the right approach in general, though I would defer to those who know better than I about how to handle conversion in Revit. I think for the senders and the receivers it makes sense. This will put some pressure on SpeckleRhino to clean itself up a bit!

radumg commented 6 years ago

Thanks @fraguada - We routinely have to wrap/unwrap Revit classes so To/From extension methods are common i would say, but which ones are need is the question the interface would answer, providing a mini-roadmap of what the Revit converter would need.

For example, i assume to/from points, lines, curves etc are needed, but how far do we go with the other more complex things (breps 😍 ) which don't always have a direct equivalent in Revit ?

didimitrie commented 6 years ago

I'm really not sure how things map between geoms and revit geoms. I just know @teocomi said it's not simple, and i'll trust him on that.

I think the whole compatibility map should happen at one point. And mostly I'm interested in what happens for Revit <-> Revit communications - what other types would need to be added to the core, etc.

Right now there's the convention for ToSpeckle, ToNative extension methods, but by all means, you guys go ahead. Just as long as we don't end up here i'm happy...

didimitrie commented 6 years ago

We're dealing with reflection so no interface is needed. @jmirtsch tried it out, but then we're forcing other libs to depend on speckle core - which is not the way to go.

We're having the following conventions for .net converter patterns:

For a reference implementation, check the rhino/grasshopper converter.

This allows you to just load up an assembly and get going with minimal (in theory) fuss. In case you don't see your converter being loaded and speckle converts everything to abstract, then you might need to use a hack to make sure the assembly is actually loaded.

This is for now the most zero touch approach that we could have. Maybe it's going to change in the future, but for 1.x.x. I'll close for now.