From 191f6edbe97208a24e23424d62812e38505357f8 Mon Sep 17 00:00:00 2001
From: Daniel Rudolf <github.com@daniel-rudolf.de>
Date: Sat, 13 May 2017 18:17:19 +0200
Subject: [PATCH] Don't pass the $plugins argument of the onPluginsLoaded event
 by reference

This is a BC breaking change!

Manipulating Pico's $plugins array is a really bad idea. We've introduced the Pico::loadPlugin() method to safely load plugins at any time, however, Pico might do unexpected things when loading plugins too late. See the class docs of Pico::loadPlugin() for more details. Nevertheless, this change breaks BC to Pico 1.0. However, I don't know a single plugin that relies on manipulating the $plugins array. If you just want to load a plugin manually, use Pico::loadPlugin() instead.
---
 lib/Pico.php            | 24 ++++++++++++++++++++++--
 plugins/DummyPlugin.php | 17 +++++++++++++++--
 2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/lib/Pico.php b/lib/Pico.php
index 7cb18bd..7d54f9a 100644
--- a/lib/Pico.php
+++ b/lib/Pico.php
@@ -363,7 +363,7 @@ class Pico
         // load plugins
         $this->loadPlugins();
         $this->sortPlugins();
-        $this->triggerEvent('onPluginsLoaded', array(&$this->plugins));
+        $this->triggerEvent('onPluginsLoaded', array($this->plugins));
 
         // load config
         $this->loadConfig();
@@ -548,7 +548,23 @@ class Pico
     /**
      * Manually loads a plugin
      *
-     * Manually loaded plugins must implement {@see PicoPluginInterface}.
+     * Manually loaded plugins MUST implement {@see PicoPluginInterface}. They
+     * are simply appended to the plugins array without any additional checks,
+     * so you might get unexpected results, depending on *when* you're loading
+     * a plugin. You SHOULD NOT load plugins after a event has been triggered
+     * by Pico. In-depth knowledge of Pico's inner workings is strongly advised
+     * otherwise, and you MUST NOT rely on {@see PicoDeprecated} to maintain
+     * backward compatibility in such cases.
+     *
+     * If you e.g. load a plugin after the `onPluginsLoaded` event, Pico
+     * doesn't guarantee the plugin's order ({@see Pico::sortPlugins()}).
+     * Already triggered events won't get triggered on the manually loaded
+     * plugin. Thus you SHOULD load plugins either before {@see Pico::run()}
+     * is called, or via the constructor of another plugin (i.e. the plugin's
+     * `__construct()` method; plugins are instanced in
+     * {@see Pico::loadPlugins()}).
+     *
+     * This method triggers the `onPluginManuallyLoaded` event.
      *
      * @see    Pico::loadPlugins()
      * @see    Pico::getPlugin()
@@ -584,6 +600,10 @@ class Pico
             $this->nativePlugins[$className] = $plugin;
         }
 
+        // trigger onPluginManuallyLoaded event
+        // the event is also called on the newly loaded plugin, allowing you to distinguish manual and auto loading
+        $this->triggerEvent('onPluginManuallyLoaded', array($this->plugins[$className]));
+
         return $plugin;
     }
 
diff --git a/plugins/DummyPlugin.php b/plugins/DummyPlugin.php
index 47d2571..d2e76e8 100644
--- a/plugins/DummyPlugin.php
+++ b/plugins/DummyPlugin.php
@@ -50,10 +50,23 @@ class DummyPlugin extends AbstractPicoPlugin
      *
      * @see    Pico::getPlugin()
      * @see    Pico::getPlugins()
-     * @param  object[] &$plugins loaded plugin instances
+     * @param  object[] $plugins loaded plugin instances
      * @return void
      */
-    public function onPluginsLoaded(array &$plugins)
+    public function onPluginsLoaded(array $plugins)
+    {
+        // your code
+    }
+
+    /**
+     * Triggered when Pico manually loads a plugin
+     *
+     * @see    Pico::getPlugin()
+     * @see    Pico::getPlugins()
+     * @param  object $plugin loaded plugin instance
+     * @return void
+     */
+    public function onPluginManuallyLoaded($plugin)
     {
         // your code
     }