[SceneKit] SCNLight.[Get|Set]Attribute do not exist on MacCatalyst, so make it explicit that they aren't. #17266

rolfbjarne · 2023-01-17T09:58:29Z

No description provided.

… so adjust attributes accordingly. Note that this adds a [MacCatalyst (13,1)] attribute - that's the implicit current behavior, so I'm just making the current behavior explicit.

haritha-mohan · 2023-01-17T17:04:14Z

@rolfbjarne Just out of curiosity, I was reviewing the docs for SCNLight, and it only explicitly states that the methods are deprecated for the macOS platform..but in the code attrs are added for the tvOS, macOS, and macCatalyst. How do you decide which platforms should be addressed and which to not include? In this case it looks like the lighting attribute also applies for iPadOS, iOS, etc.

vs-mobiletools-engineering-service2 · 2023-01-17T17:45:40Z

✅ API diff for current PR / commit

Legacy Xamarin (No breaking changes)

~~iOS~~ (no change detected)
~~tvOS~~ (no change detected)
~~watchOS~~ (no change detected)
~~macOS~~ (no change detected)

NET (empty diffs)

~~iOS~~: (empty diff detected)
~~tvOS~~: (empty diff detected)
~~MacCatalyst~~: (empty diff detected)
~~macOS~~: (empty diff detected)

✅ API diff vs stable

Legacy Xamarin (No breaking changes)

iOS: vsdrops gist (No breaking changes)
tvOS: vsdrops gist (No breaking changes)
watchOS: vsdrops gist (No breaking changes)
macOS: vsdrops gist (No breaking changes)

.NET (No breaking changes)

iOS: vsdrops gist (No breaking changes)
tvOS: vsdrops gist (No breaking changes)
MacCatalyst: vsdrops gist (No breaking changes)
macOS: vsdrops gist (No breaking changes)
Microsoft.iOS vs Microsoft.MacCatalyst: vsdrops gist

Legacy Xamarin (stable) vs .NET

iOS: vsdrops gist
tvOS: vsdrops gist
macOS: vsdrops gist

ℹ️ Generator diff

Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)

Pipeline on Agent
Hash: eaf7f929c2d94155116a7a8b55964c56004704fd [PR build]

vs-mobiletools-engineering-service2 · 2023-01-17T17:49:22Z

💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻

✅ All tests on macOS M1 - Mac Big Sur (11.5) passed.

Pipeline on Agent
Hash: eaf7f929c2d94155116a7a8b55964c56004704fd [PR build]

vs-mobiletools-engineering-service2 · 2023-01-17T17:54:55Z

📚 [PR Build] Artifacts 📚

Packages generated

View packages

Pipeline on Agent XAMBOT-1036.Monterey'
Hash: eaf7f929c2d94155116a7a8b55964c56004704fd [PR build]

vs-mobiletools-engineering-service2 · 2023-01-17T21:32:57Z

🚀 [CI Build] Test results 🚀

Test results

✅ All tests passed on VSTS: simulator tests.

🎉 All 225 tests passed 🎉

Tests counts

✅ bcl: All 69 tests passed. Html Report (VSDrops) Download
✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests: All 1 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 7 tests passed. Html Report (VSDrops) Download
✅ framework: All 8 tests passed. Html Report (VSDrops) Download
✅ generator: All 2 tests passed. Html Report (VSDrops) Download
✅ interdependent_binding_projects: All 7 tests passed. Html Report (VSDrops) Download
✅ install_source: All 1 tests passed. Html Report (VSDrops) Download
✅ introspection: All 8 tests passed. Html Report (VSDrops) Download
✅ linker: All 65 tests passed. Html Report (VSDrops) Download
✅ mac_binding_project: All 1 tests passed. Html Report (VSDrops) Download
✅ mmp: All 2 tests passed. Html Report (VSDrops) Download
✅ mononative: All 12 tests passed. Html Report (VSDrops) Download
✅ monotouch: All 25 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ mtouch: All 1 tests passed. Html Report (VSDrops) Download
✅ xammac: All 3 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 8 tests passed. Html Report (VSDrops) Download
✅ xtro: All 2 tests passed. Html Report (VSDrops) Download

Pipeline on Agent
Hash: eaf7f929c2d94155116a7a8b55964c56004704fd [PR build]

rolfbjarne · 2023-01-18T06:49:12Z

@rolfbjarne Just out of curiosity, I was reviewing the docs for SCNLight, and it only explicitly states that the methods are deprecated for the macOS platform..but in the code attrs are added for the tvOS, macOS, and macCatalyst. How do you decide which platforms should be addressed and which to not include? In this case it looks like the lighting attribute also applies for iPadOS, iOS, etc.

Good question! There are two parts to this:

The documentation only speaks about macOS, nothing about any other platforms. This typically means the API in question is not available on those other platforms.
When in doubt (and even if not), it's usually a good idea to check the header files, since they're almost always the final source of truth. In this case, the line for the attributeForKey: selector says:

- (nullable id)attributeForKey:(NSString *)key API_DEPRECATED("Use SCNLight properties instead", macos(10.8, 10.10)) API_UNAVAILABLE(ios, tvos, watchos, macCatalyst);

so it's clear this member is not available on Mac Catalyst.

but in the code attrs are added for the tvOS, macOS, and macCatalyst.

not sure what you mean here, the existing attributes:

xamarin-macios/src/scenekit.cs

Lines 1281 to 1300 in 49d13fc

    
           #if XAMCORE_3_0 
        
           		[NoiOS] 
        
           #elif !MONOMAC 
        
           		[Obsolete ("Do not use; this method only exist in macOS, not in iOS.")] 
        
           #endif 
        
           		[NoTV] 
        
           		[Deprecated (PlatformName.MacOSX, 10, 10)] 
        
           		[Export ("attributeForKey:")] 
        
           		[return: NullAllowed] 
        
           		NSObject GetAttribute (NSString lightAttribute); 
        
           #if XAMCORE_3_0 
        
           		[NoiOS] 
        
           #elif !MONOMAC 
        
           		[Obsolete ("Do not use; this method only exist in macOS, not in iOS.")] 
        
           #endif 
        
           		[NoTV] 
        
           		[Deprecated (PlatformName.MacOSX, 10, 10)] 
        
           		[Export ("setAttribute:forKey:")] 
        
           		void SetAttribute ([NullAllowed] NSObject value, NSString attribuetKey);

say that:

These methods are not available on tvOS (as expected)
They're deprecated on macOS (as expected - note that since the framework (SceneKit) is available on macOS, any types + members are by default available unless stated otherwise, so only the deprecation attribute is in the source code)
Not available on iOS starting in XAMCORE_3_0 - this is clearly a mistake in the initial binding, as evidenced by the Obsolete attribute basically saying so.

…o make it explicit that they aren't. (xamarin#17266)

[SceneKit] SCNLight.[Get|Set]Attribute does not exist on MacCatalyst,…

883ec71

… so adjust attributes accordingly. Note that this adds a [MacCatalyst (13,1)] attribute - that's the implicit current behavior, so I'm just making the current behavior explicit.

rolfbjarne requested a review from dalexsoto as a code owner January 17, 2023 09:58

This comment has been minimized.

Sign in to view

mandel-macaque approved these changes Jan 17, 2023

View reviewed changes

Turns out it's not implicit?

eaf7f92

rolfbjarne changed the title ~~[SceneKit] SCNLight.[Get|Set]Attribute does not exist on MacCatalyst, so adjust attributes accordingly.~~ [SceneKit] SCNLight.[Get|Set]Attribute do not exist on MacCatalyst, so make it explicit that they aren't. Jan 18, 2023

rolfbjarne merged commit 4bf97d1 into xamarin:main Jan 18, 2023

rolfbjarne deleted the scnlight-attributes-maccatalyst branch January 18, 2023 06:50

dustin-wojciechowski pushed a commit to dustin-wojciechowski/xamarin-macios that referenced this pull request Jan 19, 2023

[SceneKit] SCNLight.[Get|Set]Attribute do not exist on MacCatalyst, s…

de074dc

…o make it explicit that they aren't. (xamarin#17266)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SceneKit] SCNLight.[Get|Set]Attribute do not exist on MacCatalyst, so make it explicit that they aren't. #17266

[SceneKit] SCNLight.[Get|Set]Attribute do not exist on MacCatalyst, so make it explicit that they aren't. #17266

rolfbjarne commented Jan 17, 2023 •

edited

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

haritha-mohan commented Jan 17, 2023 •

edited

vs-mobiletools-engineering-service2 commented Jan 17, 2023

vs-mobiletools-engineering-service2 commented Jan 17, 2023

vs-mobiletools-engineering-service2 commented Jan 17, 2023

vs-mobiletools-engineering-service2 commented Jan 17, 2023

rolfbjarne commented Jan 18, 2023

[SceneKit] SCNLight.[Get|Set]Attribute do not exist on MacCatalyst, so make it explicit that they aren't. #17266

[SceneKit] SCNLight.[Get|Set]Attribute do not exist on MacCatalyst, so make it explicit that they aren't. #17266

Conversation

rolfbjarne commented Jan 17, 2023 • edited

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

haritha-mohan commented Jan 17, 2023 • edited

vs-mobiletools-engineering-service2 commented Jan 17, 2023

✅ API diff for current PR / commit

✅ API diff vs stable

ℹ️ Generator diff

vs-mobiletools-engineering-service2 commented Jan 17, 2023

💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻

vs-mobiletools-engineering-service2 commented Jan 17, 2023

📚 [PR Build] Artifacts 📚

Packages generated

vs-mobiletools-engineering-service2 commented Jan 17, 2023

🚀 [CI Build] Test results 🚀

Test results

Tests counts

rolfbjarne commented Jan 18, 2023

rolfbjarne commented Jan 17, 2023 •

edited

haritha-mohan commented Jan 17, 2023 •

edited