MyBB Hacks

• The check on whether the plugin is installed is a little pointless, due to the fact that the language load call will fail if that's the case
  
  PHP Code:

  ougc_awards_lang_load(); $awards = $cache->read('ougc_awards'); // If plugin no active or user has no useid then stop. if(!$mybb->user['uid'] || $mybb->settings['ougc_awards_power'] != 1 || !function_exists('ougc_awards_is_installed') || !is_array($awards) || !$awards) { error_no_permission(); }

  ---

  I suggest putting both that and the cache load after the check.

• In-loop query used here - consider using a JOIN in the query
  
  PHP Code:

  $users = $db->simple_select('ougc_awards_users', '*', "aid IN ({$award['aid']})", array('order_by' => 'date', 'order_dir' => 'desc')); while($gived = $db->fetch_array($users)) { $trow = alt_trow(); $user = get_user($gived['uid']);

  ---

• PHP Code:

  $db->write_query("CREATE TABLE `".TABLE_PREFIX."ougc_awards_users` ( `gid` bigint(30) UNSIGNED NOT NULL AUTO_INCREMENT, `uid` bigint(30) NOT NULL DEFAULT '0', `aid` bigint(30) NOT NULL DEFAULT '0', `reason` varchar(255) NOT NULL DEFAULT '', `date` int(10) NOT NULL DEFAULT '0', PRIMARY KEY (`gid`) ) ENGINE=MyISAM;" );

  ---

  I'd consider putting a unique key over (uid,aid) as it seems that it should be.  Perhaps also consider sticking indexes on uid as well as aid.

• ougc_awards_lang_load is sometimes called multiple times on a single page load.  Perhaps consider restricting this to one call per page.
• Over-sanitisation used here (and possibly in other places):
  
  PHP Code:

  if($mybb->input['reason']) { $mybb->input['reason'] = htmlspecialchars_uni($mybb->input['reason']); } if($mybb->request_method == 'post') { ... ougc_awards_give_award($award, $user['uid'], $mybb->input['reason']);

  ---

  (a htmlspecialchar'd version of the reason is being saved to the DB)

• PHP Code:

  while($award = $db->fetch_array($q1)) { $q2 = $db->simple_select('ougc_awards_users', 'uid', "aid='{$award['aid']}'"); $users = array(); while($user = $db->fetch_array($q2)) { $users[] = intval($user['uid']); } $db->free_result($q2); $update = implode(',', $users); ougc_awards_update_award($award['aid'], array('users' => $update)); }

  ---

  ...noting that the users column that is being saved to is defined as varchar(100) - ie, a large amount of users will easily overflow the column.  I recommend rethinking this behaviour altogether - do you really want to potentially cache such a huge amount of data?
  

(04-22-2012 09:04 AM)Sama34 Wrote: [ -> ]And do you really thing it would bring more problems if I cache the users? I did so to void postbit queries at at ll.

Everybody seems to be afraid of plugins that make any extra query by post at postbit.

I also though about adding a user column to save users awards there instead, but to me it looks worse since I use others plugins that do something similar (newpoints, myachievements, etc..).

function postbit_hook_function($post) {

  static $awards_cache = null;
  if(!isset($awards_cache[$post['uid']])) {
    global $pids, $db;
    isset($awards_cache) or $awards_cache = array();
    $awards_cache[$post['uid']] = array();
    if(THIS_SCRIPT == 'showthread.php' && $pids) {
      // cache awards here (I'm unsure of this query, cbf checking)
      $query = $db->query('SELECT DISTINCT a.* FROM '.TABLE_PREFIX.'posts p LEFT JOIN '.TABLE_PREFIX.'awards a ON p.uid=a.uid WHERE p.'.$pids);
      while($award = $db->fetch_array($query)) {
        $awards_cache[$award['uid']][$award['aid']] = $award;
      }
      $db->free_result($query);
    } else {
      $query = $db->simple_select('awards', '*', 'uid='.$post['uid']);
      $award = $db->fetch_array($query);
      $awards_cache[$post['uid']] = array($award['aid'] => $award);
    }
  }
  
  // awards now can be referenced through $awards_cache[$post['uid']]
}

Or just cache on the users table, as one could assume that there won't be too many awards.  If you're afraid of conflicting with another plugin, just ensure that your column name is unique.