Implement Uplynk ads api #29
Conversation
connectors/uplynk/src/main/java/com/theoplayer/android/connector/uplynk/UplynkConnector.kt
Outdated
Show resolved
Hide resolved
...lynk/src/main/java/com/theoplayer/android/connector/uplynk/internal/UplynkEventDispatcher.kt
Outdated
Show resolved
Hide resolved
| private set | ||
|
|
||
| private val eventDispatcher = UplynkEventDispatcher().also { | ||
| it.addListener(object : UplynkListener { |
There was a problem hiding this comment.
Nitpick: make UplynkConnector implementing UplynkListener
There was a problem hiding this comment.
I thought about it, but didn't do it because UplynkConnector is a part of public API and I think it might be confusing for a user of this class. So I decided to do it like this.
Do you think it would be better to make UplynkConnector implementing UplynkListener?
There was a problem hiding this comment.
Indeed, it would be confusing..
Maybe we move the checks to the internal too?
| /** | ||
| * Scheduled ad breaks | ||
| */ | ||
| var adBreaks: List<UplynkAdBreak>? = null | ||
| private set | ||
|
|
||
| /** | ||
| * The ad break that is currently played or `null` otherwise | ||
| */ | ||
| var currentAdBreak: UplynkAdBreak? = null | ||
| private set | ||
|
|
||
| /** | ||
| * The ad that is currently played or `null` otherwise | ||
| */ | ||
| var currentAd: UplynkAd? = null | ||
| private set |
There was a problem hiding this comment.
Maybe more of a philosophical question: why do we have both UplynkConnector.currentAds and player.ads.currentAds? Wasn't the whole point of the custom SSAI integrations that we can put everything behind the THEOplayer ads API?
Same thing with the new UplynkListener methods: how are those different from the existing events on player.ads?
There was a problem hiding this comment.
The only difference between them is the type of Ads object. player.ads.currentAds - has common type of an ad, that is part of THEOplayer project. UplynkConnector.currentAds has specific type for Uplynk ads. The same as for UplynkListener
I think we have to discuss if we even need this PR or if we could find some other solution
There was a problem hiding this comment.
Okay, I think this is a limitation of our custom SSAI API then. We should provide a way to add some integration-specific data to an custom Ad or AdBreak.
I suggest we park this PR for now until we have that new API.
|
This PR is not needed because of the new customData property in SSAI API |
This PR implements the following properties:
adBreaks - when the addAdBreak and removeAdBreak events are triggered.
currentAdBreak - when the adBreakBeginEvent, AdBreakEndEvent or AdBreakSkipEvent are triggered.
currentAd - when the currentAdBreak gets updated as this property represents the ads of the currentAdBreak.