Reload the Ad wont trigger `loadedChanged` signal after first time.

[li3p]

1. After A Ad has loaded successfully , internal code will call setLoaded(true) to emit loadedChanged signal.

2. But In some circumstance, we need re-load an Ad , then we should call ::load() again , but there is no code call the setLoaded(false) , so the reload wont emit loadedChanged signal any more.

3. To fix this

   • change the loadedChanged() signal to loadedChanged(bool loaded)
   • call setLoaded(false) before every emit loading()

I will make a PR for this.

[larpon]

But In some circumstance, we need re-load an Ad

I agree on resetting before calling ::load again - that's a bug. But I need to know under what circumstances this is needed? For example AdMobBanner will reload automatically and I don't know of any methods to intercept this event?

So your fix in #36 that sets loaded to false before loading again should only be for AdMobInterstitial and AdMobRewardedVideoAd - the other types are automatically reloaded - a flow you can only control by adjusting the interval in the admob web interface? EDIT Just found out that loaded is actually set to false based on the closing Firebase events: https://github.com/Larpon/QtFirebase/blob/master/src/qtfirebaseadmob.cpp#L1530 https://github.com/Larpon/QtFirebase/blob/master/src/qtfirebaseadmob.cpp#L1801

change the loadedChanged() signal to loadedChanged(bool loaded)

It's not common practice to supply the changed value as an argument to the signal (at least not in QML). The value can be read in both QML and C++ so loadedChanged should stay as it is without the bool argument.

QML

onLoadedChanged: {
    console.log(loaded)
}

C++

if(loaded()) {
    ...
}

So in conclusion: I need to see how and when you'll be reloading either an AdMobBanner or an AdMobNativeExpressAd?

[li3p]

That requirement comes from how the sdk works underlying :

1. We usually created the AdMobBanner and AdMobNativeExpressAd after the application started. And we do had set the refresh settings in the AdMob web console, and we known the minimum refresh interval is 30 secs.

2. When user using the app, he may switch between different UI screens. and we change the visibility of different Ads according which screen is on top now.

3. But I found the auto-refreshing function provided by the Ads SDK is : when your Ad is visible, it will refresh it according to the settings you set in AdMob web console; but when your Ad is not visible currently, It will not refresh it and will schedule next check after 60 seconds. So this will lead to a possibility that our Ad will never be refreshed again (because only when user switch to that screen and 60 seconds happen to timeout at that time, the Ad will refresh again) .

4. And also because the impressions it a key factor to improve your eCPM performance （within a reasonable range, the more different impressions, the more chances user would click the Ad）, So we changed our policy to reload the Ad after setting a Ad's visibility to false.

Above is the reason that why we need reload it.

It's not common practice to supply the changed value as an argument to the signal (at least not in QML). The value can be read in both QML and C++ so loadedChanged should stay as it is without the bool argument.

That is ok if it is really not a common practice. Following is my code:

void AdMob::onNativeExpressAdQuitLoaded()
{
    if (false == m_nativeExpressAdQuit->loaded())
        return;

    qDebug()  << this << "::onNativeExpressAdQuitLoaded::loaded";
    setLoading(Ad_AppQuit, false);
    m_nativeExpressAdQuit->moveTo(m_nativeAdQuitX, m_nativeAdQuitY);
    if (m_screen == m_screens[Ad_AppQuit] && quitAdEnabled()){
        m_nativeExpressAdQuit->show();
    }
}

You can see I did not use the parameter too, but by reading some other codes makes me think with a parameter is a common practice.

[larpon]

You're right. I can see in my example app that the ad area is black after a visibility change - I guess something changed in the flow in the sdk since I started the project (or I never tested it :P ).

Hmm... What project is the above code snippet from? (just curious) My point is that you won't find any value parameters in signals in e.g. QtQuick - so my intention is just to keep QtFirebase it as much "Qt/QML" as we can :)

Can you provide a PR where the signal has no value parameter - and only the AdMobBanner and AdMobNativeExpressAd has the 'setLoaded(false)' before the actual load code?

[li3p]

FYI: The above code is from a Sudoku game app I made recently.

I will make the PR you requested soon.

[li3p]

PR updated, please help me with review @Larpon . thanks.

[larpon]

@li3p - perfect. Thanks! It's merged now