Stop a hidden First Look banner from requesting, and give the pass cycle its own file by antonurankar-moloco · Pull Request #9 · cloudx-io/cloudx-unity

antonurankar-moloco · 2026-09-06T10:17:28Z

Hiding a First Look banner was supposed to stop it requesting. It did not, in four separate ways — one reported, three found in review on this PR. Each was reproduced on device against a control build rather than argued from reading the code.

Along the way the host half of the banner contract became a real file, FirstLookBannerCycle.cs, which is what lets the docs page link it instead of pasting a copy that nothing compiles.

1. The reported defect: a retry armed on a hidden slot

FirstLookScreen had two places arming the same retry, and only one knew about the hide. ToggleBanner called CancelInvoke(nameof(LoadBanner)), which empties Unity's invoke queue and nothing else; the AdLoadFailed handler armed a new load unconditionally. A load already out on the network is not in that queue, so it failed after the hide and armed a fresh request against an off-screen slot. With both sources no-filling, the backoff climbed to its 60s cap and repeated until the scene was destroyed.

It also broke a contract the README stated: "Hiding cancels the pending pass, so a hidden slot never keeps requesting in the background."

Reproduced against the unmodified parent commit, network killed so both legs no-fill:

09:29:47.513  Banner load failed (AdMob); retrying in 16s
   >>> 09:30:12.87  Hide tapped, load in flight <<<
09:30:15.349  Banner load failed (AdMob); retrying in 32s   <- armed on a hidden slot
09:30:58.580  Banner load failed (AdMob); retrying in 60s
09:32:09.814  Banner load failed (AdMob); retrying in 60s
09:33:21.513  Banner load failed (AdMob); retrying in 60s

13 SDK-level request lines after the hide, with the slot empty.

One correction to the original report. It claimed a fill in this state means the demo "bought an ad for a hidden banner and spent the pass on nothing". It does not: KeepsUnspentFill banks it, ReadySource goes non-null, Load() early-returns and the loop stops. Only the sustained no-fill branch was broken.

2-5. Four more, found in review

The first fix was necessary but not sufficient. Copilot found #2-#4; #5 came out of my own final pass, after Copilot's reviewer went down. All four were real and reachable.

#	Defect	Control build	Fixed
2	The retry guard only ran after `AdLoadFailed`, which the controller raises only when the AdMob leg fails. Hiding while the CloudX leg was in flight left `CloudXOnLoadFailed` free to start the fallback on an off-screen slot.	1 AdMob request, 1.2s after the hide	0
3	The fix for #2 cleared its flag in `Show()` — current visibility, not whether the running pass was cancelled. Hide then a quick re-show revived it.	2 AdMob requests	0
4	Only failure callbacks were gated. A stale success still raised `PassSpent` and reset the cooldown from a pass the player had dismissed.	walked against the code; regression-verified	no re-timing
5	A regression from fixing #2-#4. Suppressing a cancelled pass's terminal callback could strand the cycle with an ad still on screen. Hide, then Show, leaves the flag set while the cancelled load is still on the network; the Show arms the cooldown; that tick finds the load in flight, `Load()` early-returns, the flag is never cleared, and the stale completion is then dropped - so no `PassSpent` and no `AdLoadFailed` reach the host and nothing is pending.	code trace; not reproduced on device	cycle keeps turning

The root cause of #4 is worth naming: one flag carried two meanings — "this controller asked for the load", which decides whether a fill is banked, and "this should re-time the cycle". They come apart in exactly one case, a pass a Hide cancelled: it is still ours, but its timing is not. They are two flags now, so a cancelled pass's fill is still banked while hidden and still shows if the slot has since been shown — discarding an ad we paid a request for would be worse — but the cooldown stays with whatever the host scheduled after the hide.

The flag that ties #2–#5 together is pass-scoped: Hide() sets it and any Load() clears it, including one the in-flight guard then drops. That last part is what fixes #5: a load the host asks for means the slot is wanted again, so it adopts the pass still out on the network instead of silencing it, and that pass's completion carries the cycle forward. Show() deliberately does not clear it, so a Hide with no following Load still cancels the pass completely. _wantShown cannot do this job at all, because it is also false during the preload, and the preload has to reach the fallback — that is what banks an ad for the first tap.

On main the #5 window self-healed, through the very defect #2 fixes: the stale failure fell through to the AdMob fallback, that fill rendered into the visible view and PassSpent re-armed the cooldown. So #5 is a resilience regression this PR introduced and then removed, not a pre-existing gap — I had it filed as the latter at first, which was wrong.

The extraction

The clock only existed inside FirstLookScreen, mixed in with SDK init, the ATT gate, the interstitial and UI plumbing. That is why the docs page had to paste a host snippet — and a pasted snippet is a second copy, which had drifted into declaring FirstLookBannerController.Source, a type that does not exist, because nothing compiled it.

FirstLookBannerCycle.cs is that clock as a MonoBehaviour, in one file: the cooldown, the retry backoff, the wanted flag and the show/hide toggle. It depends on nothing but FirstLookBannerController and FirstLookSource, so the docs link it rather than copy it. FirstLookScreen drops from 390 lines to 332 and keeps only initialization, button binding and status text.

It is named for what it holds. It was FirstLookBannerHud first, which was wrong — it has no UI at all, and AdScreenUi is the file that actually owns the labels and buttons.

This is the smaller of two options. It removes one class of drift, but publishers still hand-write the host. Folding the cycle into the controller so there is no host contract at all is the follow-up, once this squashes.

`ForceCloudXNoFill` removed

An internal test switch that does not belong in the public sample. FirstLookConfig is now just the AdMob fallback ad unit ids — the pass cooldown moved into the cycle that paces it. The README points at DemoConfig for the same effect, which is what the docs page already tells publishers to do.

Interstitial: not the same hole

No change. It arms retries the same unconditional way, but a fullscreen ad has no visibility state — no Hide, nothing to cancel — and keeping one preloaded is the intent. Its "copy two files" header is also still accurate, because it needs no clock. One adjacent thing checked: neither handler calls CancelInvoke, so two LoadInterstitial invokes can stack; the second is absorbed by the guards at FirstLookInterstitialController.cs:96. Harmless.

Verification

Final code re-verified on instances created for it, because another session held the ones I had been using: emulator-5556 (AVD Pixel_2_API_33), a physical Pixel 10 Pro, and a fresh FirstLookVerify simulator (iPhone 17 Pro, iOS 26.2).

Check	emulator-5556	Pixel 10 Pro	iOS simulator
Pass cadence, shown	29.8s, 30.0s, 29.9s	30.0s, 30.2s over 6 passes	30.1s, 31.6s
Hide -> CloudX / AdMob requests	0 / 0 over 88s	0 / 0 over 76s	0 requests, 0 revenue events over 85s
Re-show	banked ad back, next pass +29.9s	+30.1s	+30.5s
Interstitial	shown (CloudX)	shown (CloudX)	shown (CloudX)
Batchmode compile	0 `error CS`	—	—

Markers: [CXAdView] ... load() in logcat on Android; on iOS the CloudX internals are os_log at debug level, which log show cannot recover after the fact, so those counts come from a live log stream --level debug started before each window and confirmed alive at the end of it. Unity's own Debug.Log on iOS is printf to stderr and comes from simctl launch --console-pty.

The earlier control-build numbers for #1-#4 stand as recorded in the sections above; they were taken against control builds of the revisions that carried each defect. Reproducing #2 and #3 needed care: with an invalid ad unit the CloudX leg is only ~33ms, so a hide cannot land inside it. Using the real ad unit with the network off stretches the leg to 1-2s through an HttpRetry cycle, and the taps were triggered off the [CXAdView] load() log line rather than timed by hand.

Not reproduced: the three race cases and the preload retry need the network down, and the iOS simulator has no per-device network lever, so those are Android-only evidence — I am not arguing them on iOS from equivalence, they were not observed there either way. #5 is not reproduced on either platform: it needs a load outstanding past the 30s cooldown on a slot that has been hidden and shown again, and I did not manufacture that window. It rests on the code trace above plus the non-regression runs, and I would rather say so than imply a capture exists.

A correction to earlier evidence in this PR. An earlier revision logged not retrying while hidden, and I cited it. Once the controller began dropping the terminal failure of a cancelled pass, that branch became unreachable and the current build cannot emit it. The dead branch is gone, and what stands in its place is the request count — which is the thing that actually matters.

Scope

File	Change
`FirstLookBannerCycle.cs`	New. The pass cycle, extracted from the screen.
`FirstLookBannerController.cs`	Changed. Pass-cancellation state; both CloudX and AdMob failure paths, and both success paths, now distinguish a cancelled pass.
`FirstLookScreen.cs`	Delegates the banner to the cycle; keeps init, buttons, status text.
`FirstLookConfig.cs`	Loses the pass cooldown and the internal no-fill switch.
`README.md`	File table, copy instructions, hide rule.

No scene and no ProjectSettings. FirstLookInterstitialController.cs untouched.

Follow-ups, not in this PR

No per-pass timeout. A load whose callback never arrives strands _isLoadingCloudX set for the life of the controller, so every later Load() is dropped. Genuinely pre-existing, and distinct from Import CloudX Unity SDK 4.6.1 and document callback threading #5: Import CloudX Unity SDK 4.6.1 and document callback threading #5 was about the flag outliving a pass that did complete, which is fixed.
Folding the cycle into the controller, so publishers have no host contract to get wrong at all.

Companion docs PR: cloudx-io/docs#407 — merge this one first, or its links 404.

…to two formats Banner readiness was tracked with two sticky booleans that were set on load and never cleared, so the first successful fill from either source froze the slot for the whole screen visit. After a CloudX no-fill the AdMob fallback owned the placement until the scene was destroyed, and CloudX was never asked again - the opposite of what First Look promises. The screen's backoff retry and the LoadBanner() inside ToggleBanner were both unreachable in that state. An inline ad has no consumption event the way a fullscreen one does, so displaying it is now what spends the pass: a load into an already-visible view renders immediately, which makes "on screen" the one honest moment to treat a fill as used. ShowSource clears both flags and raises PassSpent; the screen schedules the next pass PassCooldownSeconds later, and that pass starts at CloudX again. Reloading is in place rather than a recreate, so a visible ad is replaced only once the new one has filled and the slot never blanks. The cycle turns only while an ad is on screen - hiding cancels the pending pass - and a fill the AdMob console refreshed on its own does not count as a pass, or an AdMob unit with Automatic refresh enabled would postpone CloudX's next first look on every refresh. The screen now covers interstitial and banner only. Those are the two shapes the rule has to handle; rewarded repeats the interstitial and MREC repeats the banner, and the General screen already demonstrates all four formats. With two formats left, each family base had exactly one subclass, so the bases are gone and each controller is one self-contained file a publisher can copy on its own alongside FirstLookSource.cs. That duplicates about fifty lines of ad-unit and dispose bookkeeping between the two files, on purpose, so neither drags a shared base into someone else's project. AdScreenUi closes the portrait column when a screen hides buttons, so the two remaining ones sit together instead of leaving a hole. It is a no-op when nothing is hidden, which is the General screen. Verified on the Android emulator and the iOS simulator, on the CloudX path and the forced no-fill path: CloudX re-asked on every pass, a 30 s cadence that follows the constant rather than the SDK's own inert refresh timer, zero requests while hidden, no blank frame on swap, and no drift across rotations.

OnAdMobLoaded set _adMobLoaded unconditionally, including for a fill the AdMob console refreshed on its own. When that fill was not displayed - which happens exactly when CloudX is the shown source, because ShowIfWanted refuses to let a non-pass fill take the slot from CloudX - the flag stayed set. ReadySource then reported AdMob, the scheduled Load() returned early, and since nothing went on screen no PassSpent fired and no further pass was ever scheduled. The cycle stalled for good with CloudX on screen and a stale AdMob fill banked: the original latch, re-entered through the refresh path. A fill from a pass may sit unspent until the slot is shown - that is what banks an ad for the first tap - but a fill nobody asked for may not. ShowIfWanted now reports whether it displayed, and both load handlers keep the loaded flag only for an unshown fill that came from a pass. Nothing is lost by forgetting the rest: the native view keeps the creative and the next pass reloads that side. Found by Copilot on PR #8. Re-verified on the Android emulator and the iOS simulator, both fill paths: 30 s cadence held (Android CloudX 30.2/30.3 s, forced no-fill 30.6/30.5/30.6 s across four passes), an unsolicited AdMob refresh at 14:55:46 did not move the next pass at 14:55:50, the preloaded fill was still banked and shown on the first tap with no new load, and hide then show re-showed with zero new requests.

The docs page used to carry its own copy of both controllers and now links here instead, which makes these files the thing a publisher actually reads. The 48-line banner header was written for the other arrangement: it re-explained the whole pass cycle, which the page already does, and it cited things a publisher copying the file does not have - FirstLookScreen.ToggleBanner, GeneralScreen, FirstLookConfig.PassCooldownSeconds. What each header keeps is what someone reading this file needs and cannot get from the code: what to copy, the reading order, why an inline ad needs a pass cycle when a fullscreen one does not, the two things the host must do or the cycle stalls, and the AdMob console setting no code can apply. Everything else now points at the page. The reasoning per rule stays where it always was, in the comments at the lines it governs. The CloudX placement and custom data strings say they are this demo's and are the caller's to replace, which nothing said before. Comments only - no logic changed, verified by diffing out comment lines - so the device verification from the earlier commits still stands.

The two controllers already carried the link. The other three did not, and one of them is a file the page tells publishers to copy: FirstLookSource.cs. Anyone who lands in it from a repo browse or a copy-paste had no way back to the explanation. All five now carry it, and the round trip closes: the page links to the files, the files link to the page. FirstLookScreen.cs gets one extra line, because it is the only place that shows the half of the banner contract the controller cannot keep on its own - ScheduleNextBannerPass starting the next pass a cooldown after PassSpent, and ToggleBanner cancelling it on hide. That is what a reader is looking for when the page tells them the host owns the clock. Comments only, verified by diffing out comment lines. The URL returns 200.

FirstLookScreen had two places arming the same banner retry and only one knew about the hide. ToggleBanner cancelled the pending pass with CancelInvoke, which empties Unity's invoke queue and nothing else; the AdLoadFailed handler armed a new load unconditionally. A load already out on the network is not in that queue, so it failed after the hide and armed a fresh request against an off-screen slot. Load() never consults _wantShown, so the request went out, and with both sources no-filling the backoff climbed to its 60s cap and repeated until the scene was destroyed. Reproduced on a Pixel 6 emulator (API 35) against the unmodified parent commit: hide at 09:30:12.87, the in-flight load failed 2.5s later and armed a retry, then kept requesting at 09:30:58, 09:32:09 and 09:33:21 with the slot empty. The guard is the one the docs page already publishes in its host snippet: a flag survives a callback that arrives after the hide, where CancelInvoke cannot. It starts true on purpose - IsShown is not a substitute, because it is also false during the preload before the first Show(), where a retry is still wanted. The demo now matches its own documented snippet. A fill arriving while hidden was never part of the defect: KeepsUnspentFill banks it, ReadySource goes non-null and Load() early-returns, so that path already terminated. Also removes ForceCloudXNoFill and its ad-unit helper. It is an internal test switch and does not belong in the public sample; the README now points at DemoConfig for the same effect, as the docs page already does. Verified after the fix: "not retrying while hidden" then zero ad requests for 3m17s; preload retry intact (5 retries with the banner never shown); pass cadence 30.17/30.43s on Android and 31/30s on iOS; hide for 120s gives zero requests on both platforms and re-show returns the banked ad with no new load; interstitial unchanged. The loop itself was not reproduced on the iOS simulator, which has no network lever - iOS covers the fixed behaviour only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01441wbqMjaRA3DKBBZp52tC

…-latch # Conflicts: # Assets/Scripts/FirstLook/FirstLookConfig.cs # Assets/Scripts/FirstLook/FirstLookScreen.cs # README.md

The docs page had to paste a host snippet, because the half of the banner contract the controller cannot keep - the clock - only existed inside FirstLookScreen, mixed in with SDK init, the ATT gate, the interstitial and UI plumbing. A pasted snippet is a second copy, and it drifted: it declared its handler as FirstLookBannerController.Source, a type that does not exist, because nothing ever compiled it. FirstLookBannerHud is that clock as a MonoBehaviour, in one file. It owns the cooldown, the retry backoff, the wanted flag and the show/hide toggle, and it depends on nothing but FirstLookBannerController and FirstLookSource, so the docs can link it instead of copying it. Its three rules are the ones an integration gets wrong, and they are stated at the top of the file. FirstLookScreen keeps only what a demo screen should: initialization, button binding and status text. It drops from 390 lines to 332. The pass cooldown moves with the cycle it paces, so FirstLookConfig is now just the AdMob fallback ad unit ids. Verified on a Pixel 6 emulator (API 35) and an iPhone 17 Pro simulator (iOS 26.2). Cadence 30.17/30.13s Android and 31/30s iOS; hide gives zero ad requests for 24 minutes on Android and 120s on iOS, and re-show returns the banked ad with no new load; a load in flight at the hide still logs "not retrying while hidden" and starts nothing; the preload retry still runs with the banner never shown; interstitial unchanged on both. One small loss: the demo used to log the retry delay ("retrying in 8s") and now logs only whether it retried, because the backoff moved inside the hud. The two outcomes are still distinguishable via FirstLookBannerHud.IsWanted.

Copilot's review point on the docs PR applies here too: the README said a load in flight when the player hides "fails after the hide", as if that were certain. It is not. Hiding does not invalidate a load - a fill is banked for the next show, which is exactly the behaviour the pass cycle relies on. Only a load that then fails can re-arm the retry, so the bullet now says that. NextRetryDelay took its counter by ref back when the banner and the interstitial shared it. The banner's backoff moved into FirstLookBannerHud, so both remaining call sites pass the same field; it is now an instance method over _interstitialRetries. Same arithmetic, same counter. Also names the hud in the file table row for FirstLookScreen.

Copilot

🟡 Changes recommended

Hiding during an in-flight CloudX load can still start a new AdMob fallback request while hidden.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Extracts banner retry/cooldown orchestration into a reusable HUD component and prevents terminal retries while hidden.

Changes:

Adds FirstLookBannerHud to own banner lifecycle timing.
Simplifies screen/config integration.
Updates fallback and integration documentation.

File summaries

File	Description
`README.md`	Documents the extracted banner cycle and fallback setup.
`Assets/Scripts/FirstLook/FirstLookScreen.cs`	Delegates banner behavior to the HUD.
`Assets/Scripts/FirstLook/FirstLookConfig.cs`	Removes cooldown and forced-no-fill configuration.
`Assets/Scripts/FirstLook/FirstLookBannerHud.cs`	Implements banner visibility, cooldown, and retry handling.
`Assets/Scripts/FirstLook/FirstLookBannerHud.cs.meta`	Registers the new Unity script asset.

Review details

Files not reviewed (1)

Assets/Scripts/FirstLook/FirstLookBannerHud.cs.meta: Generated file

Files reviewed: 4/5 changed files
Comments generated: 1
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

A HUD is a heads-up display. This class has no UI at all - no UnityEngine.UI, no Text, Button, Canvas or Rect - and what it does hold is the pass cooldown, the retry backoff, ScheduleNextPass and the show/hide toggle. That is the pass cycle, which is the term the README, the docs page and the controller comments already use for it. The name also collided with the file that really is the display: AdScreenUi owns the labels and buttons. Having AdScreenUi draw while something called Hud kept time was backwards. The name came from the old docs snippet, which was called BannerHud; it was carried over when the file was extracted rather than chosen. Rule 3 in the file header still said a load in flight "still fails afterwards" - the same overstatement corrected elsewhere after Copilot's review point, missed here. A fill is banked for the next show; only a failure re-arms the retry. The meta guid is unchanged, so nothing that referenced the file loses it.

…fallback Copilot's review point on #9, confirmed on device. The retry guard in FirstLookBannerCycle only runs after AdLoadFailed, and the controller raises that only when the AdMob leg fails. Hiding while the CloudX leg was still in flight left CloudXOnLoadFailed free to call LoadAdMobFallback, so one AdMob request went out on a slot that was already off screen. That contradicts the "a hidden slot never keeps requesting" claim in the README and on the docs page, and my earlier zero-request measurements did not cover it: they were taken from after the failure had resolved. _wantShown cannot be the gate, because it is also false during the preload before the first Show, and the preload has to be allowed to reach the fallback - that is what banks an ad for the first tap. So this adds _hidden, set by Hide and cleared by Show, which separates "never shown yet" from "explicitly hidden". Measured on a Pixel 6 emulator, hiding inside the CloudX leg (network off, so the leg lasts through an HttpRetry cycle rather than failing instantly): with the gate load() 21:47:00.626, hide 21:47:00.825 -> 0 AdMob requests without the gate load() 21:49:25.594, hide 21:49:25.796 -> 1 AdMob request at 21:49:27.032, 1.2s after the hide Regression on the same build: pass cadence 30.27s, hide gives zero loads and zero AdMob requests over 70s, re-show returns the banked ad with no new load, and a preload against an invalid CloudX unit still falls back to AdMob.

Copilot

🟡 Changes recommended

Re-showing before an in-flight callback completes can revive the cancelled pass and bypass the restarted cooldown.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files not reviewed (1)

Assets/Scripts/FirstLook/FirstLookBannerCycle.cs.meta: Generated file

Suppressed comments (1)

Assets/Scripts/FirstLook/FirstLookBannerCycle.cs:120

Re-enabling _wanted here similarly revives a cancelled AdMob pass. When the player hides during an AdMob load and re-shows the retained creative before the no-fill callback arrives, that old callback reaches OnAdLoadFailed after _wanted became true and schedules a retry, so the new show's 30-second cooldown is bypassed. Keep the cancelled in-flight pass distinct from the slot's current wanted state and suppress its retry even after a re-show.

        _wanted = true;

Files reviewed: 5/6 changed files
Comments generated: 1
Review effort level: Balanced

Copilot's follow-up on #9, confirmed on device. The previous commit cleared the flag in Show(), which tracks whether the slot is currently visible rather than whether the running pass was cancelled. Hide then a quick re-show while a CloudX load is still in flight therefore revived it: the stale terminal callback arrived with the flag already cleared, started the fallback at once, and jumped the 30s cooldown the re-show had just restarted. The flag now belongs to the pass. Hide sets it; only the start of a new pass in Load() clears it. Show leaves it alone, so a cancelled pass stays cancelled however the slot is toggled afterwards. The same reasoning applies to the AdMob leg, which Copilot raised as a second case: its terminal failure is no longer forwarded to the host for a cancelled pass, so the host cannot schedule a retry that jumps the cooldown either. Measured on a Pixel 6 emulator, hiding and re-showing inside the CloudX leg (network off, so the leg lasts through an HttpRetry cycle): pass-scoped flag hide 22:05:41.507, show 22:05:41.588 -> 0 AdMob requests cleared on Show hide+show 22:07:31.846 -> 2 AdMob requests Regression: cadence 30.23s, hide gives zero loads and zero AdMob requests over 65s, re-show returns the banked ad with no new load. Copilot also noted that a cooldown load firing before a stale callback is dropped by the _isLoadingCloudX guard. That is the pre-existing no-per-pass- timeout gap already recorded as a follow-up; it predates this branch and is not addressed here.

Copilot

🟡 Changes recommended

Integration guidance and the declared PR scope remain inconsistent with the extracted component and controller changes.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files not reviewed (1)

Assets/Scripts/FirstLook/FirstLookBannerCycle.cs.meta: Generated file

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

Assets/Scripts/FirstLook/FirstLookBannerController.cs:30

The controller’s updated integration contract still says Show() starts the cycle, while the header at lines 8–11 tells users to copy only this controller and FirstLookSource and calls this file “the whole flow.” After this extraction, following those instructions omits FirstLookBannerCycle, so no cooldown/retry clock is installed and the banner stalls after its first pass. Update this class-level guidance to name all three required files and make the cycle responsible for timing.

Files reviewed: 5/6 changed files
Comments generated: 1
Review effort level: Balanced

The controller header still told readers to copy this file and FirstLookSource.cs and called it "the whole flow". That was true before the timing was extracted; following it now leaves nothing driving the cycle, so the banner shows one ad and stops. It names all three files, and says what happens if you take only this one. The two host rules stay, since anyone driving the controller from their own component still needs them, but they now say FirstLookBannerCycle already does both. The Hide-ends-the-pass behaviour moves out of that list, because it is this controller's job rather than the host's. FirstLookInterstitialController keeps the same two-file wording: a fullscreen ad is consumed by being shown, so it needs no clock and the claim is accurate there.

Copilot

🟡 Changes recommended

Successful callbacks can still revive a cancelled pass after a quick re-show.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files not reviewed (1)

Assets/Scripts/FirstLook/FirstLookBannerCycle.cs.meta: Generated file

Files reviewed: 5/6 changed files
Comments generated: 1
Review effort level: Balanced

Copilot's third-round point, and it is a violation of the contract the header gained last round: it says a later Show does not revive a cancelled pass, and the success callbacks did exactly that. Hide, then a quick Show, then the stale CloudX or AdMob fill arrives with its loading flag still set, so it raised PassSpent and reset the cooldown from a pass the player had already dismissed. The cause is that one flag carried two meanings. spendsPass answered both "did this controller ask for the load" - which decides whether the fill is banked, and whether it may take the slot from the other source - and "should this re-time the cycle". Those come apart exactly once: a pass a Hide cancelled is still ours, but its timing is not. So they are two flags now. "ours" keeps the banking and source-replacement behaviour unchanged, and spendsPass is ours && !_passCancelled. A cancelled pass's fill is still banked while the slot is hidden, and still shows if the slot has since been shown - throwing away an ad we paid a request for would be worse - but the cooldown stays with whatever the host scheduled after the hide. Regression on a Pixel 6 emulator: cadence 30.23s, hide gives zero loads and zero AdMob requests over 60s, re-show returns the banked ad with no new load.

Begin's comment said "after both SDKs have answered", which contradicts what FirstLookScreen does and what the docs page now says. The screen starts both initializations and calls Begin on the CloudX result alone, because Google Mobile Ads queues loads issued while it is still initializing and the fallback is lazy regardless. Part of a three-way inconsistency Copilot found across this comment, the Info block on the docs page, and the usage section; the other two are fixed in cloudx-io/docs#407.

Copilot

🟢 Approval recommended

The cancellation flow is coherent, with only a non-blocking unreachable diagnostic branch remaining.

Review details

Files not reviewed (1)

Assets/Scripts/FirstLook/FirstLookBannerCycle.cs.meta: Generated file

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

Assets/Scripts/FirstLook/FirstLookScreen.cs:263

FirstLookBannerController drops cancelled AdMob failures before raising this event (lines 491–493), and cancelled CloudX failures never raise it either, so every failure reaching this handler still has IsWanted == true. The not retrying while hidden arm is therefore unreachable and cannot produce the diagnostic cited in verification; log the retry outcome unconditionally here (or add a distinct cancellation event if cancellations must be observable).

Files reviewed: 5/6 changed files
Comments generated: 0 new
Review effort level: Balanced

Copilot's non-blocking note, and it is right. Once the controller started dropping the terminal failure of a cancelled pass, nothing reaching the screen's AdLoadFailed handler can be from a hidden slot, so the "not retrying while hidden" arm became dead and IsWanted lost its only caller. The handler now logs the one outcome that can actually happen. This also corrects the record: that log line was real evidence when it was gathered, against the build at the time, but the current build cannot emit it. What replaces it as evidence is the request count, which is the thing that actually matters - zero AdMob requests after a hide, measured against a control build that produces one. The cycle keeps its _wanted guard even though the controller now makes it unreachable, because the rule is the host's to keep: anyone driving the controller from their own component needs that line. The comment says so rather than leaving it looking like live logic. Regression: cadence 30.25s, zero loads and zero AdMob requests over 55s hidden, re-show returns the banked ad with no new load.

Copilot

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

Copilot

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

Copilot

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

Copilot

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

Suppressing the terminal callback of a pass a Hide cancelled stops a hidden slot requesting, but it opened a window where a visible banner could stop refreshing. Hide, then Show, leaves _passCancelled set while the cancelled load is still on the network; the Show raises PassSpent and arms the cooldown; that tick then finds the load still in flight, Load() early-returns, and the flag is never cleared. When the stale load completes it is dropped, so no PassSpent and no AdLoadFailed reach the host, nothing is pending, and the cycle stalls with an ad on screen until the next toggle. Clearing the flag above the guard fixes it: a load is only ever asked for on a slot that is wanted, so it supersedes the cancellation even when the guard drops the call, and the in-flight pass carries the cycle forward instead of being silenced. Show still does not clear it, so a Hide with no following Load cancels the pass completely. Load() now leans on the host never calling it while the slot is hidden, so that rule joins the two the header already listed, and the note about what revives a cancelled pass is corrected to match. README: rewrap the hide bullet, which had grown to 141 columns and stated the cancellation twice.

Copilot

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

Copilot

🔵 Needs a closer look

The asynchronous cross-SDK race handling was not reproduced on iOS and warrants final human review.

Review details

Files not reviewed (1)

Assets/Scripts/FirstLook/FirstLookBannerCycle.cs.meta: Generated file

Files reviewed: 5/6 changed files
Comments generated: 0 new
Review effort level: Balanced

Copilot

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

Copilot

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

Copilot

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

The rule as written forbade the preload, which the cycle itself performs from Begin before anything has been shown. What Load must not follow is a Hide; before the first Show nothing has been dismissed, and the preload has to be allowed to reach the fallback - as the _passCancelled comment already says. Reword to name the dismissal rather than the absence of an ad on screen.

antonurankar-moloco and others added 8 commits September 4, 2026 14:42

Merge remote-tracking branch 'origin/main' into tomi-firstlook-inline…

6d29dce

…-latch # Conflicts: # Assets/Scripts/FirstLook/FirstLookConfig.cs # Assets/Scripts/FirstLook/FirstLookScreen.cs # README.md

antonurankar-moloco requested a balanced review from Copilot September 6, 2026 19:29

Copilot started reviewing on behalf of antonurankar-moloco September 6, 2026 19:29 View session

Copilot AI reviewed Sep 6, 2026

View reviewed changes

Comment thread Assets/Scripts/FirstLook/FirstLookBannerCycle.cs Outdated

antonurankar-moloco added 2 commits September 6, 2026 21:33

antonurankar-moloco requested a balanced review from Copilot September 6, 2026 19:59

Copilot started reviewing on behalf of antonurankar-moloco September 6, 2026 20:00 View session

Copilot AI reviewed Sep 6, 2026

View reviewed changes

Comment thread Assets/Scripts/FirstLook/FirstLookBannerController.cs Outdated

antonurankar-moloco requested a balanced review from Copilot September 6, 2026 20:13

Copilot started reviewing on behalf of antonurankar-moloco September 6, 2026 20:14 View session

Copilot AI reviewed Sep 6, 2026

View reviewed changes

Comment thread Assets/Scripts/FirstLook/FirstLookBannerController.cs

antonurankar-moloco requested a balanced review from Copilot September 6, 2026 20:19

Copilot started reviewing on behalf of antonurankar-moloco September 6, 2026 20:19 View session

Copilot AI reviewed Sep 6, 2026

View reviewed changes

Comment thread Assets/Scripts/FirstLook/FirstLookBannerController.cs Outdated

antonurankar-moloco added 2 commits September 6, 2026 22:26

antonurankar-moloco requested a balanced review from Copilot September 6, 2026 20:29

Copilot started reviewing on behalf of antonurankar-moloco September 6, 2026 20:29 View session

Copilot AI reviewed Sep 6, 2026

View reviewed changes

antonurankar-moloco requested a balanced review from Copilot September 7, 2026 07:23

Copilot started reviewing on behalf of antonurankar-moloco September 7, 2026 07:24 View session

Copilot AI reviewed Sep 7, 2026

antonurankar-moloco requested a balanced review from Copilot September 7, 2026 07:30

Copilot started reviewing on behalf of antonurankar-moloco September 7, 2026 07:31 View session

Copilot AI reviewed Sep 7, 2026

antonurankar-moloco requested a balanced review from Copilot September 7, 2026 07:41

Copilot started reviewing on behalf of antonurankar-moloco September 7, 2026 07:42 View session

Copilot AI reviewed Sep 7, 2026

antonurankar-moloco requested a balanced review from Copilot September 7, 2026 07:44

Copilot started reviewing on behalf of antonurankar-moloco September 7, 2026 07:44 View session

Copilot AI reviewed Sep 7, 2026

antonurankar-moloco requested a balanced review from Copilot September 7, 2026 11:41

Copilot started reviewing on behalf of antonurankar-moloco September 7, 2026 11:41 View session

Copilot AI reviewed Sep 7, 2026

antonurankar-moloco requested a balanced review from Copilot September 7, 2026 11:50

Copilot started reviewing on behalf of antonurankar-moloco September 7, 2026 11:51 View session

Copilot AI reviewed Sep 7, 2026

View reviewed changes

antonurankar-moloco requested a balanced review from Copilot September 7, 2026 12:02

Copilot started reviewing on behalf of antonurankar-moloco September 7, 2026 12:03 View session

Copilot AI reviewed Sep 7, 2026

antonurankar-moloco requested a balanced review from Copilot September 7, 2026 12:06

Copilot started reviewing on behalf of antonurankar-moloco September 7, 2026 12:06 View session

Copilot AI reviewed Sep 7, 2026

antonurankar-moloco requested a balanced review from Copilot September 7, 2026 12:15

Copilot started reviewing on behalf of antonurankar-moloco September 7, 2026 12:16 View session

Copilot AI reviewed Sep 7, 2026

antonurankar-moloco merged commit 5addc7e into main Sep 7, 2026

Uh oh!

Conversation

antonurankar-moloco commented Sep 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1. The reported defect: a retry armed on a hidden slot

2-5. Four more, found in review

The extraction

ForceCloudXNoFill removed

Interstitial: not the same hole

Verification

Scope

Follow-ups, not in this PR

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Files not reviewed (1)

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Files not reviewed (1)

Suppressed comments (1)

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Files not reviewed (1)

Suppressed comments (1)

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Files not reviewed (1)

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Files not reviewed (1)

Suppressed comments (1)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Files not reviewed (1)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

antonurankar-moloco commented Sep 6, 2026 •

edited

Loading

`ForceCloudXNoFill` removed