Skip to content

UnityAds adapter for 4.19.0 written in Swift#20

Open
vita-unity wants to merge 6 commits into
mainfrom
unity-ads-swift-adapter-4.19.0
Open

UnityAds adapter for 4.19.0 written in Swift#20
vita-unity wants to merge 6 commits into
mainfrom
unity-ads-swift-adapter-4.19.0

Conversation

@vita-unity

Copy link
Copy Markdown

No description provided.

@vita-unity
vita-unity force-pushed the unity-ads-swift-adapter-4.19.0 branch 2 times, most recently from a8dfb4c to 84ee65d Compare May 25, 2026 17:24
@vita-unity
vita-unity requested a review from Copilot May 28, 2026 19:26

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 41 out of 41 changed files in this pull request and generated 7 comments.

Comment thread adapters/Unity/UnityAdapter/UnityAdsClient.swift
Comment thread adapters/Unity/UnityAdapter/UnityAdsClient.swift Outdated
Comment thread adapters/Unity/UnityAdapter/BannerAdLoader.swift
Comment thread adapters/Unity/UnityAdapter/Util.swift
Comment thread AdapterUnitTests/AdapterUnitTests.xcodeproj/project.pbxproj
Comment thread AdapterUnitTests/AdapterUnitTests.xcodeproj/project.pbxproj Outdated
Comment thread adapters/Unity/UnityAdapter/UnityAdapter.swift Outdated
Comment thread adapters/Unity/UnityAdapter/BannerAdLoader.swift Outdated
Comment thread adapters/Unity/UnityAdapter/InterstitialAdLoader.swift Outdated
Comment thread adapters/Unity/UnityAdapter/RewardedAdLoader.swift Outdated
Comment thread adapters/Unity/UnityAdapter/UnityAdapter.swift
Comment thread adapters/Unity/UnityAdapter/UnityAdsClient.swift Outdated
Comment thread adapters/Unity/UnityAdapter/UnityAdapterError.swift Outdated
@vishaldhiman-unity
vishaldhiman-unity requested review from u-pr and removed request for u-pr June 4, 2026 06:47
Comment thread adapters/Unity/UnityAdapter/UnityAdapterError.swift
Comment thread adapters/Unity/UnityAdapter/Util.swift Outdated
Comment on lines +65 to +80
@MainActor static func rootViewController() -> UIViewController? {
var viewController: UIViewController?
if #available(iOS 13.0, *) {
let activeScene =
UIApplication.shared.connectedScenes
.filter { $0.activationState == .foregroundActive }
.first(where: { $0 is UIWindowScene }) as? UIWindowScene

let keyWindow = activeScene?.windows.first(where: { $0.isKeyWindow })
viewController = keyWindow?.rootViewController
} else {
viewController = UIApplication.shared.keyWindow?.rootViewController
}

return viewController
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Dead code — rootViewController() is defined here but grep-verified zero call sites in adapters/Unity/ or AdapterUnitTests/UnityAdapterTests/. Suggest deleting. (For context: BidMachine/BidMachineAdapter/Util.swift:89 and PubMatic/PubMaticAdapter/Util.swift:174 keep an identical helper because their loaders call it during present(from:). Unity's loaders receive the host's UIViewController directly via MediationFullScreenAd.present(from:), so this helper has no role here.)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fixed in commit bc82a0f7Util.rootViewController() deleted. Unity loaders receive the host's UIViewController directly via MediationFullScreenAd.present(from:) so the helper has no role.

Comment thread adapters/Unity/UnityAdapter/UnityAdapter.swift Outdated
Comment thread adapters/Unity/UnityAdapter/BannerAdLoader.swift
Comment thread AdapterUnitTests/UnityAdapterTests/UnityAdapterTests.swift
vishaldhiman-unity added a commit that referenced this pull request Jun 4, 2026
Fixes from the review of PR #20:

Release-blocker — version files aligned to 4.19.0:
- CHANGELOG.md, podspec.json, Adapter.xcconfig all bumped to 4.19.0 /
  4.19.0.0 / 41900 to match the in-code adapterVersionString.

Correctness — load-completion contract:
- BannerAdLoader / InterstitialAdLoader / RewardedAdLoader: on the SDK
  error path, pass (nil, error) to the GMA completion handler instead of
  (self, error). Matches the contract documented in
  GADMediationXxxLoadCompletionHandler and the Moloco/Verve/Bigo convention.

Error model:
- Restore the legacy ObjC two-domain pattern in UnityAdapterError:
  errorDomain = "com.google.mediation.unity" for adapter-originated errors;
  sdkErrorDomain = "com.google.mediation.unitySDK" for wrapped Unity SDK
  errors. Change static var → let; drop nonisolated(unsafe).
- Preserve the published GADMAdapterUnityErrorCode numeric mapping by
  remapping UnityAdapterError.ErrorCode raw values: 101 (legacy
  InvalidServerParameters), 105 (ShowAdNotReady), 111 (AdUnsupportedAdFormat),
  201 (new invalidAdConfiguration — outside the legacy 101-111 range).
- Rename UnityAdsError.toAdapterError() → toNSError(); returns an NSError
  in sdkErrorDomain instead of wrapping into UnityAdapterError, so SDK error
  codes can't collide with adapter codes in downstream filters.

Privacy/consent forwarding tests:
- Add setNonBehavioral(_:) to the UnityAdsClient protocol; route
  UnityAdapter.updatePrivacyPreferences through client.setNonBehavioral
  instead of UnityAds.setNonBehavioral so the call is testable via the
  injected FakeUnityAdsClient.
- FakeUnityAdsClient captures lastNonBehavioralValue +
  setNonBehavioralCallCount.
- New UnityAdapterPrivacyTests suite with 7 cases covering the
  tagForChildDirectedTreatment x tagForUnderAgeOfConsent x nil/true/false
  matrix.

Dead-code removal:
- Deleted stale 4.18.0.0 constant trio in UnityAdsClientImpl (private
  static let mediationName / adapterVersion / watermarkKey — grep-verified
  unused, with stale version string).
- Deleted Util.rootViewController() (unused; Unity loaders receive the
  UIViewController via MediationFullScreenAd.present(from:) directly).
- Deleted UnityAdapter.supportedFormats (private static let, never
  referenced; nonisolated(unsafe) on immutable let was also noise).

Banner view fallback:
- BannerAdLoader.view: replace silent `bannerAd?.view ?? UIView()` with a
  guard + Util.log call when bannerAd is nil (Moloco-style). Util.log is
  DEBUG-gated so production builds stay quiet.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
vishaldhiman-unity added a commit that referenced this pull request Jun 4, 2026
- Drop three (null) PBXBuildFile entries (B891FDAA, CDA101AF, CDCA7FA1)
  and their references in IronSourceAdapterTests and LiftoffMonetizeAdapterTests
  build phases; all introduced by 84ee65d as a side-effect of Xcode auto-save.
- Drop redundant direct PBXFileReference + PBXBuildFile for libAdapter.a
  (7FF2757F / 7FF27580) in UnityAdapterTests; the existing cross-project
  PBXReferenceProxy to UnityAdapter.xcodeproj's AdapterWithoutValidationScript
  target (AD6E38922EA1664B / AD6E3B3A2EA16D3C) is the canonical pattern shared
  with PubMatic/Verve/etc.

Addresses Copilot review comments on PR #20.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
vishaldhiman-unity added a commit that referenced this pull request Jun 4, 2026
UnityAdapter is an internal class, so the `public` keyword on `loadBanner`
has no effect (Swift emits a "Declaring a public instance method for an
internal class" warning) and is inconsistent with sibling loadInterstitial
and loadRewardedAd on the same class.

Addresses Copilot review comment on PR #20.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vishaldhiman-unity
vishaldhiman-unity requested a review from a team June 4, 2026 16:42
vita-unity and others added 4 commits June 4, 2026 19:00
Fixes from the review of PR #20:

Release-blocker — version files aligned to 4.19.0:
- CHANGELOG.md, podspec.json, Adapter.xcconfig all bumped to 4.19.0 /
  4.19.0.0 / 41900 to match the in-code adapterVersionString.

Correctness — load-completion contract:
- BannerAdLoader / InterstitialAdLoader / RewardedAdLoader: on the SDK
  error path, pass (nil, error) to the GMA completion handler instead of
  (self, error). Matches the contract documented in
  GADMediationXxxLoadCompletionHandler and the Moloco/Verve/Bigo convention.

Error model:
- Restore the legacy ObjC two-domain pattern in UnityAdapterError:
  errorDomain = "com.google.mediation.unity" for adapter-originated errors;
  sdkErrorDomain = "com.google.mediation.unitySDK" for wrapped Unity SDK
  errors. Change static var → let; drop nonisolated(unsafe).
- Preserve the published GADMAdapterUnityErrorCode numeric mapping by
  remapping UnityAdapterError.ErrorCode raw values: 101 (legacy
  InvalidServerParameters), 105 (ShowAdNotReady), 111 (AdUnsupportedAdFormat),
  201 (new invalidAdConfiguration — outside the legacy 101-111 range).
- Rename UnityAdsError.toAdapterError() → toNSError(); returns an NSError
  in sdkErrorDomain instead of wrapping into UnityAdapterError, so SDK error
  codes can't collide with adapter codes in downstream filters.

Privacy/consent forwarding tests:
- Add setNonBehavioral(_:) to the UnityAdsClient protocol; route
  UnityAdapter.updatePrivacyPreferences through client.setNonBehavioral
  instead of UnityAds.setNonBehavioral so the call is testable via the
  injected FakeUnityAdsClient.
- FakeUnityAdsClient captures lastNonBehavioralValue +
  setNonBehavioralCallCount.
- New UnityAdapterPrivacyTests suite with 7 cases covering the
  tagForChildDirectedTreatment x tagForUnderAgeOfConsent x nil/true/false
  matrix.

Dead-code removal:
- Deleted stale 4.18.0.0 constant trio in UnityAdsClientImpl (private
  static let mediationName / adapterVersion / watermarkKey — grep-verified
  unused, with stale version string).
- Deleted Util.rootViewController() (unused; Unity loaders receive the
  UIViewController via MediationFullScreenAd.present(from:) directly).
- Deleted UnityAdapter.supportedFormats (private static let, never
  referenced; nonisolated(unsafe) on immutable let was also noise).

Banner view fallback:
- BannerAdLoader.view: replace silent `bannerAd?.view ?? UIView()` with a
  guard + Util.log call when bannerAd is nil (Moloco-style). Util.log is
  DEBUG-gated so production builds stay quiet.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Drop three (null) PBXBuildFile entries (B891FDAA, CDA101AF, CDCA7FA1)
  and their references in IronSourceAdapterTests and LiftoffMonetizeAdapterTests
  build phases; all introduced by 84ee65d as a side-effect of Xcode auto-save.
- Drop redundant direct PBXFileReference + PBXBuildFile for libAdapter.a
  (7FF2757F / 7FF27580) in UnityAdapterTests; the existing cross-project
  PBXReferenceProxy to UnityAdapter.xcodeproj's AdapterWithoutValidationScript
  target (AD6E38922EA1664B / AD6E3B3A2EA16D3C) is the canonical pattern shared
  with PubMatic/Verve/etc.

Addresses Copilot review comments on PR #20.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
UnityAdapter is an internal class, so the `public` keyword on `loadBanner`
has no effect (Swift emits a "Declaring a public instance method for an
internal class" warning) and is inconsistent with sibling loadInterstitial
and loadRewardedAd on the same class.

Addresses Copilot review comment on PR #20.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Util.log:
- Switch the message parameter to @autoclosure so the interpolated string
  is only built when DEBUG is set. Eliminates allocation/format cost in
  release builds where the print() is compiled out.
- Bracket the prefix as "[UnityAdapter] ..." for consistency with other
  third-party adapter logs (AppLovin, AdMob, etc.).

UnityAdapter.setUp:
- Log when initialization starts (gameId, testMode), when it succeeds,
  when the SDK reports a failure, and when setUp aborts before reaching
  the SDK (e.g. missing gameId in server config). Gives integrators
  enough breadcrumbs from the adapter alone to diagnose init issues
  without having to attach to the SDK's internal logger.

Note: For the logs to surface in pod consumers, the host app's Podfile
must propagate SWIFT_ACTIVE_COMPILATION_CONDITIONS=DEBUG into pod
targets — CocoaPods does not do this by default.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vishaldhiman-unity
vishaldhiman-unity force-pushed the unity-ads-swift-adapter-4.19.0 branch from a3fd2ea to 65cf433 Compare June 5, 2026 02:06
The cross-project reference and header search paths pointed at
../adapters/UnityAdapter/... (which does not exist) instead of
../adapters/Unity/UnityAdapter/... The wrong path was introduced by
upstream mediation commits in 2024-07 (file reference) and 2025-07
(header search paths) and blocks UnityAdapterTests from finding the
UnityAdapter swiftmodule when running locally via xcodebuild or Xcode UI.

Renames in the affected pbxproj fields:
- ../adapters/UnityAdapter/UnityAdapter.xcodeproj → ../adapters/Unity/UnityAdapter.xcodeproj
- ${SRCROOT}/../adapters/UnityAdapter/UnityAdapter → ${SRCROOT}/../adapters/Unity/UnityAdapter
- ${SRCROOT}/../adapters/UnityAdapter/Public/Headers → ${SRCROOT}/../adapters/Unity/Public/Headers

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 44 out of 44 changed files in this pull request and generated 2 comments.

Comment on lines 141 to 143
files = (
86780D368F5E5F2189E185B5 /* Pods_Default_Adapter.framework in Frameworks */,
);
}
}

func presentRewarded(_ rewardedAd: UADSRewardedAd?, configuration: UADSShowConfiguration, delegate: UADSRewardedShowDelegate) throws(UnityAdapterError) {
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants