-
Notifications
You must be signed in to change notification settings - Fork 507
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
[Xcode12][GameKit] Update for beta 1-2 #9126
[Xcode12][GameKit] Update for beta 1-2 #9126
Conversation
src/GameKit/GKLeaderboardSet.cs
Outdated
if (UIKit.UIDevice.CurrentDevice.CheckSystemVersion (14, 0)) | ||
#elif WATCH | ||
if (WatchKit.WKInterfaceDevice.CurrentDevice.CheckSystemVersion (7, 0)) | ||
#elif MONOMAC |
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.
there's a #if !MONOMAC
earlier
src/GameKit/GKLeaderboardSet.cs
Outdated
namespace GameKit { | ||
public partial class GKLeaderboardSet { | ||
|
||
public void LoadLeaderboardsWithHandler (GKLeaderboardsHandler completionHandler) |
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.
remove WithHandler
since it's implied
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.
ideally there would be a [Async]
version of the API
It will need to be manually added in this case
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.
Unfortunately there's already an API named LoadLeaderboards
in GKLeaderboardSet
namespace.
The group of related APIs have this (not great) naming scheme:
LoadLeaderboards
- deprecated, generatedLoadLeaderboardsWithCompletionHandler
- new! generatedLoadLeaderboardsWithHandler
- new! manual wrapper for the above
So we can change the names of 2 & 3 but not 1.
Ideas for other names very welcome!
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.
are the parameter names blocking the use of overloads ?
if so then I don't have any better idea atm
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.
Yep, they all have the same signature, (GKLeaderboardsHandler completionHandler)
😕
@@ -32,3 +32,6 @@ | |||
!missing-null-allowed! 'UIKit.UIViewController GameKit.GKAchievement::ChallengeComposeController(GameKit.GKPlayer[],System.String,GameKit.GKChallengeComposeHandler)' is missing an [NullAllowed] on parameter #0 | |||
!missing-null-allowed! 'UIKit.UIViewController GameKit.GKAchievement::ChallengeComposeController(GameKit.GKPlayer[],System.String,GameKit.GKChallengeComposeHandler)' is missing an [NullAllowed] on return type | |||
!missing-null-allowed! 'UIKit.UIViewController GameKit.GKScore::ChallengeComposeController(System.String[],System.String,GameKit.GKChallengeComposeHandler)' is missing an [NullAllowed] on return type | |||
!missing-null-allowed! 'System.String GameKit.GKAchievement::get_PlayerID()' is missing an [NullAllowed] on return type | |||
!missing-null-allowed! 'System.String[] GameKit.GKMatch::get_PlayersIDs()' is missing an [NullAllowed] on return type | |||
!missing-null-allowed! 'System.String[] GameKit.GKVoiceChat::get_PlayerIDs()' is missing an [NullAllowed] on return type |
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.
please add them :)
that will make it easier to complete nullability support (at some point) if all new API already have them
@@ -45,3 +45,5 @@ | |||
!missing-null-allowed! 'System.Void GameKit.GKDialogController::set_ParentWindow(AppKit.NSWindow)' is missing an [NullAllowed] on parameter #0 | |||
!missing-null-allowed! 'System.Void GameKit.GKGameSession::SendMessage(System.String,System.String[],Foundation.NSData,GameKit.GKCloudPlayer[],System.Boolean,System.Action`1<Foundation.NSError>)' is missing an [NullAllowed] on parameter #2 | |||
!missing-null-allowed! 'System.Void GameKit.GKMatchmakerViewController::set_DefaultInvitationMessage(System.String)' is missing an [NullAllowed] on parameter #0 | |||
!missing-null-allowed! 'System.String[] GameKit.GKMatch::get_PlayersIDs()' is missing an [NullAllowed] on return type | |||
!missing-null-allowed! 'System.String[] GameKit.GKVoiceChat::get_PlayerIDs()' is missing an [NullAllowed] on return type |
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.
same
src/gamekit.cs
Outdated
|
||
[TV (14, 0), NoWatch, Mac (10, 16), iOS (14, 0)] | ||
[Field ("GKPlayerIDNoLongerAvailable")] | ||
NSString GKPlayerIdNoLongerAvailable { get; } |
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.
we normally remove the prefix on fields, e.g. IdNoLongerAvailable
when inside the type GKPlayer
src/gamekit.cs
Outdated
@@ -803,6 +939,11 @@ interface GKLocalPlayer | |||
[Static] | |||
[Export ("local")] | |||
GKLocalPlayer Local { get; } | |||
|
|||
[NoWatch] | |||
[TV (14,0), NoWatch, Mac (10,16), iOS (14,0)] |
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.
NoWatch
twice present
src/gamekit.cs
Outdated
[Export ("active")] | ||
bool Active { [Bind ("isActive")] get; set; } | ||
|
||
[TV (14,0), NoMac, NoiOS] |
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.
no need for [TV (14,0)]
since it's already on the type itself
Build failure Test results10 tests failed, 83 tests passed.Failed tests
|
Build failure 🔥 Build failed 🔥 |
Build failure 🔥 Build failed 🔥 |
Build failure 🔥 Build failed 🔥 |
src/gamekit.cs
Outdated
[Deprecated (PlatformName.iOS, 14, 0, message: "Use 'LoadEntries' to obtain the size of the leaderboard.")] | ||
[Deprecated (PlatformName.TvOS, 14, 0, message: "Use 'LoadEntries' to obtain the size of the leaderboard.")] | ||
[Deprecated (PlatformName.MacOSX, 10, 16, message: "Use 'LoadEntries' to obtain the size of the leaderboard.")] | ||
[Deprecated (PlatformName.WatchOS, 7, 0, message: "Use 'LoadEntries' to obtain the size of the leaderboard.")] |
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.
better share the existing string "Use 'LoadEntries' instead."
src/gamekit.cs
Outdated
[Deprecated (PlatformName.iOS, 14, 0, message: "Use 'LoadEntries' to obtain scores instead.")] | ||
[Deprecated (PlatformName.TvOS, 14, 0, message: "Use 'LoadEntries' to obtain scores instead.")] | ||
[Deprecated (PlatformName.MacOSX, 10, 16, message: "Use 'LoadEntries' to obtain scores instead.")] | ||
[Deprecated (PlatformName.WatchOS, 7, 0, message: "Use 'LoadEntries' to obtain scores instead.")] |
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.
same
src/gamekit.cs
Outdated
[NoTV] | ||
[Deprecated (PlatformName.iOS, 14, 0, message: "Use 'initWithState' instead")] | ||
[Deprecated (PlatformName.TvOS, 14, 0, message: "Use 'initWithState' instead")] | ||
[Deprecated (PlatformName.MacOSX, 10, 16, message: "Use 'initWithState' instead")] |
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.
that looks like a selector and not the managed name
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.
also messages should end with .
src/gamekit.cs
Outdated
[iOS (7,0)][Mac (10,10)] // Marked 10.9 in header, apple 17612948 | ||
[Deprecated (PlatformName.iOS, 14, 0, message: "Use 'initWithLeaderboard' instead.")] | ||
[Deprecated (PlatformName.TvOS, 14, 0, message: "Use 'initWithLeaderboard' instead.")] | ||
[Deprecated (PlatformName.MacOSX, 10, 16, message: "Use 'initWithLeaderboard' instead.")] |
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.
same, selector vs managed name
Build failure Test results9 tests failed, 84 tests passed.Failed tests
|
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.
After @spouliot comments are addressed 👍
Build failure ✅ Build succeeded |
Build failure Test results8 tests failed, 85 tests passed.Failed tests
|
Build failure Test results2 tests failed, 91 tests passed.Failed tests
|
Build success |
37ec2d3
to
117eba0
Compare
Build success |
No description provided.