#8795 closed patch
Plugins versioning
| Reported by: | jvprat | Owned by: | fingolfin |
|---|---|---|---|
| Priority: | normal | Component: | --Other-- |
| Version: | Keywords: | ||
| Cc: | Game: |
Description
I've been following the move to more general plugins with interest, but since there was just a high level roadmap on #scummvm I'm not sure how to help.
It looks like one of the open issues is versioning. The attached patch is a proposal to handle general plugin API version, plugin types and plugin types API version.
It adds a "type" parameter to the REGISTER_PLUGIN macro (currently it's just used to match dynamic plugins API versions).
I think having a generic PLUGIN_alloc function that returns a void pointer to the "plugin meta object" could be enough, since the generic plugin class would know how to cast it depending on the plugin type. With this, the general plugin API could be very stable, even when adding new plugin types, and all plugin types would have the same interface.
But as I said, I'm not sure how you envision the implementation details, so I don't know if it fits your approach. Let me know what can I do to help with this.
Ticket imported from: #1885986. Ticket imported from: patches/900.
Attachments (1)
Change History (6)
by , 17 years ago
| Attachment: | plugins_version.patch added |
|---|
comment:1 by , 17 years ago
comment:2 by , 17 years ago
* For the engine API version, using ScummVM's version as a base makes sense. Maybe MAJOR and MINOR (and VERSION_DEVELOPMENT_BUILD) would be enough? If you accept linking with other PATCH releases, you wouldn't need to keep track of it, right?
* Having the version and other vital information inside PluginObject makes me wonder what would happen in case of a change in that base class. Wouldn't it break the binary compatibility? And maybe you wouldn't even be able to query the version. Instead of this, having the basic information (plugin type and version) in separate functions returning plain integers should be compatible forever. I didn't consider the overhead of creating the plugin object, but the API stability.
* The management of different types of plugins was one of the dark areas for me. Having a different manager for each kind of engine would simplify it a lot.
comment:3 by , 17 years ago
I've committed an updated version of the patch. There are some things left to do, like having an automatic version for engine plugins, or having several plugin managers, but that will be done later.
comment:4 by , 17 years ago
| Status: | new → closed |
|---|
comment:5 by , 6 years ago
| Component: | → --Other-- |
|---|
Hi Jordi, great to know you are interested in working on this, too. I am sure this will speed up things a lot, as I myself am rather busy. So I'll appreciate whatever work you do on this.
Some specific remarks to the patch and the general idea:
* Yeah, allowing different plugin "types" is a goal.
* Versioning: I thought a lot about this. It would be important as long as we want to allow 3rd party binary plugin distributions, *but* we'd have to be very careful about what constitutes an "API" change. For Engines, this is very problemtic, because they make heavy use of backlinking to the core code. So any change to the code in common/, sound/, etc, is likely to create a binary incompatibility. So for engines, we can't really hope for a reliable version flag. However, we could simply use the ScummVM *version* as a flag: Let the version be (MAJOR<<16 | MINOR<<8 | PATCH) for plugins in release builds. In SVN builds, "OR" in VERSION_DEVELOPMENT_BUILD = 1<<31 or so, just to mark the binary as "dangerous". Then, for release builds, allow only loading of binaries with matching plugin version. Optionally, for PATCH releases, also support linking against all versions between MAJOR.MINOR.0 and MAJOR.MINOR.PATCH. Does that make sense?
For scaler plugins, we probably could get away with a manually updated API version, as they shouldn't have (m)any reasons for backlinking, if designed well.
* Binary API for type/version: Actually, my original idea was a bit different from what your patch does (but I am not married to it; I simply state it here for consideration, so we can discuss pros/cons). Namely, I wanted to have a single "PluginObject *PLUGIN_getObject()" method, which returns, well, an instance of a PluginObject subclass. PluginObject would simply have getAPIVersion and getType methods (well and maybe also getName and getCopyright?). MetaEngine would be a subclass of it. So, this seems to match precisely with your idea of a PLUGIN_alloc() method. Good ;-).
* Once we have multiple plugin types, of course it wouldn't be appropriate to have type specific methods (like "findGame"). So, the plugin manager would need to be adjusted to "sort" plugins by type and provide specific APIs for them. Or (possibly better ?), have seperate EngineManager / ScalerManager singletons which manage the resp. engine types.\ Note that the unloadAllPluginsExcept() would have to be adapted / moved to EngineManager anyway, since we certainly don't want to unload all scalers when launching a game, only all other engines ;-).
* The advantage of being able to query the API version / type of a plugin w/o instantiating an object: less overhead if we need code which only loads all plugins of a certain type. OTOH, the PluginObject classes should be very low overhead anyway, so I am not sure this would be an important optimization...