Opened 16 years ago

Closed 16 years ago

Last modified 2 years ago

#8397 closed patch

Experimental plugin code revision

Reported by: fingolfin Owned by: fingolfin
Priority: normal Component: --Other--
Keywords: Cc:
Game:

Description

The attached patch modifies the way plugins (both static and dynamic are detected):

1) Dynamic plugins now are built in a "plugins/" subdirectory; ScummVM doesn't hard code a plugin file/path list anymore, rather, all files in the "plugins/" dir are checked; if the filename has the right prefix & suffix, we try to load it

2) Static plugins use an old C++ trick to "auto-register". This is potentially troublesome, if your C++ compiler / linker is odd.

It would be nice if this could be tested on non-OSX systems, in particular: * on Linux using GC 3.3, 3.2 * on Windows using mingw * on Windows using MS VC++ * on Dreamcast, which has a custom plugin system * on anything else, too, of course :-)

For a proper test, you should compile & run ScummVM once in a normal build, and once with plugins enabled (the latter is only supported on Linux, FreeBSD, OSX and Dreamcast at this time).

Ticket imported from: #1117443. Ticket imported from: patches/502.

Attachments (4)

plugin.patch (6.8 KB ) - added by fingolfin 16 years ago.
plugin2.patch (7.4 KB ) - added by fingolfin 16 years ago.
Cleaned up patch
plugin3.patch (12.5 KB ) - added by fingolfin 16 years ago.
Revised patch
plugin4.patch (12.6 KB ) - added by fingolfin 16 years ago.
4th version

Download all attachments as: .zip

Change History (21)

by fingolfin, 16 years ago

Attachment: plugin.patch added

comment:1 by fingolfin, 16 years ago

Owner: zeldin removed

comment:2 by sev-, 16 years ago

Can we avoid use of hardcoded plugin names completely? We can use several approaches here.

o Have plugins in some text file o Though better way would be to add generic function for each plugin something like "char *pluginName()" and query each so library/DLL in specified directory for that function.

comment:3 by sev-, 16 years ago

FreeBSD: --enable-plugins. Works ok. Loads plugins from plugins/

But with statically built plugins I still get reduced size binary without ability to run any plugin.

comment:4 by fingolfin, 16 years ago

If you read my patch, -sev, the point exactly is to get rid of the hard coded plugin names. It already does so -- the list of plugin names in plugin.cpp is insides an #if 0 block, and the list in plugins.h I simply forgot to remove -- it is not used at all.

As for the static build not working, this seems to be a problem with GNU ld, see also the discussion I had with wjp on IRC today.

by fingolfin, 16 years ago

Attachment: plugin2.patch added

Cleaned up patch

comment:5 by fingolfin, 16 years ago

Slightly revised patch, removes some left over cruft to avoid confusion.

comment:6 by Kirben, 16 years ago

Just tried a normal build under mingw using patch (plugin2.patch), compiled without problems and linked fine. But I ended up with a small ScummVM (0.99MB), without any usable plugins. I just get an 'is an invalid target' for any target I tried to use in ScummVM.

by fingolfin, 16 years ago

Attachment: plugin3.patch added

Revised patch

comment:7 by fingolfin, 16 years ago

Here is a revised version of the patch. While it isn't as clean anymore (plugins.cpp again contains an explicit reference to each static plugin), it still is an improvement over the old code, I think. Esp. for the "dynamic plugins" build...

comment:8 by Kirben, 16 years ago

Just tried a normal build under mingw using updated patch (plugin3.patch), but have exact same issue as before. A ScummVM, without any usable plugins.

comment:9 by fingolfin, 16 years ago

That's very strange, Kirben. Can you use a debugger to step through PluginManager::loadPlugins(); in particular, where it invokes LINK_PLUGIN. After each of these statements, what value does plugin have?

Only explanation I can think of is that your compiler tries to be overly clever (maybe due to an "-O3" or something similar) and optimizes the access to the global variables away. That would be trivial to fix, though.

comment:10 by Kirben, 16 years ago

You are right, the patch works fine, if I disable the optimisation in the Makefile.

comment:11 by Kirben, 16 years ago

I tried patch using each optimization level and it fails at all levels (Even level 1)..

comment:12 by fingolfin, 16 years ago

Here's another try, slightly modified, to avoid the optimization issue (I hope).

by fingolfin, 16 years ago

Attachment: plugin4.patch added

4th version

comment:13 by Kirben, 16 years ago

I tried a normal build under mingw using updated patch (plugin4.patch) and it works fine now.

comment:14 by sev-, 16 years ago

Works perfect in both modes under FreeBSD. I think it's ready for commit. Only one thing. It seems that it is not DISABLE_ENGINEXXX-aware.

comment:15 by fingolfin, 16 years ago

You are right Eugene. I fixed that and did some cleanup, and checked the code into CVS.

comment:16 by fingolfin, 16 years ago

Owner: set to fingolfin
Status: newclosed

comment:17 by digitall, 2 years ago

Component: --Other--
Note: See TracTickets for help on using tickets.