[RFC|Merged] Extensions

These requests for comments/change have lead to an implemented feature that has been successfully merged into the 3.1/Ascraeus branch. Everything listed in this forum will be available in phpBB 3.1.
Post Reply
User avatar
PayBas
Registered User
Posts: 305
Joined: Tue Jul 29, 2008 6:08 pm
Contact:

Re: [RFC|Merged] Extensions

Post by PayBas »

leschek wrote:I would like to ask if there is reason why "Extension management" is not on the top of "Customise" tab in ACP? While I'm not 100% sure I think users use it more often than "Style management".
Agreed. It's been brought up before. Please search the forum if there is already an RFC/topic dedicated to this. If not (maybe we only discussed it on IRC), you should make an RFC.

aleha
Registered User
Posts: 143
Joined: Tue Mar 26, 2013 2:19 am

Re: [RFC|Merged] Extensions

Post by aleha »

+1

Just to mention that you can put it on top:

ACP=>SYSTEM(tab)=>(Module Management) Administration Control Panel =>Customise and then click the UP arrow.


User avatar
AmigoJack
Registered User
Posts: 110
Joined: Wed May 04, 2011 7:47 pm
Location: グリーン ヒル ゾーン
Contact:

Re: [RFC|Merged] Extensions

Post by AmigoJack »

Code: Select all

extract($phpbb_dispatcher->trigger_event('core.viewforum_get_topic_data', compact($vars))); 
This call is sub-optimal, and I don't know why this call pattern has evolved. Cons:
  1. extract() is slower than i.e. foreach() $$var= $value
  2. extract() might fail when incorrect data is passed. Documentation comments say it fails for everything, including data which might be processed/parsed correctly.
  3. compact() creates variable copies - why not directly using references? That would also kill the need of extract()
  4. Both functions are always called (including all the work they do), even if they're not needed at all (no event matches to be triggered). Why not having an implementation where an if first checks for trigger matches and only after that all the extract/compact work is done? Considering how many times event trigger checks are encountered (and will surely increase in future releases) this is a questionable waste of overhead.
As a first alternative suggestion I propose an own definition:

Code: Select all

// Checking before executing code which is never needed
if( $phpbb_dispatcher-> does_trigger_event( 'core.viewforum_get_topic_data' ) ) {

    // As before: provide which variables should be accessible
    $aVars= array
    ( 'forum_data'
    , 'sql_array'
    );

    // Can't find an automated way which could provide this:
    // compact() doesn't create references, and an own function
    // would always sit beyond the scope of the variables
    foreach( $aVars as $iKey=> $sName ) {
        $aVars[$sName]=& $$sName;
        unset( $aVars[$iKey] );
    }

    // The previous call to .does_trigger_event stored the event ID
    $phpbb_dispatcher-> trigger_event_last_asked( $aVars );

    // Nothing to do anymore: variables are touched directly
    // without anyone being able to inject array keys or else
} 
Sadly I'm not fond of Symfony right now to exactly point to where one could improve performance on handling events, so checking for a matching event is not bound to passing data anymore.

Post Reply