A New Plugin System

[OrbWeaver]

One of the things that has been keeping me from diving into the Objectives Editor immediately (apart from the current crop of bugs), is the feeling that being a DarkMod-specific component, this should really be a plugin rather than part of the main binary. This would allow alternative "vanilla Doom 3 only" builds of DR to be produced without unnecessary code.

 

Needless to say, the current plugin API would not be adequate for this task, so it might be time to rewrite it from scratch according to our needs. Since we are not using any plugins at present, we should have free reign to do this without breaking anything.

 

Although I have not produced any kind of solid design, my first thought is that a plugin should, on loading, be queried for and return a list of "command objects" which detail all of the functions that the plugin can perform. Each command object would contain the following data:

 

Command destination -- an enum specifying where this command should be made visible: main menu, context menu, toolbar etc. We could even make the group dialog a destination, so that plugins could add extra tabs here (and the entity inspector/media browser could be moved into plugins), but this would require more design because the plugin would have to draw itself rather than just return commands.

Command text -- the textual name of the command, that would be displayed in the menu

Command icon -- the icon for the command, for menu, toolbar or elsewhere

Availability callback -- a functor class (boost::function or other interface) which would return true if the command is currently available, or false if it should be disabled. This would allow, for example, a command in the context menu to grey itself out if an appropriate object was not selected.

Command callback -- a functor class (etc) that would be called to actually execute the command.

 

Once the plugin command was executed, it would operate like any other code with free reign to call API functions on any other module including IRadiant. This would include displaying its own GUI if necessary, since the parent window pointer is available via IRadiant::getMainWindow().

 

Issue: If we allow plugins to add commands to the main menu structure, how should this be specified? Do we put everything in a single "Plugins" menu, or allow plugins to specify a path like "/DarkMod/Objectives..." or "/View/Do something"? Is the latter even possible with the current menu implementation?

 

Any thoughts?

[Springheel]

is the feeling that being a DarkMod-specific component, this should really be a plugin rather than part of the main binary

 

Isn't DarkRadiant supposed to be a DarkMod-specific editor?

[OrbWeaver]

Isn't DarkRadiant supposed to be a DarkMod-specific editor?

 

For our purposes, yes. However there is also a "market" for a better Doom 3 editor, which DarkRadiant also fulfils (or will fulfil in future), and for this reason it is preferable from a design point of view to keep DarkMod-specific functionality separate from the core Doom 3 functionality.

[sparhawk]

Especially when you consider that a plugin system makes sense for such things unless we try to intentionally break compatibillity.

[greebo]

Issue: If we allow plugins to add commands to the main menu structure, how should this be specified? Do we put everything in a single "Plugins" menu, or allow plugins to specify a path like "/DarkMod/Objectives..." or "/View/Do something"? Is the latter even possible with the current menu implementation?

I'd vote for the latter style ("menu/level/Edit _Objectives"). It's not possible with the current system, but I already opened an issue on the bugtracker concerning this. It could work similarly to the preference system (at startup, all the modules register their interest on adding preferences and get asked to do so during the actual preference dialog construction).

 

The modules would inherit from a "MenuConstructor" class (or any other name that fits) and would get asked during the actual construction process (which I planned to be called the "MenuManager"). The MenuManager cycles through the registered MenuConstructors and the result is then baked into the menu. (I don't know if it's possible to add menu commands during runtime, that's why I think the MenuManager will have to do some "preprocessing"/sorting stuff before creating the menu).

[OrbWeaver]

The modules would inherit from a "MenuConstructor" class (or any other name that fits) and would get asked during the actual construction process (which I planned to be called the "MenuManager"). The MenuManager cycles through the registered MenuConstructors and the result is then baked into the menu. (I don't know if it's possible to add menu commands during runtime, that's why I think the MenuManager will have to do some "preprocessing"/sorting stuff before creating the menu).

 

I don't think the plugins themselves should have to concern themselves with how the menu is constructed -- they should only return a string, such as "/DarkMod/Edit _Objectives" to the calling function when they are queried for the command objects. The calling function (presumably some part of the PluginManager) could then do whatever is necessary to tell the menu system to add commands appropriately.

 

You certainly can add menu commands at runtime from a GTK perspective, but how easy that would be to fit into the current DR architecture is a different matter. To be honest I don't like the current hardcoded approach of building the entire menu in one go using references to global commands -- really it should be possible to supply a new command AND a menu item at the same time and have it inserted by the menu manager -- but this would probably involve a rather large amount of refactoring.

[sparhawk]

Will plugins also be allowed to add buttons to a toolbar?

[OrbWeaver]

Will plugins also be allowed to add buttons to a toolbar?

 

Well nothing is decided yet, but in theory there is any number of places a plugin command could go -- main menu, toolbars, context menu etc.

[greebo]

I don't think the plugins themselves should have to concern themselves with how the menu is constructed -- they should only return a string, such as "/DarkMod/Edit _Objectives" to the calling function when they are queried for the command objects. The calling function (presumably some part of the PluginManager) could then do whatever is necessary to tell the menu system to add commands appropriately.

That's what I had in mind, the plugins shouldn't bother with gtk details.

 

Thinking about this - in the case of the menu it could be even more simplified, the plugin/module just has to place a call like

 

GlobalMenuManager().addCommand("/edit/_Objectives", <Callback here>)

in their constructor. The requirement would of course be that the menu system is initialised by the time the module constructor is loaded (which could be taken care of by a GlobalMenuModuleRef or similar).

 

If we wanted a more generalised interface, we could go with an UIConstructor implementation, that sends all the UI-related commands to the GlobalUIManager() at construction time (like addMenuCommand(), addToolbarIcon(), etc.)

This would cover all the possible UI elements, not just the menu.

[OrbWeaver]

Thinking about this - in the case of the menu it could be even more simplified, the plugin/module just has to place a call like

 

GlobalMenuManager().addCommand("/edit/_Objectives", <Callback here>)

in their constructor. The requirement would of course be that the menu system is initialised by the time the module constructor is loaded (which could be taken care of by a GlobalMenuModuleRef or similar).

 

That's an interesting approach actually. Rather than plugins reporting their commands to a plugin manager which adds them to the menu, the menu (and toolbars etc) could be exposed via a module interface and the plugin could then add as many commands as it wanted.

 

That might indeed be a better design, provided such an interface to the menu system could be provided. Essentially the plugins would be totally unmanaged, and just insert their commands themselves like any other part of the main codebase.

[greebo]

I would even do the next step and combine the other UI elements like Toolbar Buttons or GroupDialogWindows into this interface. This way we wouldn't need a GlobalMenu() and GlobalToolbar() and GlobalStatusbar() and and and.

[greebo]

I was thinking a bit more about this and I think that the modules should definitely register their commands in the EventManager before adding menu items, like it's already happening in the current code:

 

GlobalEventManager().addCommand("EditObjectives", <callback>)

 

Afterwards, the command can not only be used in toolbars and menuitems alike, but also assigned a shortcut via the Shortcut Editor as well (without needing to change any algorithms).

 

GlobalUIManager().addMenuItem("/edit/_Edit Objectives", "EditObjectives")
GlobalUIManager().addToolButton("/main/edit", "EditObjectives")

 

The only thing that is missing is the interface to add menu items and toolbar buttons, which should be the responsibility of the GlobalUIManager().

[OrbWeaver]

Quite happy with that -- adding commands and menu items can be separate operations, since they are logically distinct.

[OrbWeaver]

Now that we have the UIManager, I guess I can start with this. Is it safe to say that we are not using the old plugin system at all now? If so, I will scrap the entire thing.

[greebo]

I can't say if anything is using the old plugin system anyhow, but I guess not.

 

Have fun ripping that thing out!

[OrbWeaver]

The new plugin system is in, and the Objectives Editor is now a plugin. A "dm_objectives.so" library is added into the plugins/ folder, and this registers the command and the menu item. If you remove this file, the option simply disappears from the menu with no ill effects, which is exactly how plugins should work.

 

There are a couple of GTK warnings emitted, which are more annoying than anything else. One "Can't set a parent on widget which has a parent" appears at startup, and may have something to do with the UI manager, whereas two "gtk_widget_destroy: assertion `GTK_IS_WIDGET (widget)' failed" errors appear at shutdown, probably related to the order of window destruction.

[sparhawk]

Are there guidlines how to write independent plugins?

[OrbWeaver]

Not as such -- they are not really independent, so much as chunks of Radiant functionality which are offloaded into a DLL rather than linked into the main binary. Writing a plugin is like writing any other Radiant feature, using the existing module interfaces etc, it just gets linked differently during the build process.

 

Think of it more like Linux kernel modules, rather than an actual "plugin" interface with a binary API (like you might get in Photoshop or whatever).

[greebo]

There are a couple of GTK warnings emitted, which are more annoying than anything else. One "Can't set a parent on widget which has a parent" appears at startup, and may have something to do with the UI manager, whereas two "gtk_widget_destroy: assertion `GTK_IS_WIDGET (widget)' failed" errors appear at shutdown, probably related to the order of window destruction.

It may well be that I've missed a case or two in the UIManager code, I'll have a look at this.

[greebo]

The warnings are gone now. And I added a useGlib2() to the sconscript, it refused to compile under windows before.

[greebo]

OrbWeaver, is there already a way to shutdown the loaded plugins? I noticed there is a PluginModulesRef object instantiated in plugin.cpp which loads the plugins, is that correct?

 

It would be nice if the plugins could be shutdown before the XMLRegistry is unloaded, so that the S/R-Editor (or any other UI plugin) could save certain stuff into the Registry, like the window size and position.

[OrbWeaver]

I have no idea actually -- the ModulesRef object will cause all of the modules to be loaded, but I don't think it unloads them at any time. I am not sure if there is anything relating to the order in which plugins are destroyed.

[greebo]

How about adding a virtual void shutdown() = 0 method to the IPlugin interface? This command could be dispatched before all the other modules are unloaded in plugin.cpp.

[OrbWeaver]

I don't think it's the lack of a suitable method that's the problem (you could just as well put the cleanup code in the IPlugin derivative's destructor), it's making sure that all of the plugins are actually destroyed at shutdown. Whether a method is used or the destructor, it will be necessary to iterate over the modules in the list and explicitly destroy each one, which AFAIK does not happen at present.

[greebo]

Yes, I think a loop will be required telling the modules about the forthcoming shutdown. Anyway, at the moment it's not that urgent, but we should keep that in mind. I'll implement a shutdown() method in the S/R Editor class, so that I can link that one up as soon as the possibility is there.