News:

Want to get involved in developing SMF, then why not lend a hand on our github!

Main Menu

[2.1] Hooks in 2.1

Started by Suki, October 05, 2011, 04:13:04 PM

Previous topic - Next topic

Suki

Hi all, the Devs are considering an overhaul for the existing hooks system

We will like to hear all opinions about this, ways to improve the usage, handling, structure, design, scope, removal from database, etc.

Please share your thoughts on this :)
Disclaimer: unless otherwise stated, all my posts are personal and does not represent any views or opinions held by Simple Machines.

live627

Overhaul how, exactly?

ziycon

LDAP...............................

I know a good few people that would like this authentication option, I was going to write it myself but haven't had the time to look at how to write mod packages for SMF just yet due to life and work getting in the way.

Illori

and how does LDAP apply to the overhaul of hooks?

Suki

Quote from: live627 on October 05, 2011, 05:50:19 PM
Overhaul how, exactly?

in general, in every possible way, in every possible approach.

An example, some of us are thinking in hooks having its own table.
find a better way to handle hooks on uninstall/upgrading
finding a solution on the current system where you load a file everytime.

things like that.
Disclaimer: unless otherwise stated, all my posts are personal and does not represent any views or opinions held by Simple Machines.

ziycon

Quote from: Illori on October 05, 2011, 05:54:55 PM
and how does LDAP apply to the overhaul of hooks?
Maybe I picked it up wrong or it means different things depending what community your in but I would interpret it to be considered the extension of existing functions/functionality to allow new options or features.

Please correct me if I'm wrong in the sense of SMF :)

Illori

we mean the already built in hooks, that mod authors can use when creating mods.

http://wiki.simplemachines.org/smf/Integration_hooks

live627

Quote from: Miss All Sunday on October 05, 2011, 05:56:00 PM
Quote from: live627 on October 05, 2011, 05:50:19 PM
Overhaul how, exactly?

in general, in every possible way, in every possible approach.

An example, some of us are thinking in hooks having its own table.
find a better way to handle hooks on uninstall/upgrading
finding a solution on the current system where you load a file everytime.

things like that.
Oh.... so not only the core hook functions, but also the package manager.


Perhaps, in package-info, blocks could be used for hooks, like

<install>
    <hook point="pre_load">my_plugin</hook>
</install>

Same for uninstall.

I saw Sinan mention he wanted hooks to also call files. I'd say to do it. Then such pseudo-hooks could be removed like admin_include, pre_include,  theme_include.

I don't know about hooks living in their own table. Where they are seems fine to me. They could even be a serialized array instead of CSV which could also help in storing multiple values such as file, class, function.

I remember hearing that Matt put a mod with tons of hooks for the core on the team boards before he left. In anticipation of that, I did not suggest any new hooks (even when Norv asked me to :P)

Suki

Yes there is some ideas as to provide another tag for hooks.

Yes all the hooks SD left are now on 2.1

I will vote for having a separate table for hooks, settings is already too bloated IMO, plus it will make handling hooks a little better.

We also discuss about having an admin page where you can enable/disable or otherwise administrate hooks
Disclaimer: unless otherwise stated, all my posts are personal and does not represent any views or opinions held by Simple Machines.

Bugo

#9
integrate_message (Display.php)
integrate_member_data (Load.php)
integrate_attachments (Display.template.php)
integrate_message_buttons (Display.template.php)

feline

Add a hook in Subs for custom menu functions AFTER the load from cache:

	
call_integration_hook('integrate_dynamic_buttons', array(&$menu_buttons));

	
$context['menu_buttons'] = $menu_buttons;

Bugo

Quote from: feline on October 14, 2011, 07:24:05 AM
Add a hook in Subs for custom menu functions AFTER the load from cache:

	
call_integration_hook('integrate_dynamic_buttons', array(&$menu_buttons));

	
$context['menu_buttons'] = $menu_buttons;

Yes, it'a great idea.

live627

And this (Oh, you ask what this technocrap is. A unified diff!)

Index: Load.php
===================================================================
@@ -945,57 +984,49 @@

if ($set == 'normal')
{
- $select_columns = '
- 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.icq, mem.aim, mem.yim, mem.msn, mem.posts, mem.last_login,
- mem.karma_good, mem.id_post_group, mem.karma_bad, mem.lngfile, mem.id_group, mem.time_offset, mem.show_online,
- mem.buddy_list, mg.online_color AS member_group_color, IFNULL(mg.group_name, {string:blank_string}) AS member_group,
- pg.online_color AS post_group_color, IFNULL(pg.group_name, {string:blank_string}) AS post_group, mem.is_activated, mem.warning,
- CASE WHEN mem.id_group = 0 OR mg.stars = {string:blank_string} THEN pg.stars ELSE mg.stars END AS stars' . (!empty($modSettings['titlesEnable']) ? ',
- mem.usertitle' : '');
- $select_tables = '
- 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)';
+ $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.icq', 'mem.aim', 'mem.yim', 'mem.msn', 'mem.posts', 'mem.last_login', 'mem.karma_good', 'mem.id_post_group', 'mem.karma_bad', 'mem.lngfile', 'mem.id_group', 'mem.time_offset', 'mem.show_online', 'mem.buddy_list', 'mg.online_color AS member_group_color', 'IFNULL(mg.group_name, {string:blank_string}) AS member_group', 'pg.online_color AS post_group_color', 'IFNULL(pg.group_name, {string:blank_string}) AS post_group', 'mem.is_activated', 'mem.warning', 'CASE WHEN mem.id_group = 0 OR mg.stars = {string:blank_string} 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)',
+ );
}
elseif ($set == 'profile')
{
- $select_columns = '
- 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.openid_uri, mem.birthdate, mem.icq, mem.aim, mem.yim, mem.msn, mem.posts, mem.last_login, mem.karma_good,
- mem.karma_bad, mem.member_ip, mem.member_ip2, mem.lngfile, mem.id_group, mem.id_theme, mem.buddy_list,
- mem.pm_ignore_list, mem.pm_email_notify, mem.pm_receive_from, mem.time_offset' . (!empty($modSettings['titlesEnable']) ? ', mem.usertitle' : '') . ',
- mem.time_format, mem.secret_question, mem.is_activated, mem.additional_groups, mem.smiley_set, mem.show_online,
- mem.total_time_logged_in, mem.id_post_group, mem.notify_announcements, mem.notify_regularity, mem.notify_send_body,
- mem.notify_types, lo.url, mg.online_color AS member_group_color, IFNULL(mg.group_name, {string:blank_string}) AS member_group,
- pg.online_color AS post_group_color, IFNULL(pg.group_name, {string:blank_string}) AS post_group, mem.ignore_boards, mem.warning,
- CASE WHEN mem.id_group = 0 OR mg.stars = {string:blank_string} THEN pg.stars ELSE mg.stars END AS stars, mem.password_salt, mem.pm_prefs';
- $select_tables = '
- 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)';
+ $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.openid_uri', 'mem.birthdate', 'mem.icq', 'mem.aim', 'mem.yim', 'mem.msn', 'mem.posts', 'mem.last_login', 'mem.karma_good', 'mem.karma_bad', 'mem.member_ip', 'mem.member_ip2', 'mem.lngfile', 'mem.id_group', 'mem.id_theme', 'mem.buddy_list', 'mem.pm_ignore_list', 'mem.pm_email_notify', 'mem.pm_receive_from', 'mem.time_offset', 'mem.time_format', 'mem.secret_question', 'mem.is_activated', 'mem.additional_groups', 'mem.smiley_set', 'mem.show_online', 'mem.total_time_logged_in', 'mem.id_post_group', 'mem.notify_announcements', 'mem.notify_regularity', 'mem.notify_send_body', 'mem.notify_types', 'lo.url', 'mg.online_color AS member_group_color', 'IFNULL(mg.group_name, {string:blank_string}) AS member_group', 'pg.online_color AS post_group_color', 'IFNULL(pg.group_name, {string:blank_string}) AS post_group', 'mem.ignore_boards', 'mem.warning', 'CASE WHEN mem.id_group = 0 OR mg.stars = {string:blank_string} THEN pg.stars ELSE mg.stars END AS stars', 'mem.password_salt', 'mem.pm_prefs',
+ );
+ 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)',
+ );
}
elseif ($set == 'minimal')
{
- $select_columns = '
- mem.id_member, mem.member_name, mem.real_name, mem.email_address, mem.hide_email, mem.date_registered,
- mem.posts, mem.last_login, mem.member_ip, mem.member_ip2, mem.lngfile, mem.id_group';
- $select_tables = '';
+ $select_columns = array(
+ 'mem.id_member', 'mem.member_name', 'mem.real_name', 'mem.email_address', 'mem.hide_email', 'mem.date_registered', 'mem.posts', 'mem.last_login', 'mem.member_ip', 'mem.member_ip2', 'mem.lngfile', 'mem.id_group',
+ );
+ $select_tables = array();
}
else
trigger_error('loadMemberData(): Invalid member data set \'' . $set . '\'', E_USER_WARNING);

+ call_hook('load_member_context', array(&$set, $users, &$select_columns, &$select_tables));
+
if (!empty($users))
{
// Load the member's data.
$request = $smcFunc['db_query']('', '
- SELECT' . $select_columns . '
+ SELECT ' . implode(', ', $select_columns) . '
FROM {db_prefix}members AS mem' . $select_tables . '
WHERE mem.' . ($is_name ? 'member_name' : 'id_member') . (count($users) == 1 ? ' = {' . ($is_name ? 'string' : 'int') . ':users}' : ' IN ({' . ($is_name ? 'array_string' : 'array_int') . ':users})'),
array(
@@ -1268,6 +1299,7 @@
}
}

+ call_hook('load_member_context', array(&$memberContext[$user]));
return true;
}

@@ -1319,6 +1351,8 @@
// Robots shouldn't be logging in or registering.  So, they aren't a bot.  Better to be wrong than sorry (or people won't be able to log in!), anyway.
if ((isset($_REQUEST['action']) && in_array($_REQUEST['action'], array('login', 'login2', 'register'))) || !$user_info['is_guest'])
$context['browser']['possibly_robot'] = false;
+
+ call_hook('detect_browser', array(&$context['browser']));
}

// Load a theme, by ID.

Suki

See, this is what I was talking about here  ;)
Disclaimer: unless otherwise stated, all my posts are personal and does not represent any views or opinions held by Simple Machines.

Suki

oh! and magically they were merged :P
Disclaimer: unless otherwise stated, all my posts are personal and does not represent any views or opinions held by Simple Machines.

Matthew K.

No need for Menu Hooks if Menu Editor is implemented. Mods can then simply add their buttons to the database, and then reload the menu cache file.
Quote from: feline on October 14, 2011, 07:24:05 AM
Add a hook in Subs for custom menu functions AFTER the load from cache:

	
call_integration_hook('integrate_dynamic_buttons', array(&$menu_buttons));

	
$context['menu_buttons'] = $menu_buttons;


I agree that the hooks system should be rethought out. Definitely it's own table, that's just the smart thing to do, in my opinion.

live627

Quote from: Labradoodle-360 on October 19, 2011, 01:24:55 PM
Definitely it's own table, that's just the smart thing to do, in my opinion.
Why?


Consider: Which is more important: performance, or elegance?

4Kstore

For me the use of the hook will exist if there is good documentation on them, so that has to be good translations and good examples of their use.
the hooks are a good tool but you have to use and above all we must teach use this tool

Quote from: Bugo on October 14, 2011, 05:48:51 AM
integrate_message (Display.php)
integrate_member_data (Load.php)
integrate_attachments (Display.template.php)
integrate_message_buttons (Display.template.php)


x2 agree

¡¡NEW MOD: Sparkles User Names!!!

KensonPlays

Quote from: live627 on October 19, 2011, 08:14:13 PM
Quote from: Labradoodle-360 on October 19, 2011, 01:24:55 PM
Definitely it's own table, that's just the smart thing to do, in my opinion.
Why?


Consider: Which is more important: performance, or elegance?
Performance, then elegance. And yes hooks (more) would be nice.

live627

Quote from: Kcmartz on October 23, 2011, 03:50:26 AM
Performance, then elegance. And yes hooks (more) would be nice.
Exactly! ESPECIALLY when bootstrapping.

Advertisement: