-
Notifications
You must be signed in to change notification settings - Fork 171
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
Blocks POC (Rhino, Acad) #3519
Blocks POC (Rhino, Acad) #3519
Changes from 22 commits
9c65d1a
0648654
6313b22
43471a2
0c07f75
4137515
96be32d
2eb1f93
1e92a17
6d3b3db
31da29f
c69a781
5c1c560
4460265
f6ff940
e08d946
6cca93d
97d3f65
ba4556e
850eef8
6ae0a3f
bfd5d21
536fc6e
3991441
6ffe054
844dd02
0ee51df
33b2614
88cb939
52d1c65
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
namespace Speckle.Core.Models.Instances; | ||
|
||
/// <summary> | ||
/// Abstracts over <see cref="InstanceProxy"/> and <see cref="InstanceDefinitionProxy"/> for sorting and grouping in receive operations. | ||
/// </summary> | ||
public interface IInstanceComponent | ||
{ | ||
/// <summary> | ||
/// The maximum "depth" at which this <see cref="InstanceProxy"/> or <see cref="InstanceDefinitionProxy"/> was found. It's important to get right: as instances can be composed of other instances, we need to start from the deepest instance elements first when reconstructing them, starting with definitions first. | ||
/// </summary> | ||
public int MaxDepth { get; set; } | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
using System.Collections.Generic; | ||
|
||
namespace Speckle.Core.Models.Instances; | ||
|
||
/// <summary> | ||
/// A proxy class for an instance definition. | ||
/// </summary> | ||
public class InstanceDefinitionProxy : Base, IInstanceComponent | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. InstanceDefinitionProxy will be serialiseable and hence is derives from Base? I am circling Base atm with a view to us setting fire to it, at least in its current state, and moving to more concrete types, though I don't imagine we can avoid using it here if it is going to be serialized in place There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep! We can move things away from base when we get to it imho |
||
{ | ||
/// <summary> | ||
/// The original ids of the objects that are part of this definition, as present in the source host app. On receive, they will be mapped to corresponding newly created definition ids. | ||
/// </summary> | ||
public List<string> Objects { get; set; } // source app application ids for the objects | ||
|
||
/// <summary> | ||
/// The maximum "depth" at which this instance was found. It's important to get right: as instances can be composed of other instances, we need to start from the deepest instance elements first when reconstructing them. | ||
/// </summary> | ||
public int MaxDepth { get; set; } | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
using System.DoubleNumerics; | ||
|
||
namespace Speckle.Core.Models.Instances; | ||
|
||
/// <summary> | ||
/// A proxy class for an instance (e.g, a rhino block, or an autocad block reference). | ||
/// </summary> | ||
public class InstanceProxy : Base, IInstanceComponent | ||
{ | ||
/// <summary> | ||
/// The definition id as present in the original host app. On receive, it will be mapped to the newly created definition id. | ||
/// </summary> | ||
public string DefinitionId { get; set; } | ||
|
||
/// <summary> | ||
/// The transform of the instance reference. | ||
/// </summary> | ||
public Matrix4x4 Transform { get; set; } | ||
|
||
/// <summary> | ||
/// The units of the host application file. | ||
/// </summary> | ||
public string Units { get; set; } = Kits.Units.Meters; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How dare you mention Kits ;) We should move that definition and avoid binding tighter to it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure how at this stage? I'm hesitant to move this out of core right now... |
||
|
||
/// <summary> | ||
/// The maximum "depth" at which this instance was found. It's important to get right: as instances can be composed of other instances, we need to start from the deepest instance elements first when reconstructing them. | ||
/// </summary> | ||
public int MaxDepth { get; set; } | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems a very small interface and the name and the prop seem a bit incongruous, possibly you're expecting the interface to grow to have more props/ Just wondering if this is the right place. I can sort of see I think, but maybe worth a discussion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't expect it to grow (yet), i just need to sort both by max depth in the same pass. Not too clean, i agree, but then again could we live with this as a POC?