Skip to content

Commit

Permalink
Explicit delegate signatures (livekit#283)
Browse files Browse the repository at this point in the history
A few delegates had the same signatures (by using overloading) such as:

`room(_ room: Room, didUpdate speakers: [Participant])`
`room(_ room: Room, didUpdate metadata: String?)`
`room(_ room: Room, didUpdate isRecording: Bool)`

This pattern will break if add another method of the same type will be
added in the future:
`room(_ room: Room, didUpdate isSomeExampleFlag: Bool)`

Additionally, since labels can be omitted in Swift, the implementation
side can be ambiguous:
```Swift
func room(_: Room, didUpdate _: String?) {
  // ...
}

func room(_: Room, didUpdate _: Bool) {
  // ...
}
```

This PR makes delegate signatures explicit and future-proof by including
the label in the signature:

`room(_ room: Room, didUpdateSpeakingParticipants participants:
[Participant])`
`room(_ room: Room, didUpdateMetadata metadata: String?)`
`room(_ room: Room, didUpdateIsRecording isRecording: Bool)`

I can deprecate the old methods, but since this will be part of v2
release maybe it's not required to deprecate.
  • Loading branch information
hiroshihorie committed Dec 18, 2023
1 parent e901d1e commit 81a5ff4
Show file tree
Hide file tree
Showing 10 changed files with 143 additions and 122 deletions.
8 changes: 4 additions & 4 deletions Sources/LiveKit/Core/Room+EngineDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ extension Room: EngineDelegate {
}

delegates.notify(label: { "room.didUpdate connectionState: \(state.connectionState) oldValue: \(oldState.connectionState)" }) {
$0.room?(self, didUpdate: state.connectionState, oldValue: oldState.connectionState)
$0.room?(self, didUpdateConnectionState: state.connectionState, oldConnectionState: oldState.connectionState)
}

// Legacy connection delegates
Expand All @@ -49,9 +49,9 @@ extension Room: EngineDelegate {
delegates.notify { $0.room?(self, didConnect: didReconnect) }
} else if case .disconnected = state.connectionState {
if case .connecting = oldState.connectionState {
delegates.notify { $0.room?(self, didFailToConnect: oldState.disconnectError) }
delegates.notify { $0.room?(self, didFailToConnectWithError: oldState.disconnectError) }
} else {
delegates.notify { $0.room?(self, didDisconnect: state.disconnectError) }
delegates.notify { $0.room?(self, didDisconnectWithError: state.disconnectError) }
}
}
}
Expand Down Expand Up @@ -116,7 +116,7 @@ extension Room: EngineDelegate {
guard let self else { return }

self.delegates.notify(label: { "room.didUpdate speakers: \(activeSpeakers)" }) {
$0.room?(self, didUpdate: activeSpeakers)
$0.room?(self, didUpdateSpeakingParticipants: activeSpeakers)
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/LiveKit/Core/Room+SignalClientDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ extension Room: SignalClientDelegate {
guard let self else { return }

self.delegates.notify(label: { "room.didUpdate speakers: \(speakers)" }) {
$0.room?(self, didUpdate: activeSpeakers)
$0.room?(self, didUpdateSpeakingParticipants: activeSpeakers)
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions Sources/LiveKit/Core/Room.swift
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ public class Room: NSObject, ObservableObject, Loggable {
guard let self else { return }

self.delegates.notify(label: { "room.didUpdate metadata: \(metadata)" }) {
$0.room?(self, didUpdate: metadata)
$0.room?(self, didUpdateMetadata: metadata)
}
}
}
Expand All @@ -194,7 +194,7 @@ public class Room: NSObject, ObservableObject, Loggable {
guard let self else { return }

self.delegates.notify(label: { "room.didUpdate isRecording: \(newState.isRecording)" }) {
$0.room?(self, didUpdate: newState.isRecording)
$0.room?(self, didUpdateIsRecording: newState.isRecording)
}
}
}
Expand Down
16 changes: 8 additions & 8 deletions Sources/LiveKit/Participant/LocalParticipant.swift
Original file line number Diff line number Diff line change
Expand Up @@ -207,10 +207,10 @@ public class LocalParticipant: Participant {

// Notify didPublish
delegates.notify(label: { "localParticipant.didPublish \(publication)" }) {
$0.localParticipant?(self, didPublish: publication)
$0.localParticipant?(self, didPublishPublication: publication)
}
room.delegates.notify(label: { "localParticipant.didPublish \(publication)" }) {
$0.room?(self.room, localParticipant: self, didPublish: publication)
$0.room?(self.room, localParticipant: self, didPublishPublication: publication)
}

log("[publish] success \(publication)", .info)
Expand Down Expand Up @@ -259,10 +259,10 @@ public class LocalParticipant: Participant {
func _notifyDidUnpublish() async {
guard _notify else { return }
delegates.notify(label: { "localParticipant.didUnpublish \(publication)" }) {
$0.localParticipant?(self, didUnpublish: publication)
$0.localParticipant?(self, didUnpublishPublication: publication)
}
room.delegates.notify(label: { "room.didUnpublish \(publication)" }) {
$0.room?(self.room, localParticipant: self, didUnpublish: publication)
$0.room?(self.room, localParticipant: self, didUnpublishPublication: publication)
}
}

Expand Down Expand Up @@ -402,11 +402,11 @@ public class LocalParticipant: Participant {
let didUpdate = super.set(permissions: newValue)

if didUpdate {
delegates.notify(label: { "participant.didUpdate permissions: \(newValue)" }) {
$0.participant?(self, didUpdate: newValue)
delegates.notify(label: { "participant.didUpdatePermissions: \(newValue)" }) {
$0.participant?(self, didUpdatePermissions: newValue)
}
room.delegates.notify(label: { "room.didUpdate permissions: \(newValue)" }) {
$0.room?(self.room, participant: self, didUpdate: newValue)
room.delegates.notify(label: { "room.didUpdatePermissions: \(newValue)" }) {
$0.room?(self.room, participant: self, didUpdatePermissions: newValue)
}
}

Expand Down
10 changes: 5 additions & 5 deletions Sources/LiveKit/Participant/Participant.swift
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public class Participant: NSObject, ObservableObject, Loggable {

if newState.isSpeaking != oldState.isSpeaking {
self.delegates.notify(label: { "participant.didUpdate isSpeaking: \(self.isSpeaking)" }) {
$0.participant?(self, didUpdate: self.isSpeaking)
$0.participant?(self, didUpdateIsSpeaking: self.isSpeaking)
}
}

Expand All @@ -112,10 +112,10 @@ public class Participant: NSObject, ObservableObject, Loggable {
oldState.metadata == nil ? !metadata.isEmpty : true
{
self.delegates.notify(label: { "participant.didUpdate metadata: \(metadata)" }) {
$0.participant?(self, didUpdate: metadata)
$0.participant?(self, didUpdateMetadata: metadata)
}
self.room.delegates.notify(label: { "room.didUpdate metadata: \(metadata)" }) {
$0.room?(self.room, participant: self, didUpdate: metadata)
$0.room?(self.room, participant: self, didUpdateMetadata: metadata)
}
}

Expand All @@ -133,10 +133,10 @@ public class Participant: NSObject, ObservableObject, Loggable {

if newState.connectionQuality != oldState.connectionQuality {
self.delegates.notify(label: { "participant.didUpdate connectionQuality: \(self.connectionQuality)" }) {
$0.participant?(self, didUpdate: self.connectionQuality)
$0.participant?(self, didUpdateConnectionQuality: self.connectionQuality)
}
self.room.delegates.notify(label: { "room.didUpdate connectionQuality: \(self.connectionQuality)" }) {
$0.room?(self.room, participant: self, didUpdate: self.connectionQuality)
$0.room?(self.room, participant: self, didUpdateConnectionQuality: self.connectionQuality)
}
}

Expand Down
16 changes: 8 additions & 8 deletions Sources/LiveKit/Participant/RemoteParticipant.swift
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ public class RemoteParticipant: Participant {

for publication in newTrackPublications.values {
self.delegates.notify(label: { "participant.didPublish \(publication)" }) {
$0.participant?(self, didPublish: publication)
$0.participant?(self, didPublishPublication: publication)
}
self.room.delegates.notify(label: { "room.didPublish \(publication)" }) {
$0.room?(self.room, participant: self, didPublish: publication)
$0.room?(self.room, participant: self, didPublishPublication: publication)
}
}
}
Expand Down Expand Up @@ -132,10 +132,10 @@ public class RemoteParticipant: Participant {
try await track.start()

delegates.notify(label: { "participant.didSubscribe \(publication)" }) {
$0.participant?(self, didSubscribe: publication, track: track)
$0.participant?(self, didSubscribePublication: publication)
}
room.delegates.notify(label: { "room.didSubscribe \(publication)" }) {
$0.room?(self.room, participant: self, didSubscribe: publication, track: track)
$0.room?(self.room, participant: self, didSubscribePublication: publication)
}
}

Expand Down Expand Up @@ -163,10 +163,10 @@ public class RemoteParticipant: Participant {
func _notifyUnpublish() async {
guard _notify else { return }
delegates.notify(label: { "participant.didUnpublish \(publication)" }) {
$0.participant?(self, didUnpublish: publication)
$0.participant?(self, didUnpublishPublication: publication)
}
room.delegates.notify(label: { "room.didUnpublish \(publication)" }) {
$0.room?(self.room, participant: self, didUnpublish: publication)
$0.room?(self.room, participant: self, didUnpublishPublication: publication)
}
}

Expand All @@ -183,10 +183,10 @@ public class RemoteParticipant: Participant {
if _notify {
// Notify unsubscribe
delegates.notify(label: { "participant.didUnsubscribe \(publication)" }) {
$0.participant?(self, didUnsubscribe: publication, track: track)
$0.participant?(self, didUnsubscribePublication: publication)
}
room.delegates.notify(label: { "room.didUnsubscribe \(publication)" }) {
$0.room?(self.room, participant: self, didUnsubscribe: publication, track: track)
$0.room?(self.room, participant: self, didUnsubscribePublication: publication)
}
}

Expand Down
74 changes: 42 additions & 32 deletions Sources/LiveKit/Protocols/ParticipantDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,28 +23,35 @@ import Foundation
/// All delegate methods are optional.
///
/// To ensure each participant's delegate is registered, you can look through ``Room/localParticipant`` and ``Room/remoteParticipants`` on connect
/// and register it on new participants inside ``RoomDelegate/room(_:participantDidJoin:)-9bkm4``
/// and register it on new participants inside ``RoomDelegate/room(_:participantDidJoin:)``
@objc
public protocol ParticipantDelegate: AnyObject {
// MARK: - Participant

/// A ``Participant``'s metadata has updated.
/// `participant` Can be a ``LocalParticipant`` or a ``RemoteParticipant``.
@objc(participant:didUpdateMetadata:) optional
func participant(_ participant: Participant, didUpdate metadata: String?)
func participant(_ participant: Participant, didUpdateMetadata metadata: String?)

/// A ``Participant``'s name has updated.
/// `participant` Can be a ``LocalParticipant`` or a ``RemoteParticipant``.
@objc(participant:didUpdateName:) optional
func participant(_ participant: Participant, didUpdateName: String?)
func participant(_ participant: Participant, didUpdateName name: String?)

/// The isSpeaking status of a ``Participant`` has changed.
/// `participant` Can be a ``LocalParticipant`` or a ``RemoteParticipant``.
@objc(participant:didUpdateSpeaking:) optional
func participant(_ participant: Participant, didUpdate speaking: Bool)
func participant(_ participant: Participant, didUpdateIsSpeaking isSpeaking: Bool)

/// The connection quality of a ``Participant`` has updated.
/// `participant` Can be a ``LocalParticipant`` or a ``RemoteParticipant``.
@objc(participant:didUpdateConnectionQuality:) optional
func participant(_ participant: Participant, didUpdate connectionQuality: ConnectionQuality)
func participant(_ participant: Participant, didUpdateConnectionQuality connectionQuality: ConnectionQuality)

@objc(participant:didUpdatePermissions:) optional
func participant(_ participant: Participant, didUpdatePermissions permissions: ParticipantPermissions)

// MARK: - TrackPublication

/// `muted` state has updated for the ``Participant``'s ``TrackPublication``.
///
Expand All @@ -53,54 +60,57 @@ public protocol ParticipantDelegate: AnyObject {
///
/// `participant` Can be a ``LocalParticipant`` or a ``RemoteParticipant``.
@objc(participant:publication:didUpdateMuted:) optional
func participant(_ participant: Participant, didUpdate publication: TrackPublication, muted: Bool)
func participant(_ participant: Participant, didUpdatePublication publication: TrackPublication, isMuted: Bool)

@objc(participant:didUpdatePermissions:) optional
func participant(_ participant: Participant, didUpdate permissions: ParticipantPermissions)
// MARK: - LocalTrackPublication

/// ``RemoteTrackPublication/streamState`` has updated for the ``RemoteParticipant``.
@objc(participant:publication:didUpdateStreamState:) optional
func participant(_ participant: RemoteParticipant, didUpdate publication: RemoteTrackPublication, streamState: StreamState)
/// The ``LocalParticipant`` has published a ``LocalTrackPublication``.
@objc(localParticipant:didPublish:) optional
func localParticipant(_ participant: LocalParticipant, didPublishPublication publication: LocalTrackPublication)

/// ``RemoteTrackPublication/subscriptionAllowed`` has updated for the ``RemoteParticipant``.
@objc(participant:publication:didUpdateCanSubscribe:) optional
func participant(_ participant: RemoteParticipant, didUpdate publication: RemoteTrackPublication, permission allowed: Bool)
/// The ``LocalParticipant`` has unpublished a ``LocalTrackPublication``.
@objc(localParticipant:didUnpublish:) optional
func localParticipant(_ participant: LocalParticipant, didUnpublishPublication publication: LocalTrackPublication)

// MARK: - RemoteTrackPublication

/// When a new ``RemoteTrackPublication`` is published to ``Room`` after the ``LocalParticipant`` has joined.
///
/// This delegate method will not be called for tracks that are already published.
@objc(remoteParticipant:didPublish:) optional
func participant(_ participant: RemoteParticipant, didPublish publication: RemoteTrackPublication)
func participant(_ participant: RemoteParticipant, didPublishPublication publication: RemoteTrackPublication)

/// The ``RemoteParticipant`` has unpublished a ``RemoteTrackPublication``.
@objc(remoteParticipant:didUnpublish:) optional
func participant(_ participant: RemoteParticipant, didUnpublish publication: RemoteTrackPublication)

/// The ``LocalParticipant`` has published a ``LocalTrackPublication``.
@objc(localParticipant:didPublish:) optional
func localParticipant(_ participant: LocalParticipant, didPublish publication: LocalTrackPublication)

/// The ``LocalParticipant`` has unpublished a ``LocalTrackPublication``.
@objc(localParticipant:didUnpublish:) optional
func localParticipant(_ participant: LocalParticipant, didUnpublish publication: LocalTrackPublication)
func participant(_ participant: RemoteParticipant, didUnpublishPublication publication: RemoteTrackPublication)

/// The ``LocalParticipant`` has subscribed to a new ``RemoteTrackPublication``.
///
/// This event will always fire as long as new tracks are ready for use.
@objc(participant:didSubscribe:track:) optional
func participant(_ participant: RemoteParticipant, didSubscribe publication: RemoteTrackPublication, track: Track)
@objc(participant:didSubscribe:) optional
func participant(_ participant: RemoteParticipant, didSubscribePublication publication: RemoteTrackPublication)

/// Unsubscribed from a ``RemoteTrackPublication`` and is no longer available.
///
/// Clients should listen to this event and handle cleanup.
@objc(participant:didUnsubscribePublication:) optional
func participant(_ participant: RemoteParticipant, didUnsubscribePublication publication: RemoteTrackPublication)

/// Could not subscribe to a track.
///
/// This is an error state, the subscription can be retried.
@objc(participant:didFailToSubscribeTrackWithSid:error:) optional
func participant(_ participant: RemoteParticipant, didFailToSubscribe trackSid: String, error: Error)
func participant(_ participant: RemoteParticipant, didFailToSubscribe trackSid: String, error: LiveKitError)

/// Unsubscribed from a ``RemoteTrackPublication`` and is no longer available.
///
/// Clients should listen to this event and handle cleanup.
@objc(participant:didUnsubscribePublication:track:) optional
func participant(_ participant: RemoteParticipant, didUnsubscribe publication: RemoteTrackPublication, track: Track)
/// ``TrackPublication/streamState`` has updated for the ``RemoteTrackPublication``.
@objc(participant:publication:didUpdateStreamState:) optional
func participant(_ participant: RemoteParticipant, didUpdatePublication publication: RemoteTrackPublication, streamState: StreamState)

/// ``RemoteTrackPublication/subscriptionAllowed`` has updated for the ``RemoteTrackPublication``.
@objc(participant:publication:didUpdateCanSubscribe:) optional
func participant(_ participant: RemoteParticipant, didUpdatePublication publication: RemoteTrackPublication, isSubscriptionAllowed: Bool)

// MARK: - Data

/// Data was received from a ``RemoteParticipant``.
@objc(participant:didReceiveData:topic:) optional
Expand Down
Loading

0 comments on commit 81a5ff4

Please sign in to comment.