I believe there are a few items remaining from the OpenID code... Not a lot really. Just a few lines.
Just search for 'authenticate' (single word) and 'auth_method'. Because the authentication method is only 'password' now, the variable is useless.
As always, I would remove it myself but it may serve as a future hook point to plug OpenID and Facebook into the system so I'm leaving up to you to decide whether to remove these or not, Pete.
On another note. I'm looking into loadMemberData() and it reminds me of the discussions about queries and making them hookable.
How about turning the stuff into something like this...?
(I've already transformed all three set types.)
if ($set == 'normal')
{
$select_columns = array(
'IFNULL(lo.log_time, 0) AS is_online', 'IFNULL(a.id_attach, 0) AS id_attach', 'a.filename', 'a.attachment_type',
'mem.signature', 'mem.personal_text', 'mem.location', 'mem.gender', 'mem.avatar', 'mem.id_member', 'mem.member_name',
'mem.real_name', 'mem.email_address', 'mem.hide_email', 'mem.date_registered', 'mem.website_title', 'mem.website_url',
'mem.birthdate', 'mem.member_ip', 'mem.member_ip2', 'mem.posts', 'mem.last_login', 'mem.id_post_group', 'mem.lngfile',
'mem.id_group', 'mem.time_offset', 'mem.show_online', 'mem.media_items', 'mem.media_comments', 'mem.buddy_list', 'mem.warning',
'mg.online_color AS member_group_color', 'IFNULL(mg.group_name, {string:blank}) AS member_group',
'pg.online_color AS post_group_color', 'IFNULL(pg.group_name, {string:blank}) AS post_group', 'mem.is_activated',
'CASE WHEN mem.id_group = 0 OR mg.stars = {string:blank} THEN pg.stars ELSE mg.stars END AS stars'
);
if (!empty($modSettings['titlesEnable']))
$select_columns[] = 'mem.usertitle';
$select_tables = array(
'LEFT JOIN {db_prefix}log_online AS lo ON (lo.id_member = mem.id_member)',
'LEFT JOIN {db_prefix}attachments AS a ON (a.id_member = mem.id_member)',
'LEFT JOIN {db_prefix}membergroups AS pg ON (pg.id_group = mem.id_post_group)',
'LEFT JOIN {db_prefix}membergroups AS mg ON (mg.id_group = mem.id_group)'
);
}Then we just need to add a hook passing $select_columns and $select_tables as parameters and voilà, we have an often modified area that becomes very easy to hook into.
(Obviously, the code is adapted to fit the array format.)
Right after the set building, I added this hook:
call_hook('load_member_data', array($set, &$select_columns, &$select_tables, &$users, &$is_name));
The performance difference is barely noticeable even with benchmarking -- about 5 to 10% slower on a code block that takes less than a millisecond to execute, MySQL call included... So it would make sense to make it easier to modify.
I've added the hook to the ManagePlugins array as well.
Just wanna know if everything's in order... If you don't like it, please tell me so that I revert these changes before my next commit
;)Posted: October 17th, 2011, 11:32 AM
Oh, I'm also considering adding a 'process_member_data' hook AFTER the query, but my main problem is that I don't know WHERE exactly to put it... Right after the query? Right before the final 'return'? Somewhere in between...? Suggestions welcome!
Posted: October 17th, 2011, 11:40 AM
One other thing I'm looking into is loadMemberContext()... It's really heavy.
Moreover, it crams pretty much everything it can into the various arrays, but it's not always used.
For instance, Memberlist calls it for every single name in the list, but it only shows a subset of the data.
Okay, I can live with that... I can live with the multiple calls to censorText and other preg_replaces.
But I have a hard time with the signature parsing.
I'm thinking we should be adding a param to the function, telling Wedge not to bother with the 'heavy' processing and just fill in the blanks. I mean, if you have 50 users in the Memberlist, that means we potentially parse 50 signatures... for nothing!
And while we're at it -- I think it'd make sense to also have a param saying whether we want the member link to use group colors. They're not used in many areas and I'm not sure why. Actually, it could even make sense to always use the group colors. They're loaded at this point... Even in the topic pages it would make sense to show group colors (although I'm not sure about that.)
Posted: October 17th, 2011, 11:56 AM
More things to get rid of...
- $txt['profile_of'] is used in things like $memberContext as a title. I'd rather have it say just 'View profile' than 'View profile of...' followed by a name that is *already* in the link... Totally silly. Plus, we already have $txt['profile_of_username'] which is correctly handled by a more logical sprintf.
Opinions?
- $txt['time_am'] and $txt['time_pm']... They were removed in rev 1071, but they're actually still being used in timeformat()...! They were used twice in the function, and only one occurrence got removed. It doesn't generate an error because it's only being called when $non_twelve_hour is set to true, i.e. the server doesn't support the '%p' parameter in time formatting. I'd suggest simply replacing them with 'am' and 'pm'... (Note, though: %p will usually use AM and PM, in uppercase. I don't know which is the better one. I personally always type it in lowercase, and actually earlier versions of SMF used am and pm in lowercase, too, in timeformat()...)
Posted: October 17th, 2011, 12:15 PM
I've written a more 'accurate' hack for the year removal. Please review it and tell me if that's okay. ($year_shortcut is a static array, while $format is simply =& $user_info['time_format'])
if ($then['year'] == $now['year'])
{
if (!isset($year_shortcut[$format]))
{
if (strpos($format, ', %Y'))
$y = ', %Y';
elseif (strpos($format, ' %Y'))
$y = ' %Y';
elseif (strpos($format, '-%Y'))
$y = '-%Y';
elseif (strpos($format, '%Y-'))
$y = '%Y-';
$year_shortcut[$format] = isset($y) ? $y : '%Y';
}
$show_today = str_replace($year_shortcut[$format], '', $format);
}Posted: October 17th, 2011, 12:21 PM
PS, sorry for the huge post Pete...
:^^;:I'll take a break now.
