Skip to:
Content

Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#3020 closed defect (bug) (no action required)

bp_loaded is fired too early

Reported by: boonebgorges Owned by:
Milestone: 1.5 Priority: major
Severity: Version:
Component: Core Keywords: has-patch dev-feedback
Cc:

Description

After some recent changes to load order, I noticed that the bp_loaded hook was not firing when it should be. It's hooked to plugins_loaded, and so should be loading after BP is loaded (hence the name!), but for some reason it is happening really early on.

For the purposes of the plugin I'm working on, it's not a big deal - I just changed my hook to bp_init, with a priority of 100. But perhaps we should look into whether there's a way to make bp_loaded fire only when BP is actually loaded :)

Attachments (1)

3020.001.diff (1.6 KB) - added by cnorris23 8 years ago.

Download all attachments as: .zip

Change History (11)

#1 @DJPaul
8 years ago

  • Priority changed from normal to major

#2 @SergeyBiryukov
8 years ago

Isn't plugins_loaded fired after all the active plugins are actually loaded?

#3 @boonebgorges
8 years ago

It is, but that doesn't really mean that BP is "loaded" in the sense that a BP plugin developer might expect. For that, you would expect things like bp_setup_globals, bp_setup_nav, etc to already have been run. But because those are hooked to bp_init, which is hooked to init, which is fired *after* plugins_loaded, the order of the hooks seems counterintuitive.

I see from looking at the source for the 1.2 branch that those actions (bp_setup_globals, etc) used to be hooked to bp_loaded, so that something hooked to bp_loaded with priority 10 would load after that stuff. In 1.3, most of these things have been moved to bp_init, which loads later. So maybe we just don't need bp_loaded anymore? I don't know.

I find the whole thing a bit confusing, and would welcome feedback :)

#4 @cnorris23
8 years ago

I believe that bp_loaded now more closely resembles plugins_loaded. In WP, plugins_loaded means that all main plugin files (at minimum are those that have the WP plugin headers), aside from those set to be added via later hooks, have been loaded (or in php speak, included). It doesn't necessarily mean that any code has been executed. The plugins/bp_loaded hooks are equivalent to you "loading" your favorite record onto your turntable. It doesn't necessarily mean that the turntable is on/rotating, or that the needle is down, and the volume up, etc. So, while confusing, bp_loaded is fired at the appropriate time. Hope that helps :)

#5 @boonebgorges
8 years ago

Thanks, cnorris23 :)

That all makes sense, but it still represents a change from bp_loaded's place in the load order in 1.2 vs 1.3. And I (for one) had used the load order of bp_loaded in some of my plugins, as a way of hooking something to a point in the loading process when I knew that BuddyPress would have finished its basic setup routine. In 1.3, the semantics of bp_loaded seems to have changed. So that suggests two things: 1) we should warn plugin authors, and 2) we should think about adding another hook that does what bp_loaded did before, namely signaled the end of BP's setup routine (rather than the end of the include() process).

#6 @cnorris23
8 years ago

  • Keywords has-patch added

No worries. I almost suggested adding another hook that mimicked bp_loaded's previous functionality, but I had already submitted the comment, and I was ready to go out ;) I agree with both 1) and 2). The new bbPress plugin has a bbp_ready hook. I think a bp_ready hook would make a lot of sense. With that in mind, I've made a patch.

@cnorris23
8 years ago

#7 @DJPaul
8 years ago

  • Keywords dev-feedback added

Can we check this please, also if JJJ has any feedback. I don't really think it is a good idea if the load order variable purposes or names change, unless unavoidable. Most plugins had to be updated last time we did that in BP 1.2.5.

#8 @DJPaul
8 years ago

I'm not sure what this ticket is meant to address. So:

branch does add_action( 'plugins_loaded', 'bp_loaded', 20 );
trunk does add_action( 'plugins_loaded', 'bp_loaded', 10 );

I think changing trunk's action priority back to same as branch will break a lot of the load order fixes. Referring to http://buddypress.org/community/groups/creating-extending/forum/topic/plugin-authors-important-buddypress-1-2-5-news-you-need-to-know/ is it not sufficient to just use bp_init? That's what I've been using in my plugins since BP 1.2.5.

#9 @boonebgorges
8 years ago

  • Resolution set to invalid
  • Status changed from new to closed

Agreed. Closing the ticket.

#10 @boonebgorges
8 years ago

Postscript: use bp_init with priority 10 to be sure BP has set itself up.

Note: See TracTickets for help on using tickets.