linuxdcpp-team team mailing list archive

[Steven Sheehy <587597@xxxxxxxxxxxxxxxxxx>]

Looks like most of the things I mentioned are fixed. I still have some
major issues with it though.

- Can you explain why the calls to the PluginManager will return if they
are false? For example:
if(PluginManager::getInstance()->onIncomingConnectionData(this, aLine))
return; in dcpp/UserConnection.cpp. I would think that regardless of
what happens in the call to the plugin that you want to continue the
flow and setup the connection. If a plugin fails that should not impact
the main program. Am I missing something?

- What safeguards are there that a plugin won't cause the core to enter
an unsafe state or crash? As far as I can tell, plugin callbacks are
executed within the same core thread that initiated it so it's possible
that the plugin can cause a deadlock or other resource contention.

- The plugin architecture does not seem to be extensible. If a program
using the dc++ core (like say perhaps LinuxDC++) and wants to add a
custom event that may or may not be applicable to DC++ there is no way
to dynamically add it without modifying the core. I would think this
needs to be designed to be extensible through inheritance or other
means.

- The plugin notification system is synchronous and procedure based. All
throughout the core we see mentions of the plugin system via
PluginManager::getInstance()->onFooEvent() calls. This strongly couples
the core classes with the plugin system. It is not the responsibility of
these classes to deal with plugins and thus they should not have any
mention of them. I strongly feel that it should be an asynchronous event
driven architecture. There is already a precedent for an event driven
architecture since that is how all the listeners are utilized. For
example, instead of calling PluginManager::getInstance()->onFooEvent()
inside a core class, it should fire(FooListener::FooEvent(), args); and
the PluginManager should subscribe to that event. Where possible the
PluginManager should listen to existing core events and if no event
exists then they should be added to an applicable listener. This hides
the implementation of the plugin system from the rest of the core and
would allow us to modify the plugin system without affecting the others.
This abstraction would be useful, for example, if in the future we want
to put plugin events into a queue and process on a separate thread.

- subscribtion -> subscription

- This big of a patch should've really been a bzr branch so we could
track the changes and review the code before merging.

- Has Jacek reviewed this? This a major core change and I don't see much
discussion on it here (besides my ramblings). I see major issues with
the patch as documented above and would like to get his view on the
patch.

-- 
Plugins support
https://bugs.launchpad.net/bugs/587597
You received this bug notification because you are a member of
Dcplusplus-team, which is subscribed to DC++.

Status in DC++: New

Bug description:
Ok, I'll leave out the sales pitch... here is a potential patch for adding support for plugins to DC++, compiles cleanly (ie. shouldn't generate any warnings) under mingw and visual studio of course and has been tested and working.

The code itself has been in ApexDC++ for a while so it has gotten real life use as well, of course migrating it to clean DC++ needed a few changes here and there but nothing major.

Should also compile and work on linux as it is... but this I have not had chance to test at all so that is just on paper for now.

The major difference between this patch and what's in ApexDC++ is that it does not include a full settings page for plugins which ApexDC++ has. This is simply because I don't know a squat about DWT. Instead this patch has set of chat commands added, so you can play around with the plugins, but if this gets accepted then I certainly hope someone is willing to invest into a settings page for usability sake.

Oh and few changes to the code come directly from bcdcpp, you'll spot them I am sure :).

As for the plugins currently three exists.. a pure C sample that (you guessed it) really doesn't do anything productive and then a plugin version of bcdcpp's lua (this one is pretty direct port, so it is C++).

The third one is not so impressive... it adds support for various media player chat announces (spam is too negative of a word), although primarily I created it to proof that it is possible to make a plugin that modifies the GUI, even though the API itself has little to no support for such plugin (on windows anyways),

The first two plugins can be compiled with both mingw and visual studio and are also hopefully linux friendly, the third not so much.

Oh and you can mix plugins... so mingw compiled dcpp will cope just fine with visual studio compiled plugins or vice versa (obviously this also means that the stl's used can be different). In theory you should also be able to create plugins with languages such as VB and Delphi but this very much theoretical.

Well do what you want with it... it's posted now.