Events on $template->assign*()

General discussion of development ideas and the approaches taken in the 3.x branch of phpBB. The current feature release of phpBB 3 is 3.3/Proteus.
Forum rules
Please do not post support questions regarding installing, updating, or upgrading phpBB 3.3.x. If you need support for phpBB 3.3.x please visit the 3.3.x Support Forum on phpbb.com.

If you have questions regarding writing extensions please post in Extension Writers Discussion to receive proper guidance from our staff and community.
User avatar
Erik Frèrejean
Registered User
Posts: 207
Joined: Thu Oct 25, 2007 2:25 pm
Location: surfnet
Contact:

Events on $template->assign*()

Post by Erik Frèrejean »

Not yet starting an RFC for this because I first want to see some thoughts on this.

It is pretty safe to assume that a lot of the new events will alter variables send to the templates. One approach to handle this is by adding an event just before the data is given to the template object (see core.viewtopic_postrow as example), I want to discuss a system where the actual template methods assign_block_vars, assign_vars and assign_var have an event trigger in them. Utilising such a system will instantly make every template variable available for events before they are set rather then having to bloat the core code with an event call before every template call whom data can be adjusted by an event. At the other end this adds more empty calls to the dispatcher as it is called on every single template call.
Both implementations pose their own problems/limitations and I don't think that one is really better than the other. I personally however think that making the template methods hookable is something that instantly adds a lot of flexibility and keeps the core code a lot cleaner then when adding event calls all over the place.
Available on .com
Support Toolkit developer

User avatar
MichaelC
Development Team
Development Team
Posts: 889
Joined: Thu Jan 28, 2010 6:29 pm

Re: Events on $template->assign*()

Post by MichaelC »

With $template->assign_vars and $template->assign_var you just run the command and it will overwrite the template var in the original var.

Only assign_block_vars presents a problem and i'm not sure how an event in that would work.
Formerly known as Unknown Bliss
psoTFX wrote: I went with Olympus because as I said to the teams ... "It's been one hell of a hill to climb"
No unsolicited PMs please except for quotes.

User avatar
imkingdavid
Registered User
Posts: 1050
Joined: Thu Jul 30, 2009 12:06 pm

Re: Events on $template->assign*()

Post by imkingdavid »

I like the idea to make all assign_* methods hook-able automatically, but how would you be able to differentiate between assign_* method calls?
I do custom MODs. PM for a quote!
View My: MODs | Portfolio
Please do NOT contact for support via PM or email.
Remember, the enemy's gate is down.

User avatar
Erik Frèrejean
Registered User
Posts: 207
Joined: Thu Oct 25, 2007 2:25 pm
Location: surfnet
Contact:

Re: Events on $template->assign*()

Post by Erik Frèrejean »

Well I'm speaking without knowledge of the inner workings of the dispatcher, but you probably have a core.assign_block_vars, core.assign_vars and a core.assign_var event. Then a MOD author must know through which method his data comes in, pretty much like he currently must know which event to use.

However during trying this I found that this isn't possible in the current system, but it also appears that the dispatcher can't actually handle the core.viewtopic_postrow event. Here is an IRC log of an discussion between UKB and myself

Code: Select all

unknownbliss: erikfrerejean: replied
[9:11pm] erikfrerejean: unknownbliss: which problem?
[9:11pm] unknownbliss: assign_vars and assign_var can be overwritten with current hooks. assign_block_vars i have no idea how you'll event that as its inside a loop
[9:11pm] unknownbliss: Well its inside a loop....
[9:12pm] erikfrerejean: So is `core.viewtopic_postrow`
[9:12pm] erikfrerejean: it is however called before the template call, rather than in this case in the template call
[9:13pm] erikfrerejean: only the location of the dispatcher call changes nothing else (or at least should)
[9:14pm] erikfrerejean: I don't think (with my limited understanding of the new template/event systems) there is a difference in there
[9:14pm] unknownbliss: I can't remember off the top of my head
[9:15pm] unknownbliss: and im not sure how to describe it
[9:15pm] erikfrerejean: What is the difference between calling the event the line *before* `$template->assign_block_vars(….)`, or on the first line of that method?
[9:15pm] erikfrerejean: If the loop is an issue then the `core.viewtopic_postrow` event can't work to begin with
[9:16pm] erikfrerejean: Only the call point slightly changes, nothing else
[9:17pm] erikfrerejean: It would be really weird, if that change somehow breaks the event dispatcher
[9:18pm] unknownbliss: it can
[9:19pm] unknownbliss: outside the loop the variable you want to edit is an array and easy to edit
[9:19pm] unknownbliss: once its inside the loop its one element of the array which you can't edit it in the same way
[9:19pm] unknownbliss: so you have to modify the var being put in the loop before its in the loop
[9:20pm] unknownbliss: once its in the loop you can't do anything with it
[9:20pm] unknownbliss: thing if you add something to the array
[9:20pm] unknownbliss: then you also need an even in the array to be able to add it to the assign_block_vars
[9:20pm] unknownbliss: I'm explaining this really badly
[9:21pm] erikfrerejean: What loop? https://github.com/p/phpbb3/blob/8eaa3e003c99efe4173370a475a7b2d2a6df2c5d/phpBB/viewtopic.php#L1586 *is* already in the loop
[9:21pm] unknownbliss: assign_block_vars can't be called twice for the same thing iirc
[9:21pm] erikfrerejean: Remove that event and add an event on the first line of `assign_block_vars` with the same parameters. Then nothing changes in the setup
[9:22pm] erikfrerejean: So either the current behavior is already broken or there is no issue 
[9:31pm] erikfrerejean: I'm not seeing the issue.
[9:31pm] • erikfrerejean goes out and builds an POC
[9:33pm] unknownbliss: ok
[9:33pm] unknownbliss: it may be im just being a nub
[9:44pm] erikfrerejean: unknownbliss: so no it doesn't work, though the normal behavior is broken as well. Altering the event data in the `core.viewtopic_postrow` event throws an "[phpBB Debug] PHP Notice: in file [ROOT]/ext/test/event/test_listener.php on line 16: Indirect modification of overloaded element of phpbb_event_data has no effect"
[9:44pm] erikfrerejean: And adding an event inside that method does the same thing.
[9:44pm] unknownbliss: ah right
[9:44pm] erikfrerejean: So I'm right on the theory that it is the same but obviously the current behavior is broken 
[9:44pm] unknownbliss: feel free to deal with it, I can't at the moment. I've got so much to do and I'm on holiday. :P
[9:45pm] erikfrerejean: Well I can't neither and don't have the time to dig into the magic that is the event dispatcher.
I'll commit POC code in a bit, so someone with a deeper understanding of the system can see whether I messed something up or whether this indeed is a problem within the dispatcher. The events I used can be found in this gist
Last edited by MichaelC on Wed Jun 06, 2012 8:19 pm, edited 3 times in total.
Reason: Add link to gist
Available on .com
Support Toolkit developer

User avatar
imkingdavid
Registered User
Posts: 1050
Joined: Thu Jul 30, 2009 12:06 pm

Re: Events on $template->assign*()

Post by imkingdavid »

If you made the event "core.assign_vars" then your listener would be run on each and every assign_vars() method call, when really you want it to only run, for instance, on viewtopic. You still need uniquely-named events for the listeners to subscribe to.

EDIT: An idea is to add a another (optional) parameter to each of the three assign_* methods, in which you would define the event name. i.e. something like:

Code: Select all

$template->assign_vars(array('BLAH' => true), 'core.viewtopic'); 
And then if no event name is given, it doesn't create an event, so no overhead, but it's easy to add events.
I do custom MODs. PM for a quote!
View My: MODs | Portfolio
Please do NOT contact for support via PM or email.
Remember, the enemy's gate is down.

User avatar
Erik Frèrejean
Registered User
Posts: 207
Joined: Thu Oct 25, 2007 2:25 pm
Location: surfnet
Contact:

Re: Events on $template->assign*()

Post by Erik Frèrejean »

I'm +1 on David his implementation.
Although I do think that all phpBB core template calls should include that event parameter, but yes filtering the calls that way seems the sensible approach.
Available on .com
Support Toolkit developer

User avatar
imkingdavid
Registered User
Posts: 1050
Joined: Thu Jul 30, 2009 12:06 pm

Re: Events on $template->assign*()

Post by imkingdavid »

Just wanted to clarify, for anyone who doesn't understand: a new parameter (i.e. $event_name = '') would be added to the assign_* methods as shown above.

Then, in each of the methods, you would have the following:

Code: Select all

if ($event_name)
{
    $vars = array('whatever_the_variable_name_is_for_the_array_within_the_method_because_I_havent_looked_yet');
    extract($phpbb_dispatcher->trigger_event($event_name, compact($vars)));
}
Note that that would be placed in such a manner that any duplicates from the event listener would overwrite any of the variables in the original array.
I do custom MODs. PM for a quote!
View My: MODs | Portfolio
Please do NOT contact for support via PM or email.
Remember, the enemy's gate is down.

User avatar
imkingdavid
Registered User
Posts: 1050
Joined: Thu Jul 30, 2009 12:06 pm

Re: Events on $template->assign*()

Post by imkingdavid »

Okay, so from phpbb_template class, here is an untested implementation for assign_vars:

Code: Select all

    /**
    * Assign key variable pairs from an array
    *
    * @param array $vararray A hash of variable name => value pairs
    * @param string $event_name The name of the event that has access to this assign_vars call
    */
    public function assign_vars(array $vararray, $event_name = '')
    {
        global $phpbb_dispatcher;
        if ($event_name)
        {
            $vars = array('vararray');
            extract($phpbb_dispatcher->trigger_event($event_name, compact($vars)));
        }

        foreach ($vararray as $key => $val)
        {
            $this->assign_var($key, $val);
        }
    }
As I said, I have not tested this, but in theory it should do what we are wanting, no?
I do custom MODs. PM for a quote!
View My: MODs | Portfolio
Please do NOT contact for support via PM or email.
Remember, the enemy's gate is down.

User avatar
MichaelC
Development Team
Development Team
Posts: 889
Joined: Thu Jan 28, 2010 6:29 pm

Re: Events on $template->assign*()

Post by MichaelC »

imkingdavid wrote:If you made the event "core.assign_vars" then your listener would be run on each and every assign_vars() method call, when really you want it to only run, for instance, on viewtopic. You still need uniquely-named events for the listeners to subscribe to.

EDIT: An idea is to add a another (optional) parameter to each of the three assign_* methods, in which you would define the event name. i.e. something like:

Code: Select all

$template->assign_vars(array('BLAH' => true), 'core.viewtopic');
And then if no event name is given, it doesn't create an event, so no overhead, but it's easy to add events.
The same behaviour can be done by adding an event just afterwards and then overwriting the template vars?

Its not I don't think its a good idea, its just that it can already be done.

As for assign_block_vars, I agree that needs looking into, testing etc.
Formerly known as Unknown Bliss
psoTFX wrote: I went with Olympus because as I said to the teams ... "It's been one hell of a hill to climb"
No unsolicited PMs please except for quotes.

User avatar
Erik Frèrejean
Registered User
Posts: 207
Joined: Thu Oct 25, 2007 2:25 pm
Location: surfnet
Contact:

Re: Events on $template->assign*()

Post by Erik Frèrejean »

imkingdavid wrote:Okay, so from phpbb_template class, here is an untested implementation for assign_vars:

Code: Select all

    /**
    * Assign key variable pairs from an array
    *
    * @param array $vararray A hash of variable name => value pairs
    * @param string $event_name The name of the event that has access to this assign_vars call
    */
    public function assign_vars(array $vararray, $event_name = '')
    {
        global $phpbb_dispatcher;
        if ($event_name)
        {
            $vars = array('vararray');
            extract($phpbb_dispatcher->trigger_event($event_name, compact($vars)));
        }

        foreach ($vararray as $key => $val)
        {
            $this->assign_var($key, $val);
        }
    } 
As I said, I have not tested this, but in theory it should do what we are wanting, no?
Yes :),
Although as posted that causes problems when used in $template->assign_block_vars but that might be a problem with my test event.
Available on .com
Support Toolkit developer

Post Reply