googleads-mobile-flutter

googleads-mobile-flutter copied to clipboard

[REQUIRED] Step 2: Describe the problem

The entire google_mobile_ads API is built with callback APIs. These are bug prone and very non-idiomatic for Flutter. Really non-idiomatic in any language post ~2005. This means each dev more or less needs to write a wrapper around the current API to convert it to Flutter idiomatic futures.

Here is an example of what the API would look like using Flutter/Dart idioms:

// Future allows the dev to use `FutureBuilder` instead of customized stateful logic.
Future<RewardedAd> ad = RewardedAd.load();

// Now we don't have to navigate several different callbacks.
// The stream will complete with either an earned or 
// dismissed event to indicate the outcome of showing the
// ad.
Stream<RewardedEvent> result = (await ad).show();

// Class that signals events while the ad is up.
// Subclasses could include things like:
//    RewardEarned(Reward reward);
//    AdDismissed(DismissDetails details);
abstract class RewardedAdEvent { ... }

However even this isn't very Flutter idiomatic. It really isn't even going far enough. The ideal would be a widget using the controller pattern roughly as follows (see ScrollController and Scrollable for full Flutter idiomatic examples of the controller pattern):

// Wrap a widget subtree in this to allow it to show ads.
class RewardedAdView extends StatefulWidget {
  static RewardedAdController of(BuildContext context) { ... }

  // Can be passed in the constructor or makes a default one.
  RewardedAdController controller;
  ...
}

class _RewardedAdView extends State<RewardedAdView> {
  Widget build(BuildContext context() {
    // Wraps child in a provider so that the subtree can call `.of`.
    return _RewardedAdControllerProvider(controller: widget.controller, ...);
  }
}

class _RewardedAdControllerProvider extends InheritedWidget {
  RewardedAdController controller;
  ...
}

// Manages current ad state.
class RewardedAdController  with ChangeNotifier {
  RewardedAd? ad;

  // Sets `ad`. Could possibly be private and just called
  // by the view class's `initState` method.
  Future<void> load() { ... }

  // Show the ad and get the results stream.
  Stream<RewardedAdEvent> show() async* { ... };
}

// Somewhere in a build function.
return MyButton(
  child: Text('Oh yeah! Show me an ad, I love ads!'),
  onPressed: () {
    final result = RewardedAdView.of(context).show();
    ...
  },
);

I think this is a wide scope issue and may need a lot of codebase refactoring, not sure about the feasibility and amount of effort required for now. So, I'm labeling this with a single label for more information from others.

This would be a welcome change!!! We already experienced a huge amount of refactoring from the 0. versions of the plugin until today, so anything that is more like what we are used to using everyday, will likely cause a lot less issues. The way to use the plugin is a bit weird and doesn't feel like the rest of my code.

Any news here? The existing API is really hard to use.

I'm now adding Banner Ads to my app and am reminded how hilariously hard to use the AdMob APIs are: https://codelabs.developers.google.com/codelabs/admob-ads-in-flutter#6

@override
void initState() {
  ...

  // Why are we creating an instance if we're not going to hold a reference to it??
  BannerAd(
    adUnitId: AdHelper.bannerAdUnitId,
    request: AdRequest(),
    size: AdSize.banner,
    // Why is a simple callback it's own class??????
    listener: BannerAdListener(
      onAdLoaded: (ad) {
        setState(() {
          // WHHHHHYYYYYY do we have to do type casting here?????
          _bannerAd = ad as BannerAd;
        });
      },
      onAdFailedToLoad: (ad, err) {
        print('Failed to load a banner ad: ${err.message}');
        ad.dispose();
      },
    ),
  ).load();
}

This feels like 2010s "Enterprise grade" Java or something. This is really painful. I hope you guys fix it.

@caseycrogers this is something we are actively looking into but no immediate news to share. However, comments like these show the importance of this feature request. Thanks

I have done this using simple completers. If someone wants inspiration for using this api. Maybe these methods can be a part of the sdk one day...

Future<RewardedInterstitialAd> _populateRewardedInterstitialAd({
    required String adUnitId,
    required VoidCallback onAdDismissedFullScreenContent,
    required VoidCallback onAdShowedFullScreenContent,
  }) async {
    try {
      final adCompleter = Completer<Ad?>();
      await RewardedInterstitialAd.load(
        adUnitId: adUnitId,
        request: const AdRequest(),
        rewardedInterstitialAdLoadCallback: RewardedInterstitialAdLoadCallback(
          onAdLoaded: (ad) {
            ad.fullScreenContentCallback = FullScreenContentCallback(
              onAdShowedFullScreenContent: (ad) {
                onAdShowedFullScreenContent();
              },
              onAdDismissedFullScreenContent: (ad) {
                onAdDismissedFullScreenContent();
              },
            );

            adCompleter.complete(ad);
          },
          onAdFailedToLoad: adCompleter.completeError,
        ),
      );

      final rewardedInterstitial = await adCompleter.future;
      if (rewardedInterstitial == null) {
        throw const AdsClientException('Rewarded Interstitial Ad was null');
      }
      return rewardedInterstitial as RewardedInterstitialAd;
    } catch (error, stackTrace) {
      Error.throwWithStackTrace(AdsClientException(error), stackTrace);
    }
  }