Skip to content
This repository has been archived by the owner on Apr 26, 2023. It is now read-only.

Updated PlaybackCycle detector to correctly fire complete trackers #32

Merged
merged 1 commit into from
Jan 22, 2019

Conversation

balavor
Copy link
Contributor

@balavor balavor commented Jan 18, 2019

Changes

Updated adPlaybackCycle detector to reset the state but not fire the complete tracker when the ad is finished.

Tests

screen shot 2019-01-18 at 19 14 31

@VerizonAdPlatforms/mobile-sdk-developers: Please review.

Copy link
Contributor

@AndriiMoskvin AndriiMoskvin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make separate detector - you have rised complexity of this detector - which is not a good sign.

sources/video detectors/Player_VideoEvents.swift Outdated Show resolved Hide resolved
@balavor balavor force-pushed the OMSDK-2148/ad-complete branch 2 times, most recently from e19c288 to bf6833e Compare January 21, 2019 14:49
@vvp-sdk-bot
Copy link

vvp-sdk-bot commented Jan 21, 2019

1 Warning
⚠️ This PR does not have any reviewers yet.

Current coverage for PlayerCore.framework is 54.85%

No files affecting coverage found


Current coverage for VerizonVideoPartnerSDK.framework is 36.36%

Files changed - -
TrackingPixelsConnector.swift 0.00% 💀
Player_VideoEvents.swift 0.00% 💀
TrackingPixelsConnector_Ad.swift 0.00% 💀
PlayerProperties_Init.swift 89.30%
AdPlaybackCycleDetector.swift 100.00%

Powered by xcov

Generated by 🚫 Danger

var isStartRecorded = false

func process(streamPlaying: Bool, isSuccessfullyCompleted: Bool, isForceFinished: Bool) -> Result {
guard isForceFinished == false else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@balavor balavor merged commit 4e1e704 into master Jan 22, 2019
@balavor balavor deleted the OMSDK-2148/ad-complete branch January 22, 2019 15:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants