Skip to content

[arkit] Update for Xcode 9 GM - #2681

Merged
spouliot merged 3 commits into
dotnet:xcode9from
VincentDondain:arkit-gm
Sep 14, 2017
Merged

[arkit] Update for Xcode 9 GM#2681
spouliot merged 3 commits into
dotnet:xcode9from
VincentDondain:arkit-gm

Conversation

@VincentDondain

Copy link
Copy Markdown
Contributor

No description provided.

@rolfbjarne rolfbjarne left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor thing, otherwise 👍.

Comment thread src/arkit.cs Outdated
[Export ("vertexCount")]
nuint VertexCount { get; }

// @property (readonly, nonatomic) const vector_float3 * _Nonnull vertices;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment can be removed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup missed. I'll remove in next PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rolfbjarne that's minor though, not PR blocking, I'll fix in next PR.

Comment thread src/arkit.cs

[iOS (11,0)]
[NoWatch, NoTV, NoMac]
[BaseType (typeof(ARLightEstimate))]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

base type has a [DisableDefaultCtor] so the subclass should too

@VincentDondain VincentDondain Sep 13, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right ARDirectionalLightEstimate subclass doesn't have init

Comment thread src/arkit.cs

[iOS (11,0)]
[NoWatch, NoTV, NoMac]
[BaseType (typeof(ARConfiguration))]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's a [DisableDefaultCtor] on the base type
but in this case it might be valid but...
The abstract base class for AR session configurations. https://developer.apple.com/documentation/arkit/arconfiguration

so there's an [Abstract] missing from ARConfiguration

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ARConfiguration should indeed be [Abstract] and [DisableDefaultCtor] is right here (:

/** Unavailable */
- (instancetype)init NS_UNAVAILABLE;
+ (instancetype)new NS_UNAVAILABLE;

@monojenkins

Copy link
Copy Markdown
Contributor

Build success

Comment thread src/arkit.cs Outdated
[iOS (11,0)]
[NoWatch, NoTV, NoMac]
[BaseType (typeof(ARConfiguration))]
[DisableDefaultCtor]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry if it was confusing ;-)
but in this case it might be valid but...
meant that I did not knew if init made sense - because the base type did not have it but was abstract
It turns out it's valid
https://developer.apple.com/documentation/arkit/arorientationtrackingconfiguration/2923556-init?language=objc

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sorry I misinterpreted that and thought you checked (:

Comment thread src/arkit.cs Outdated
[iOS (11,0)]
[NoWatch, NoTV, NoMac]
[BaseType (typeof(ARConfiguration))]
[DisableDefaultCtor]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

double check headers/doc - I suspect it's available

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/arkit.cs Outdated
[iOS (11,0)]
[NoWatch, NoTV, NoMac]
[BaseType (typeof (ARConfiguration))]
[DisableDefaultCtor]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

double check headers/doc - I suspect it's available

@monojenkins

Copy link
Copy Markdown
Contributor

Build failure

@monojenkins

Copy link
Copy Markdown
Contributor

Build failure

@spouliot

Copy link
Copy Markdown
Contributor

aborted build after 4 hours

@spouliot

Copy link
Copy Markdown
Contributor

build

@monojenkins

Copy link
Copy Markdown
Contributor

Build success

@spouliot
spouliot merged commit 05dde4c into dotnet:xcode9 Sep 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants