Duplicate Banner Ads

I had the same problem, and now I found the cause. The banner_db.php module (correctly) tries to prevent a banner from occurring multiple times by removing chosen banners from the ballot.

The problem is that if one gives chances larger than 1, the ballot gets filled with multiple instances of a banner (to increase its chances to get picked). Once a banner is picked, the module removes only one instance, but leaves the remaining instances in the ballot. Then they can get picked again, and so on.
The problem does not occur if all banners have chance equal to 1, since every banner is only one time in the ballot and will get removed when picked.

I have most banners with a chance of 10, except a rare one which I gave 1. If the module picks one of my high-probability banners, it removes one of the 10, but then there are still 9 left in the ballot. The correct way would be to remove the adjacent identical banners in both directions from the one picked. That way all 10 get removed from the ballot and cannot be picked anymore.

I am not sure how to do this the fastest (since there could be banners with a chance of 100), but going in a loop to remove one by one seems the only thing coming to my mind. If the array_splice command is slow, maybe one could just count the positions and at the end take out the section from position1 to position 2 (the first and last occurrence of the same banner instances). Maybe there is a faster way, e.g. replacing all the identical banners in the array with "" and then throw them out. Not sure if there are slick array functions for this. I will keep thinking. Contact me if you find a better solution.

This might not be the most elegant way, but I fixed it in a way that only adds a few lines and can be understood easily. Since it doesn't dig around within the array numbers, I also thought it is less error prone. Regarding speed, it would only slow down the algorithm if the random number generator keeps picking the same banner, e.g. if its chance was, say 100. But worst case, all 100 are removed from the ballot, and then the random number generator can move on. In a normal scenario, there are probably just a few banners to be displayed out of a large pool of banners. So it is a rare occurrence that the random number generator wouldn't find a new banner right away.

+ $banners_chosen=array(); //rseiser: start with array where no banners were chosen yet
  $counter = 0;
  while ($counter < $count && count($ballot)) {
    // choose random banner
    $max = count($ballot) - 1;
    if ($max > 0) {
      $random = mt_rand(0, $max);
    }
    else {
      $random = 0;
    }
    $nid = $ballot[$random];

-   // get banner and remove it from the ballot
-   $banner = $banners[$nid];
+   // remove banner from the ballot
    array_splice($ballot, $random, 1);
+   if(!isset($banners_chosen[$nid])) { //rseiser: If this banner was not chosen yet. Else we basically just skip over and try again
+   $banners_chosen[$nid]=$nid; //rseiser: add banner to the list of chosen. Use $nid as array key for fast finding
+   $banner = $banners[$nid];

    // update view statistics, admin and owner views are not counted
    if ($uid != 1 && $uid != $banner->uid) {
    :
    :
    :
    :
    :
    print $banner->cache;

    $counter++;
+   } //rseiser: until here was skipped if we found a duplicate banner
  }

I also don't know how to submit or apply a patch properly. My above comment was meant for further review for the developer. If you want to apply it to your code, you might have to do this by hand. Comment out the lines marked with "-", and insert the lines with "+".