Opened 12 years ago

Closed 12 years ago

Last modified 2 years ago

#8917 closed patch


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


The patch implements a singleton object that the middleware code can use to search/open files (e.g. theme files, sound fonts, plugin detection files).

The common code has been updated to use the new object (except for the cases outlined below):

* Infogrames: only used by GOB. * DXAPlayer: used by AGOS, HE, SWORD1, SWORD2. * ImageDecocer: used by ImageManager. * BaseAnimationState (mpeg player). * AudioStream::openStreamFile().

The patch also removes a large number of unneeded dependencies on 'common/file.h' in the codebase.

Ticket imported from: #2093502. Ticket imported from: patches/1022.

Attachments (3)

defaultsearchmanager.patch (22.9 KB ) - added by peres 12 years ago.
Search manager for middleware.
mt32-archive.patch (1.6 KB ) - added by fingolfin 12 years ago.
searchmanager.patch (2.6 KB ) - added by peres 12 years ago.

Download all attachments as: .zip

Change History (9)

by peres, 12 years ago

Attachment: defaultsearchmanager.patch added

Search manager for middleware.

by fingolfin, 12 years ago

Attachment: mt32-archive.patch added

comment:1 by fingolfin, 12 years ago

Good work already. But of course I have lots of niggly bits to complain about. Consider this as high level criticism, please :)

* The SearchMan itself looks fine to me, maybe you can commit that as a first step.

* For a smooth transition, we should not remove the File::addDir calls from base/main.cpp just now, because this will break lots of things. So, how about keeping those in, then we can apply the rest of your patch in little steps, instead of cooking up one enormous patch, with all the associated problems... :)

* Your change to base/main.cpp, changing the way extrapaths are handled, is a bit flawed:

(1) You see, *each* game target may come with a custom extrapath. But the patch adds extrapath only once, namely before the main loop of scummvm_main() is entered. That will only work for games started from command line. If a game is started from the launcher, it will ignore its extrapath. Worse, if you start game (A) from the command line, then use the RTL feature and start game (B) from the launcher, it will still use the extrapath from game (A). Which can be fatal if those two are variants of a single game, and hence share data files.

One possible solution: Extend the SearchMan API to allow giving added paths an explicit path (like "extrapath_app" and "extrapath_game"), and then add extrapath_game just before the game starts, and remove it again after the game ends.

(2) Next problem with the modified extrapath handling: If the user modifies the extrapath via the config dialog, in game, that change is not reflected.

* With this patch unzip.cpp does not compile. It tries to invoke method open() on a Stream, which of course fails. For now, I just reverted the unzip.cpp changes in order to be able to compile (I started writing a ZipArchive class anyway, so don't bother with that file).

* I get errors compiling sound/softsynth/mt32.cpp. Maybe you didn't enable the MT-32 emu & don't have zlib? :) Attached is a patch with which mt32.cpp compiles correctly (made against HEAD SVN, not against your patch).

* Oh and another minor bit: Maybe you can commit those whitespace fixes separately (like, *now*), that would cut down the patch a bit and makes it easier to find the actual changes *g*.

* As for the other cases you mentioned (and which you emailed me about with more details, but I don't have the email with me): Several of them seem to be easy to resole, but of course requires changes to the engines. And yeah, AudioStream::openStreamFile() is tricky. Let's postpone that for now. What we really should do is this:

(1) Get this patch ready for commit (2) Implement OSXBundleArchive (I'll try to do that tonight or during the weekend) -> with those two ready, we should be able to replicate 100% of the functionality of class File -- well, maybe except for File::exists (we need to check how engines use that, and come up with a plan, e.g. just change it to SearchSet.hasFile calls).

(3) Get back to scummvm-devel, present the whole Archive & SearchSet shebang in detail, ask for some last minute comments. and commit. or maybe commit first, then ask for comments. We need to discuss that on IRC, I guess :). File Added: mt32-archive.patch

by peres, 12 years ago

Attachment: searchmanager.patch added


comment:2 by peres, 12 years ago

Extracted the SearchManager code to a separate patch, to ease evaluation. File Added: searchmanager.patch

comment:3 by fingolfin, 12 years ago

The savemanager has been added. Now we need to start using it... I'll commit a few fixes for it, too.

comment:4 by lordhoto, 12 years ago

Since we accepted that patch and there's no ongoing discussion here, I'll just close this item now.

comment:5 by lordhoto, 12 years ago

Status: newclosed

comment:6 by digitall, 2 years ago

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