sdkbox / issues

The issue tracker for SDKBOX
MIT License
4 stars 0 forks source link

Change volume / mute ads sound #59

Open angelvet opened 5 years ago

angelvet commented 5 years ago

Some users complains that they can't control or mute ads sound. I think this will be a great feature for AdMob plugin to support this apis: Mobile Ads, setup/mute sound

Is there any chance this will be implemented. Thanks.

ghost commented 5 years ago

yes, I'll add this feature .

angelvet commented 4 years ago

Hi @yinjimmy , I have updated sdkbox up to v2.5.0.6 and trying to integrate new setAppMuted and setAppVolume methods. I have tried to call setAppMuted(false) and setAppVolume(1.0) and also setAppMuted(true) and setAppVolume(0.0). In both cases interstitial and rewarded video ads are playing with sounds turned on. I'm using AdMob plugin to show ads.

How this apis are supposed to be used? Thanks.

EDIT: When calling above api methods I see next errors: 11-15 21:30:01.150 20442-20482/? E/JniHelper: Classloader failed to find class of org.cocos2dx.lib.Cocos2dxHelper 11-15 21:30:01.150 20442-20482/? E/JniHelper: Failed to find class org.cocos2dx.lib.Cocos2dxHelper 11-15 21:30:01.151 20442-20482/? E/JniHelper: Failed to find static java method. Class name: org.cocos2dx.lib.Cocos2dxHelper, method name: setStringForKey, signature: (Ljava/lang/String;Ljava/lang/String;)V

ghost commented 4 years ago

let me check next week.

angelvet commented 4 years ago

Hello, are there any news? I have checked SDKBox 2.5.1.2, ad sound still can't be muted :(

angelvet commented 4 years ago

Is mute ads sound fixed in SDKBox 2.6.0.1? Thanks.

ghost commented 4 years ago

Hi, I think it has been fixed.

I used

MobileAds.setAppMuted(muted);
MobileAds.setAppVolume(volume);

internal.

angelvet commented 4 years ago

Hello @yinjimmy ,

I have upgraded to v2.6.0.1 and it still isn't working,

After calling sdkbox::PluginAdMob::setAppVolume() andsdkbox::PluginAdMob::setAppMuted() any following JNI call on android is failing. For example see this logcat:

E/JniHelper: Classloader failed to find class of org/cocos2dx/lib/Cocos2dxDataManager E/JniHelper: Failed to find class org/cocos2dx/lib/Cocos2dxDataManager E/JniHelper: Failed to find static java method. Class name: org/cocos2dx/lib/Cocos2dxDataManager, method name: onSceneLoaderBegin, signature: ()V

So I tried to test this on emulator (Android 9.0) and got more details on what's happening:

[ java_vm_ext.cc:545] JNI DETECTED ERROR IN APPLICATION: JNI GetObjectClass called with pending exception java.lang.IllegalStateException: MobileAds.initialize() must be called prior to setting the app volume. [ java_vm_ext.cc:545] at void com.google.android.gms.common.internal.Preconditions.checkState(boolean, java.lang.Object) (com.google.android.gms:play-services-basement@@17.2.1:2) [ java_vm_ext.cc:545] at void com.google.android.gms.internal.ads.zzxu.setAppVolume(float) (com.google.android.gms:play-services-ads-lite@@19.0.1:2) [ java_vm_ext.cc:545] at void com.google.android.gms.ads.MobileAds.setAppVolume(float) (com.google.android.gms:play-services-ads-lite@@19.0.1:1) [ java_vm_ext.cc:545] at void com.sdkbox.plugin.PluginAdMob.setAppVolume(float) (PluginAdMob.java:1)

But I'm surely calling setAppVolume and setAppMuted after the sdkbox::PluginAdMob::init() call on applicationDidFinishLaunching.

Is this true that MobileAds.initialize() called under the sdkbox::PluginAdMob::init()? Maybe there are some errors occurs during this init call?

Other warnings that I saw on the log are (maybe not related to this issue):

E/AdMob: WARNING: set appid with sdkbox_config.json E/AdMob: is not in sdkbox_config.json

ghost commented 4 years ago

E/AdMob: WARNING: set appid with sdkbox_config.json E/AdMob: is not in sdkbox_config.json

thats the issue. I think it's better to crash the app to let developer know that the appid is mandatory.

What do you think ?

angelvet commented 4 years ago

Thanks @yinjimmy , adding appid into sdkbox_config.json fixed this issue,

As for Android, according to the Google Mobile Ads docs AdMob App ID must be added into the AndroidManifest.xml (since version 17.0.0). Failure to add this App ID as a tag will result in a crash with the message: The Google Mobile Ads SDK was initialized incorrectly.

Therefore, I think that when appid is missing from sdkbox_config.json, then sdkbox could check AndroidManifest.xml and use appid from there.

If app id has been found on both locations, maybe you could give more weight for that one that is specified in sdkbox_config.json, but I think both appids must be equal to work properly.

App will crash anyway if appid is missing from both locations.

I didn't check how Google Mobile Ads looks for appid on iOS.

angelvet commented 4 years ago

Hi @yinjimmy,

Adding appid into sdkbox_config.json perfectly fixed the above issues but there is another one appeared.

Some users experiencing ANRs issues. Application is stucking on MobileAds.initialize call. This is happening on Huawei devices with Android 7.

Obviously this is not an sdkbox issue but Google Mobile Ads SDK fault. By looking on documentation I found that there are several MobileAds.initialize signatures available:

static void initialize(Context context, String applicationCode);
static void initialize(Context context);
static void initialize(Context context, String applicationCode, MobileAds.Settings settings);
static void initialize(Context context, OnInitializationCompleteListener listener);

Is it possible to provide ability to choose what initialize routine will be called by SDKBox? I would like to know if Mute functions will work when initialize(Context context) will be called instead ofinitialize(Context context, String applicationCode)?

If that true then option to choose initialize routine could help.

ghost commented 4 years ago

in next version, sdkbox-admob will use static void initialize(Context context); api.

would you help to test with it PluginAdMob.jar.zip

angelvet commented 4 years ago

I have tested PluginAdMob.jar. As a result I could say that it's doesn't matter what initialize call to choose. Unfortunately, on that particular Huawei devices with Android 7, any of the first two initialize calls leads to ANR. So, for that users, my only option is to use initialize(appid) call with empty appid and don't use mute/volume calls.

Thanks for letting me to check this.

ghost commented 4 years ago

what about static void initialize(Context context, OnInitializationCompleteListener listener); , does it leads to ANR ?

ghost commented 4 years ago

how do you initialize sdkbox-admob ?

if (device-is-HW-android-7)
{
    sdkbox::setConfig(hw_config);
}
else
{
    sdkbox::setConfig(config);
}

right ?

I want to remove the appid filed in sdkbox_config.json, does it affect you ?

only use MobileAds.initialize(mContext); to initialize AdMob;

angelvet commented 4 years ago

what about static void initialize(Context context, OnInitializationCompleteListener listener); , does it leads to ANR ?

I don't know, I can't test it, I'm using Mobile Ads only through out SDKBox. I only tested latest SDKBox version and that custom build you gave me above.

how do you initialize sdkbox-admob ?

In the end I decided not to use ads mute functionality for now. I use latest SDKBox without adding appid into sdkbox_config.json because I can't control which MobileAds.initialize method will be called.

I want to remove the appid filed in sdkbox_config.json, does it affect you ?

It seems that the only way to get around of Android 7 ANR issues on Huawei devices is to call initialize(Context context, String applicationCode) overload with an empty application id. I agree that keeping appid in the sdkbox_config.json is unnecessary and could be removed.

What about introducing second init() function into the PluginAdMob interface with only one application id parameter? So first init overload will call MobileAds.initialize(mContext) (but please look at my next comment), and the second one - initialize(Context context, String applicationCode)? This could help me to choose correct one depending on the Android version. Maybe you could suggest a better solution?

only use MobileAds.initialize(mContext); to initialize AdMob;

I think that this simple changing of init routine can break current SDKBox users that didn't use appid in the sdkbox_config.json. After the update they will start seeing that same ANRs we discussed earlier. appid parameter is not mentioned in the PluginAdMob docs, so I could assume that the most of the users didn't use it in the sdkbox_config.json.

ghost commented 4 years ago

If we turn sdkbox into a paid model, subscribe or buy out, would you consider supporting it? How much do you think it is acceptable to integrate SDK in each project?

angelvet commented 4 years ago

It's a difficult question :) Depends on a paid model. If it will have subscription model similar to that one that Unity has where price (free or paid) depends on the client revenue I would subscribe when my revenue will became high enough. But SDKBox contains a lot of plugins, not related to the revenue. On my opinion if SDKBox turns into a paid model, having a free version to support devs that doesn't earn anything (or earn a little) from theirs projects is reasonable.

ghost commented 4 years ago

Thanks for your reply.