2

Closed

Bug in source code. Method LoadAdSettings.

description

Hello,

I noticed and have already wrote that remote config file is not used. It is reproduced by the following steps:
  1. Run sample 'BlankXNAProject-LocationAware' and notice that different ADs are served.
  2. Set in default config file all probabilities to Zero except AdDuplex. Set AdDuplex to 100%.
  3. Run sample again and notice that only AdDuplex is served (each time, regardless that remote file contains not zero probabilities for different ADs).
I found a reason why it happens. The problem is in method "LoadAdSettings".
  1. If "FetchAdSettingsThreaded" is called then always default XML file is used. (because XML file retrieving from storage happens only if "FetchAdSettingsThreaded" is not called)
  2. Line 1286 in "AdRotatorXNA.cs" contains following: var settings = (AdSettings)xs.Deserialize(isfStream);
    It makes no sense to put retrieved settings object in the local variable and not use it afterwards.
  3. It is necessary as well to add the following method call "LoadAdSettings();" in the method "FetchAdSettingsFile" after remote file is loaded (to load just saved remote file and use it).
I transformed this method to the following, and it works perfect. I tested above steps and noticed that even if "default" settings file contains zero probabilities for all ADs except AdDuplex, different ADs are served, because remote file contains not zero probabilities for all ADs. It means that remote file is used.

private void LoadAdSettings()
    {
        //If not checked remote && network available - get remote
        if (!_remoteAdSettingsFetched && !String.IsNullOrEmpty(SettingsUrl) && _isNetworkAvailable)
        {
            FetchAdSettingsThreaded();
        }
        //else
        //DON'T NEED HERE 'ELSE' CONDITION, 
        //BECAUSE OTHERWISE WHEN APPLICATION IS STARTED ALWAYS DEFAULT SETTINGS ARE TAKEN
        {
            try
            {
                var isfData = IsolatedStorageFile.GetUserStoreForApplication();
                IsolatedStorageFileStream isfStream = null;
                if (isfData.FileExists(SETTINGS_FILE_NAME))
                {
                    using (isfStream = new IsolatedStorageFileStream(SETTINGS_FILE_NAME, FileMode.Open, isfData))
                    {
                        XmlSerializer xs = new XmlSerializer(typeof(AdSettings));
                        try
                        {
                            //MAKES NO SENSE TO ASSIGN HERE TO LOCAL VARIABLE
                            //var settings = (AdSettings)xs.Deserialize(isfStream);
                            //return settings;
                            _settings = (AdSettings)xs.Deserialize(isfStream);                                
                        }
                        catch
                        {

                        }
                    }
                }
            }
            catch (IsolatedStorageException)
            {
            }
        }

        if (_settings == null)
        {
            _settings = GetDefaultSettings();
            OnLog("Failed Setings retrieved from local trying defaults");
        }
        if (_settings == null)
        {
            OnLog("Ad control disabled no settings available");
            IsEnabled = false;
        }
        else
        {
            //Everything OK, continue loading
            _initialised = true;
            System.Windows.Deployment.Current.Dispatcher.BeginInvoke(Invalidate);
        }
        //return _settings;
    }
public void FetchAdSettingsFile(object state)
    {
        var request = (HttpWebRequest)HttpWebRequest.Create(new Uri(SettingsUrl));
        request.BeginGetResponse(r =>
        {
            try
            {
                var httpRequest = (HttpWebRequest)r.AsyncState;
                var httpResponse = (HttpWebResponse)httpRequest.EndGetResponse(r);
                var settingsStream = httpResponse.GetResponseStream();

                var s = new System.Xml.Serialization.XmlSerializer(typeof(AdSettings));
                _settings = (AdSettings)s.Deserialize(settingsStream);
                // Only persist the settings if they've been retreived from the remote file
                SaveAdSettings(_settings);
                _remoteAdSettingsFetched = true;
                _initialised = true; OnLog("Setings retrieved from remote");
                LoadAdSettings();//TO LOAD JUST SAVED FILE
            }
            catch
            {
                _remoteAdSettingsFetched = true;
                _initialised = true;
                LoadAdSettings(); // GetDefaultSettings();
            }
        }, request);
    }
Closed Oct 1, 2013 at 3:34 PM by Darkside
Included in 1.1

comments

phoenecke wrote Sep 12, 2013 at 3:17 AM

Worked for me. Without this, the remote settings are never used at all for me.

Darkside wrote Sep 12, 2013 at 10:33 AM

Ok, in the next update we'll review and add your code into the base.

Thanks for your effort

phoenecke wrote Sep 13, 2013 at 1:05 AM

Instead of removing the else as suggested, I instead add a return; after FetchAdSettingsThreaded(). I kept RadikSalakhov's other changes.
if (!_remoteAdSettingsFetched && !String.IsNullOrEmpty(SettingsUrl) && _isNetworkAvailable)
{
     FetchAdSettingsThreaded();
     return;
}
else
{
This way no ad is shown until the remote settings are fetched (success or not).

Darkside wrote Oct 1, 2013 at 3:28 PM

Included in the 1.1 beta release

wrote Oct 1, 2013 at 3:34 PM