Default arg to "bbp_add_forums_roles()" can cause errors

[naxoc]

The bbp_add_forums_roles() function has a default arg of null for $wp_roles. That causes problems when we try to use it in the loop in the function. This patch adds a check and initializes the $wp_roles var if necessary.

[johnjamesjacoby]

Interesting! 🤔

I have a question about your use case:

when we try to use it in the loop in the function

Are you able to share an example? I'd love to know more about how you're using bbPress roles!

---

History

(When implementing them, I suppose I'd not considered that bbp_add_forums_roles() would be called directly, or outside of anyplace that $wp_roles as a WP_Roles object would not be passed in.)

Usually, bbPress roles get invoked directly from WP_Roles (which is the $wp_roles global):

• $wp_roles->__construct()
• $wp_roles->for_site()
• $wp_roles->init_roles()
• wp_roles_init hook
• bbp_roles_init hook
• bbp_add_forums_roles()

Multisite blog-switching will add these to the beginning of the above stack:

• switch_blog hook
• wp_switch_roles_and_user()

---

Patch notes

Thank you again 🙏 for including a patch with this issue. I've applied & tested it, and (if I understand your use-case) it seems like it does what you (may) need it to, but with 1 caveat worth mentioning...

Calling wp_roles() will trigger the wp_roles_init action, and doing so where you've suggested will cause a single runtime recursion, where bbp_add_forums_roles() gets called from inside of bbp_add_forums_roles() – the call is coming from inside the house! 👻

It is possible to conditionally "juggle" the hook in there to avoid recursing (and that may be necessary here) but typically that is an early indicator there may be a better approach that involves fewer compromises or work-arounds.

Edit: Regardless of anything, you have identified code that we can improve together. bbPress should not attempt to blindly call methods on the $wp_roles global variable without confirming they are callable. 💫

[naxoc]

I have a question about your use case:

when we try to use it in the loop in the function

Yes, sorry. That was not very clear. The loop I meant was the foreach further down in the function where the $wp_roles var is used. Not the WP-loop :)

the call is coming from inside the house! 👻

Woops! I didn't catch that one. Let's find a better solution. I guess it would be a breaking change to remove the default null arg, but how about this patch that just creates a stdClass if we need to?