Remove unnecessary conditional defines from scenekit.cs #14503
Conversation
tj-devel709
added
do-not-merge
| @@ -2498,35 +2502,28 @@ interface SCNProgram : NSCopying, NSSecureCoding { | |||
| [BaseType (typeof (NSObject))] | |||
| [Model, Protocol] | |||
| interface SCNProgramDelegate { | |||
| #if MONOMAC | |||
| #if XAMCORE_3_0 | |||
There was a problem hiding this comment.
I don't believe removing XAMCORE_3_0 is a safe transformation.
There was a problem hiding this comment.
we already have XAMCORE_3_0, right? Since NET == XAMCORE_4_0, The if was just around a NoiOS, so it gives no harm since the method was not there.
There was a problem hiding this comment.
I removed the #if XAMCORE_3_0's because I believe the compiler would not let me add another [NoiOS] outside of it (since it had duplicate attributes) and when I didn't add it, xtro would complain that things were being set for iOS but were not really there. I could be misremembering the above though.
I figured that if I have to add a #if !XAMCORE_3_0 to add the [NoiOS] there as well and the fact that it was already inside a #if MONOMAC and not being able to be called for iOS anyways, that it would maybe be okay to remove.
Any thoughts on this? @mandel-macaque @chamons
There was a problem hiding this comment.
That's some convoluted code... in general removing XAMCORE_3_0 isn't a safe transformation, but in this particular case, the code inside the XAMCORE_3_0 condition didn't do anything, because it was also inside a MONOMAC condition, thus the XAMCORE_3_0 condition is safe to remove.
There was a problem hiding this comment.
Looks good to me, but I do agree with @chamons regarding the using being 3 times, move it to a single if.
| #if MONOMAC | ||
| [iOS (8,0)] |
There was a problem hiding this comment.
What was that IOS attr doing there?!?!
| @@ -2498,35 +2502,28 @@ interface SCNProgram : NSCopying, NSSecureCoding { | |||
| [BaseType (typeof (NSObject))] | |||
| [Model, Protocol] | |||
| interface SCNProgramDelegate { | |||
| #if MONOMAC | |||
| #if XAMCORE_3_0 | |||
There was a problem hiding this comment.
we already have XAMCORE_3_0, right? Since NET == XAMCORE_4_0, The if was just around a NoiOS, so it gives no harm since the method was not there.
| @@ -1784,15 +1790,13 @@ interface SCNSceneLoadingOptions { | |||
| [Export ("SCNSceneSourceLoading.OptionPreserveOriginalTopology")] | |||
| bool PreserveOriginalTopology { get; set; } | |||
|
|
|||
| #if !TVOS && !WATCH | |||
| // note: generator's StrongDictionary does not support No* attributes yet | |||
There was a problem hiding this comment.
This comment seems important: does the generator's StrongDictionary support No* attributes now? If not, we can't remove the #if conditions (and if it does, then you can remove the comment).
There was a problem hiding this comment.
@chamons would you happen to know this one?
There was a problem hiding this comment.
I would go check the generated code and see what it generates before and after your change.
| @@ -2498,35 +2502,28 @@ interface SCNProgram : NSCopying, NSSecureCoding { | |||
| [BaseType (typeof (NSObject))] | |||
| [Model, Protocol] | |||
| interface SCNProgramDelegate { | |||
| #if MONOMAC | |||
| #if XAMCORE_3_0 | |||
There was a problem hiding this comment.
That's some convoluted code... in general removing XAMCORE_3_0 isn't a safe transformation, but in this particular case, the code inside the XAMCORE_3_0 condition didn't do anything, because it was also inside a MONOMAC condition, thus the XAMCORE_3_0 condition is safe to remove.
❌ [CI Build] Tests failed on VSTS: simulator tests iOS ❌Tests failed on VSTS: simulator tests iOS. Test results2 tests failed, 106 tests passed.Failed tests
Pipeline on Agent XAMBOT-1030.Monterey |
❌ [CI Build] Tests failed on VSTS: simulator tests iOS ❌Tests failed on VSTS: simulator tests iOS. Test results2 tests failed, 106 tests passed.Failed tests
Pipeline on Agent XAMBOT-1017.Monterey' |
This PR works to remove the unneeded conditional defines and put in a better input for the generator!