live627

  • Should five per cent appear too small / Be thankful I don't take it all / 'Cause I'm the taxman, yeah I'm the taxman
  • Posts: 1,670
Plugin hooks
« on November 10th, 2010, 10:51 PM »
And what about the integration hooks SMF has?
A confident man keeps quiet.whereas a frightened man keeps talking, hiding his fear.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Plugin hooks
« Reply #1, on November 10th, 2010, 10:55 PM »
SMF's integration hooks were mostly useless up until 2.0 RC4 wherein they were upgraded to support multiple functions being called at the same point in the execution.

We're keeping most of the existing hooks and adding a lot more of them generally, and combined with SMF's general structure of keeping the current running state in $context (while we'd like to phase that out, doing so is less than trivial), means that you can manipulate an awful lot of things whilst running in the hooks without having to do any editing of the code itself.

So yeah, we're not getting rid of them, we're jugging a few around and adding plenty more.
When we unite against a common enemy that attacks our ethos, it nurtures group solidarity. Trolls are sensational, yes, but we keep everyone honest. | Game Memorial

Nao

  • Dadman with a boy
  • Posts: 16,079
Re: Plugin hooks
« Reply #2, on November 11th, 2010, 09:42 PM »
Quote from Arantor on November 10th, 2010, 10:55 PM
SMF's integration hooks were mostly useless up until 2.0 RC4 wherein they were upgraded to support multiple functions being called at the same point in the execution.
I'm thinking we could be doing things even better...
For instance, create a global local array (if I may say), something like $local_array, which we'd global everytime we write a function that offers hooks.
Now, instead of adding a hook point and specifying the vars that can be shared with it, we'd just add a hook point -- period. Hooks would then automatically be able to access the local array where we put all of the code. ($local_array['this_variable'] for instance.)
At the beginning of the original function, we could add $this_variable = &$local_array['this_variable'] or something. Then we can change $this_variable, and the hook function would still be able to access $this_variable without needing to determine which local vars we'll pass through the hook.

Aaaaand, once again, I'm totally obscure and probably offering to do something that would complicate everything needlessly. :P

Still, most of the SMF calls to call_integration_hook provide only a variable or two, AND they often 'forget' to provide a reference to the variable, instead of a copy of the variable. It not only makes it a bit longer to process -- but it also prevents the mod from tampering with the variable at all. Which can be counter-productive.
Quote
We're keeping most of the existing hooks and adding a lot more of them generally, and combined with SMF's general structure of keeping the current running state in $context (while we'd like to phase that out, doing so is less than trivial),
Yeah, $context could be a place to store most of these vars. We could also delete the vars after they become useless, but I don't know, maybe it'd be a waste of time.
Also, rename $context to $cx or something, to make it faster to type?
Quote
So yeah, we're not getting rid of them, we're jugging a few around and adding plenty more.
And we should/would/will try to be open about hook addition requests. Like, "if you add a hook here, I could do this or that without modding the code". If it seems like something that would benefit more mods, then it'll definitely warrant a mod.

Seriously, I think there are many places that should never have been moddable in the first place -- like, adding menu entries. It should be done exclusively via hooks. We just need to educate modders about it.

PS: maybe this conversation should be split into the private area.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Plugin hooks
« Reply #3, on November 11th, 2010, 11:31 PM »
Quote
For instance, create a global local array (if I may say), something like $local_array, which we'd global everytime we write a function that offers hooks.
That's called $context :P
Quote
Aaaaand, once again, I'm totally obscure and probably offering to do something that would complicate everything needlessly.
It's an interesting idea but not entirely practical, mostly because there's already $context, and partly because hooks are there for a specific purpose, not just a convenient place in time; the actions hook for example is there specifically to be used with modifying the list of actions.

Anything already in $context, $settings, $options, $modSettings etc is already available if the hook wants it, it's only really if the hook needs to send something through to the hooked function that's not a global scope, e.g. the list of actions, or if calling the new-topic one, the details for the new topic and so on.

Really, pushing things to a 'global local' function is bad juju because really one thing I want to do is empty some stuff out of $context rather than fill it up more.
Quote
Yeah, $context could be a place to store most of these vars. We could also delete the vars after they become useless, but I don't know, maybe it'd be a waste of time.
Also, rename $context to $cx or something, to make it faster to type?
Faster, yes, more likely to collide with new local (temporary) variables but no real issue either way. Pruning only helps with saving memory in low memory configurations; it isn't an issue normally.
Quote
And we should/would/will try to be open about hook addition requests. Like, "if you add a hook here, I could do this or that without modding the code". If it seems like something that would benefit more mods, then it'll definitely warrant a mod.
Sure :) Though a fair amount will be anticipation as well, I have a litany of mods in mind that I'm constantly thinking... "how can I make that work without editing the code?" That's really my baseline when I'm thinking of the changes to make.
Quote
Seriously, I think there are many places that should never have been moddable in the first place -- like, adding menu entries. It should be done exclusively via hooks. We just need to educate modders about it.
Yes on both counts.
Quote
PS: maybe this conversation should be split into the private area.
I'm actually fine with it here, because I want to not only explain why things won't work but also the rationale behind the level of changes we're doing and the real benefits of hooks - while I'm actively going to be encouraging hooks, I want people to understand not only the 'how' but the 'why' as well. Remember, those modders who migrate from SMF are going to initially want to reuse what they've done before, and while I'm not planning on pruning the package manager that far, I really don't want to encourage them to continue bad habits. I want them to learn new ones, better ones, ones that will help them grow as developers, knowing as I do that most mod writers are not professional grade programmers.

live627

  • Should five per cent appear too small / Be thankful I don't take it all / 'Cause I'm the taxman, yeah I'm the taxman
  • Posts: 1,670
Re: Plugin hooks
« Reply #4, on November 12th, 2010, 12:00 AM »
Quote from Nao/Gilles on November 11th, 2010, 09:42 PM
Also, rename $context to $cx or something, to make it faster to type?
I'd recommend against it and leave it as a full word so as to be less confusing.

Nao

  • Dadman with a boy
  • Posts: 16,079
Re: Plugin hooks
« Reply #5, on November 12th, 2010, 08:02 AM »
Quote from live627 on November 12th, 2010, 12:00 AM
Quote from Nao/Gilles on November 11th, 2010, 09:42 PM
Also, rename $context to $cx or something, to make it faster to type?
I'd recommend against it and leave it as a full word so as to be less confusing.
Well, we'll probably be renaming other vars, so...
Like, $smcFunc. What does SMC mean? Most people don't know. (And I don't even remember, myself.)
$wedgeFunc is too long and $weFunc may sound like "we fuck", so... $wf?
Might as well use $cx and $wf. And then document the change somewhere.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Plugin hooks
« Reply #6, on November 12th, 2010, 08:58 AM »
SMC = Simple Machines Core, for things that wouldn't necessarily be strictly forum based but usable in other SM apps; it was in the mists of time actually $smfFunc.

There are other things we can do at this point, more on that elsewhere.

Nao

  • Dadman with a boy
  • Posts: 16,079
Re: Plugin hooks
« Reply #7, on November 12th, 2010, 10:16 AM »
Quote from Arantor on November 12th, 2010, 08:58 AM
SMC = Simple Machines Core, for things that wouldn't necessarily be strictly forum based but usable in other SM apps; it was in the mists of time actually $smfFunc.
Back in Beta 2, I think.
Quote
There are other things we can do at this point, more on that elsewhere.
First of all we should really consider renaming global variables...
Shortening them is a solution. $wf, $cx, $opt, $set, $mset or something.

BTW I never understood why $modSettings is named as such... I mean, it's certainly not reserved to mods! It's a bit disturbing, even for a veteran like me.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Plugin hooks
« Reply #8, on November 12th, 2010, 10:20 AM »
Well, I'd love to get stuff out of the global scope generally, there's really not a need for some of the stuff to pollute global scope the way it does.

$modSettings is for modifiable settings, it contains the contents of the admin panel's options that are forum wide (as opposed to $settings which is theme wide and $options which is user-centric) that are modifiable through the admin panel without editing any files, and is the general home of modification settings too.

Nao

  • Dadman with a boy
  • Posts: 16,079
Re: Plugin hooks
« Reply #9, on November 12th, 2010, 10:31 AM »
Quote from Arantor on November 11th, 2010, 11:31 PM
It's an interesting idea but not entirely practical,
Yeah, probably isn't, but I just wanted to discuss that.
Quote
mostly because there's already $context, and partly because hooks are there for a specific purpose, not just a convenient place in time; the actions hook for example is there specifically to be used with modifying the list of actions.
But, see, what if the modder wants to do something in that place, but unrelated to the list of actions?

...

...Granted, it's VERY unlikely they would want to. They'll probably even struggle to use the correct hook in itself, uh.
I think modders can start using hooks, as long as we provide them with a dummy mod that would show the correct ways of doing things. (Or some prominent mods start using hooks and they look into their code.)

And as I said earlier, we really should rename the hook functions, too. Right now, it's a bit too long. Do you remember the exact function to call? Yes you do, but how about everyone else?
add_hook('hook_name', 'my_function') would be better, for instance. Or wedge_add_hook().
Even just adding an alias or something...
Quote
Really, pushing things to a 'global local' function is bad juju because really one thing I want to do is empty some stuff out of $context rather than fill it up more.
Yeah, beginning with $context['user']...
Quote
Faster, yes, more likely to collide with new local (temporary) variables but no real issue either way.
This is the kind of change that would be added to the list for modders and themers to look into, so it wouldn't be a problem, they'd just need to do a mass rename and make sure they're not already using $cx somewhere...
Just an idea though, not something I believe in religiously.
Quote
Sure :) Though a fair amount will be anticipation as well,
I'm not much of a fan of anticipation, not because it's extra work for us, but more likely because if we start adding hooks in useless places, like at the beginning of every single function, it will waste CPU time for everyone, possibly for nothing.
Quote
I have a litany of mods in mind that I'm constantly thinking... "how can I make that work without editing the code?" That's really my baseline when I'm thinking of the changes to make.
And that's commendable!
Quote
I'm actually fine with it here, because
...it shows people how we come up with dozens of new ideas every minute, fail miserably at most of them and think of new ideas to build on the failed ones :P
Quote
I want to not only explain why things won't work but also the rationale behind the level of changes we're doing and the real benefits of hooks - while I'm actively going to be encouraging hooks, I want people to understand not only the 'how' but the 'why' as well. Remember, those modders who migrate from SMF are going to initially want to reuse what they've done before,
Maybe for the most important mods, if there are any, we could offer the modder to convert their mods for them. (Uh, well, what do you know, I did exactly that to get SMF2 versions of most of the mods I'm using on Noisen :P)
Quote
and while I'm not planning on pruning the package manager that far, I really don't want to encourage them to continue bad habits.
I think a good policy would be to add a warning when installing a package. In red. "This addon makes modifications to the Wedge files. Please note that if you ever upgrade Wedge, the add-on will stop working. Also, it may get into conflicts with other modification-type addons. Please consider using a plugin-type addon if this doesn't suit you."
Quote
I want them to learn new ones, better ones, ones that will help them grow as developers, knowing as I do that most mod writers are not professional grade programmers.
We've all been noobs before, yes :)
Quote from Arantor on November 12th, 2010, 10:20 AM
$modSettings is for modifiable settings, it contains the contents of the admin panel's options that are forum wide (as opposed to $settings which is theme wide and $options which is user-centric) that are modifiable through the admin panel without editing any files, and is the general home of modification settings too.
Yes, but it's not very instinctive, see?
'mod', to me, really means 'modification', not 'modifiable'... Another fail of SMF's terminology system.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Plugin hooks
« Reply #10, on November 12th, 2010, 10:45 AM »
Quote
But, see, what if the modder wants to do something in that place, but unrelated to the list of actions?
I did that, actually, in a shoutbox I built; as part of the plugin system SimpleDesk had, I did actually add the same hook that the action handler routine has (though calling SD's hook rather than SMF having it), and abused the hook to not only add my action but also to receive the AJAX calls and handle them, as well as manipulating $context at the same time.

Goes back to what I said before - the running state in $context is fully manipulatable, but really there should be no need to abuse such a hook when a better structure can be put in place.
Quote
I think modders can start using hooks, as long as we provide them with a dummy mod that would show the correct ways of doing things. (Or some prominent mods start using hooks and they look into their code.)
Yes :) This is something I'm quite keen on, not only providing the framework for modders but working with them to build add-ons, so not only providing dummy add-ons but full new features as add-ons that demonstrate how to achieve various things.
Quote
And as I said earlier, we really should rename the hook functions, too.
Well, they were designed for integration which is where the name comes in, but sure, we can rename them to be more appropriate.
Quote
Yeah, beginning with $context['user']...
I still have no idea why that exists when $user_info contains virtually everything you'd want to know anyway.
Quote
I'm not much of a fan of anticipation, not because it's extra work for us, but more likely because if we start adding hooks in useless places, like at the beginning of every single function, it will waste CPU time for everyone, possibly for nothing.
I wasn't going to do it that way, I had a simpler plan - once the action handler was known and the index knew where it was sending the user, I planned to add a hook before and after it has run (since some functions don't return back, and some users will no doubt want to expand upon what's already been done). Yes, there is of course a performance hit there, but the increased flexibility more than makes up for it.
Quote
Maybe for the most important mods, if there are any, we could offer the modder to convert their mods for them.
For the ones that aren't in the core itself, I'll likely put some time aside to sit and implement some of the smaller ones, partly as an example of 'how-to' and partly so we have that functionality available for people. But I'd definitely want to work with mod authors to help them make the jump.
Quote
I think a good policy would be to add a warning when installing a package. In red.
* Arantor likes that idea.
Quote
Yes, but it's not very instinctive, see?
Oh, I'm not disputing that :P I'm well aware of the idiosyncratic nature of SMF's code, just explaining the basis for it...

Nao

  • Dadman with a boy
  • Posts: 16,079
Re: Plugin hooks
« Reply #11, on November 12th, 2010, 10:56 AM »
Quote from Arantor on November 12th, 2010, 10:45 AM
I did that, actually, in a shoutbox I built; as part of the plugin system SimpleDesk had, I did actually add the same hook that the action handler routine has (though calling SD's hook rather than SMF having it), and abused the hook to not only add my action but also to receive the AJAX calls and handle them, as well as manipulating $context at the same time.
I don't understand a word of it, but for instance we could modify said hook to do what SD needs. :P
(You want SD to be ported to Wedge, don't you...? :^^;:)
Quote
Well, they were designed for integration which is where the name comes in, but sure, we can rename them to be more appropriate.
Integration is its purpose, but it shouldn't be its definition. Its definition is a hook.
Imagine that instead of calling parse_bbc(), you had to call parse_bbc_into_an_html_string()
:lol:
Quote
Quote
Yeah, beginning with $context['user']...
I still have no idea why that exists when $user_info contains virtually everything you'd want to know anyway.
Me neither. Probably another example of two vars that developed at pretty much the same time and were favored by one dev or another... Or they wanted to have the "me" equivalent to you's "$context['member']" or something.
Quote
I wasn't going to do it that way, I had a simpler plan - once the action handler was known and the index knew where it was sending the user, I planned to add a hook before and after it has run (since some functions don't return back, and some users will no doubt want to expand upon what's already been done). Yes, there is of course a performance hit there, but the increased flexibility more than makes up for it.
Yeah, adding hooks in the main callee is perfectly fine and I would recommend it, too. Just don't add hooks in every Subs.php function, that's all ;)
Quote
Quote
I think a good policy would be to add a warning when installing a package. In red.
* Arantor likes that idea.
I'm sure most modders will work into making sure their modifications are all turned into hooks at that point.
(It's that's possible, of course. But right now I can't think of anything that isn't.)

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Plugin hooks
« Reply #12, on November 12th, 2010, 11:02 AM »
Quote
You want SD to be ported to Wedge, don't you...?
Well, it's more a rewrite than port given the changes already to the editor component, not to mention the other changes we've already done. More importantly, though, the plugin system would need to be rethought and brought more in line with how RC4 implemented it (which, I have to say, was cleaner than how I did it in some ways)
Quote
Integration is its purpose, but it shouldn't be its definition. Its definition is a hook.
Imagine that instead of calling parse_bbc(), you had to call parse_bbc_into_an_html_string()
Sign me up :D
Quote
Me neither. Probably another example of two vars that developed at pretty much the same time and were favored by one dev or another... Or they wanted to have the "me" equivalent to you's "$context['member']" or something.
$context['member'] actually makes sense for the most part since its main use is in the profile area when you're looking at another user's profile, so it is the contextual information for that user. $context['user'] though overlaps mostly with $user_info for no discernible reason.
Quote
Yeah, adding hooks in the main callee is perfectly fine and I would recommend it, too. Just don't add hooks in every Subs.php function, that's all
That's not anticipation, that would just be stupid :P
Quote
I'm sure most modders will work into making sure their modifications are all turned into hooks at that point.
(It's that's possible, of course. But right now I can't think of anything that isn't.)
A lot of the really little tweaks currently published as mods would be tricky to implement directly because of what they do - rearranging little bits of templates, and realistically, those are not possible to hook simply because of the mess you get into because you'd be adding hooks for every single minor point.

Nao

  • Dadman with a boy
  • Posts: 16,079
Re: Plugin hooks
« Reply #13, on November 12th, 2010, 02:16 PM »
Quote from Arantor on November 12th, 2010, 11:02 AM
$context['member'] actually makes sense for the most part since its main use is in the profile area when you're looking at another user's profile,
Yeah, didn't complain about 'member' but about 'user' instead ;)
Quote
Quote
Yeah, adding hooks in the main callee is perfectly fine and I would recommend it, too. Just don't add hooks in every Subs.php function, that's all
That's not anticipation, that would just be stupid :P
I'm working on hooks right now...
- Changing call_integration_hook to call_hook and same for add_hook and remove_hook
- Changing all of the "integrate_*" strings to just "*". This saves space and the modder shouldn't have to care about colliding into a $modSettings variable or something. I haven't decided whether I'll automatically add integrate_ to the name, or do a 'hooks' array, serialized or something, that will be more logically integrated (ahah) into $modSettings.
- Looking into reference passing. It's a bit confusing... Basically, in PHP5, passing references at call time is deprecated, but since an array of references (of varying length) is passed, there's no other way (and that, is not deprecated, obviously.)
However, sometimes SMF passes vars by reference, sometimes not. It's not always obvious WHY. One would think, security? For instance, integrate_reset_pass doesn't pass by reference... Except for ONE function, resetPassword precisely, which does. Now that makes it complicated, doesn't it..? I don't know why exactly one would want to add a hook to resetPassword, but let's say they do. Then should they expect to be able to modify the original variables? If yes, what for? If not, then why do we pass vars by variable in one function?
Anyway, I'll be committing when I'm done. I'll try to document all of my changes to reference passing.
Quote
A lot of the really little tweaks currently published as mods would be tricky to implement directly because of what they do - rearranging little bits of templates, and realistically, those are not possible to hook simply because of the mess you get into because you'd be adding hooks for every single minor point.
I was thinking... Maybe, maybe, we could allow them to rearrange these little bits of templates at runtime, for instance in ob_sessrewrite...? It'd probably be overkill, though. But do you see what I mean? Doing what SMF would usually do at mod applying time, but this time in real time. "Look for <div id='something'>, and add <hello> before it."
Yeah... Overkill, I know ;) But it's something that could avoid having to uninstall & reinstall when upgrading.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Plugin hooks
« Reply #14, on November 12th, 2010, 02:26 PM »
To a point you're getting into the game of second-guessing the developer intent, and without any secondary information, it's hard to be sure.

Remember, the integration functions were originally for linking SMF to a third party app, so reset_pass makes sense there, because what you're doing is when a user changes their password in SMF, you're flagging a routine in A.N.Other system to update the same user's password there too.

I'd argue in that case, there shouldn't be a need for the value to be byref. Other instances, however, yes it should be a byref, particularly if it seems logical for a third party app to want to modify something before SMF has its wicked way with it.

We're sort of taking that idea and expanding on it, meaning that any original purpose is likely now somewhat irrelevant. For each hook, look at where it is, what local variables are applicable and make a judgement call as to whether it is reasonable and appropriate for third party code (of any kind) to be modifying that. reset_pass is one case it shouldn't be necessary, for example.