Need Feedback on a mod. I'm modifying fetch_template & need advice

[Mutt]

I like the builtin ad template system. It's nice how you can use conditionals to limit which forums or usergroups receive the ads. problem is there isn't a nice way to rotate banners. I made a small edit to handle this but I'd lile to ask you guys if you feel this edit would casue issues.

my plan is simple. just make a new template the same nice as any of the existing ad templates & add a number or other identifier to the end of it. example we already have ad_footer_end so you could make ad_footer_end2, ad_footer_end3, & ad_footer_end4 all with different banner codes.

my edit to function fetch_template looks for templates that begin with "ad_". if it does, it grabs a random template that contains the requested template name so in my example above it would grab and 1 of those 4 versions of ad_footer_end.

for it to work, I have it skip addeing the ad_ templates to the cache. this is why I'm talking to you all. will this cause a problem? My board is very large (20 million page views per month) so I certainly don't want to do something that slows down something as vital as fetch_template.

here's the edit. if you guys think it's OK, I'll certainly release it. been awhile since I released anything but I've been doing my long overdo 3.8 upgrade now & I'm sure I'll be releasing several mods.


in includes/functions.php under function fetch_template I'm changing this

PHP Code:

    if (isset($vbulletin->templatecache["$templatename"]))
    {
        $template = $vbulletin->templatecache["$templatename"];
    }
    else
    {
        DEVDEBUG("Uncached template: $templatename");
        $GLOBALS['_TEMPLATEQUERIES']["$templatename"] = true;

        $fetch_tid = intval($templateassoc["$templatename"]);
        if (!$fetch_tid)
        {
            $gettemp = array('template' => '');
        }
        else
        {
            $gettemp = $vbulletin->db->query_first_slave("
                SELECT template
                FROM " . TABLE_PREFIX . "template
                WHERE templateid = $fetch_tid
            ");
        }
        $template = $gettemp['template'];
        $vbulletin->templatecache["$templatename"] = $template;
    } 


to this

PHP Code:

// Mutt rotating banners
if (substr($templatename, 0, 3)=="ad_") {
        // get random ad template & skip template cache
        $gettemp = $vbulletin->db->query_first_slave("
            SELECT template
            FROM " . TABLE_PREFIX . "template
            WHERE title LIKE '$templatename%'
            ORDER BY Rand() LIMIT 1
        ");
        $template = $gettemp['template'];
} else {
// Mutt rotating banners
    if (isset($vbulletin->templatecache["$templatename"]))
    {
        $template = $vbulletin->templatecache["$templatename"];
    }
    else
    {
        DEVDEBUG("Uncached template: $templatename");
        $GLOBALS['_TEMPLATEQUERIES']["$templatename"] = true;

        $fetch_tid = intval($templateassoc["$templatename"]);
        if (!$fetch_tid)
        {
            $gettemp = array('template' => '');
        }
        else
        {
            $gettemp = $vbulletin->db->query_first_slave("
                SELECT template
                FROM " . TABLE_PREFIX . "template
                WHERE templateid = $fetch_tid
            ");
        }
        $template = $gettemp['template'];
        $vbulletin->templatecache["$templatename"] = $template;
    }
// Mutt rotating banners
}
// Mutt rotating banners 


it only changes how templates are handled if the template name begins with "ad_" but it does run substr every time any template is called.

I considered moving my if inside the existing else right before

PHP Code:

$gettemp = $vbulletin->db->query_first_slave 


so it's only run if the template isn't already cached. with that I wouldn't be worried about how it effect performance but I was worried that the damn templates would somehow get cached and then they wouldn't rotate anymore.

anyway, can I get some feedback? thanks