[Scummvm-devel] Avoiding loading of all plugins during startup

[Johannes Schickel]

On 06/24/2010 12:12 PM, Max Horn wrote:
> Am 23.06.2010 um 19:50 schrieb Johannes Schickel:
>
>    
>>
>> Sounds fine for a temporary solution to me. I would imagine this could
>> be used to first focus on implementing the plugin loaders and test them
>> (for example on NDS, where we know of serious memory limitations ;-).
>> Though I would be rather against merging this into the mainline later on.
>>      
> Hm, and why?
>    

Basically doing a load over all plugins one after another does not sound 
like a too nice solution to me. For example we might still load all 
plugins in the worst case too (as you already pointed out), which might 
not be too fast on the NDS for example.

If all that turns out not to be a big problem, I'm fine with a merge 
too, but right now I'm rather skeptical there... :-)


>> Sounds fine to me. We might also use that engine key to check whether
>> the plugin for a given target really exists.
>>      
> Of course. And if it doesn't, show an according error message.
>
>    

Sure.


>> I have one question though: what exactly do we store there? The base
>> name of the engine? If so what will we do about engine renaming (like it
>> happened with simon ->  agos in the past)?
>>      
> Foremost, we'd try to avoid it ;) but if it happens, we do the same what we've done with engine renames: Keep a mapping table from old to new engine somewhere. In ScummVM itself, for example. Not beautiful, but simple and effective.
>    

Hehe ok :-).


>>
>> Sounds fine, but what exactly would we want to store there? Currently we
>> store the ScummVM version there plus optionally the SVN revision. This
>> behavior seems too inconsistent to use it directly IMHO. So we would
>> need some proper versioning there too.
>>      
> Exactly. My idea would be that we use a simple integer value, as for savegames in SCUMM, Tinsel, AGI,... which we increment when we feel the need for it. And if we notice that "version" is *not* a simple integer, we can assume that an old ScummVM version last wrote that config file.
>    

Sounds sensible.

>> Apart it seems fine to issue a warning on a newer config file being
>> found. In case the target upgrader really only upgrades the targets (and
>> maybe stuff like the guioptions etc.), but not the game name etc. I'm
>> having no strong feelings against running that one silently in case an
>> too old config file version was found.
>>      
> The upgrader would of course not touch the description, only the gameid/engine/guioptions. Well, we *could* add a way to allow the user to explicitly request an update for the description, too, but again, may intention was to have this at most optionally.
>    

Ok that sounds fine to me then.


>>
>> I am not sure, but actually in my config file all SCI games I have added
>> (KQ4-6) use only the "sci" gameid, on the other hand KQ1-3 are using
>> "agi" as gameid, so I am not sure why you bring up LSL1 as an example?
>> Maybe those games use a different gameid.... or maybe even nowadays
>> there's a different gameid for them...
>>      
> Right, my fault, I forgot that those engines were switched to use a single gameid!  With all its pros and cons... E.g. Pro: Reduces pressure on the gameid namespace, reduces inconsistencies between what the config file claims and what is really there, no more troubles when a gameid changes... Con: Forces the engine to redetect the game each time it is started.
>    

Actually I think most (all?) games using the AdvancedDetector do a full 
detection of the game on startup. Or at least I remember it used to be 
that way...


>> Personally I rather envision the gameid as some hint to the engine, so
>> I'm not really against this, but I can understand that it might be bad
>> for backwards compatibility.
>>      
>
> Yes, we could make the gameid completely optional and only rely on the engine "id". But as you correctly point out,  that would hamper backward compatibility, so I'd not consider this until we've had "engine=" support in at least one major release.
>
> (BTW, maybe the key should be "engineid" for consistency with "gameid" ?
>    

Might be fine for consistency yeah. Though when we really consider 
dropping the gameid I would rather name it "engine_plugin" or the like, 
at least that seems more clear to me than engineid.


>>
>> Sounds nothing like for the long term to me though. I.e. we would
>> require tons of extra tables (1 per engine at least) or the like to map
>> that and thus for every gameid change (may it be updates etc.) we would
>> need to also maintain those. That's rather annoying IMHO and I suspect
>> it would be easier to just split the whole detection into a separate
>> plugin then.
>>      
> Hm, while I am not *that* fond of this approach myself, I think this is not at all that bad. :)
>
> Most engines already have a table of gameids they support, and it would be a simple matter of duplicating that table. Not much work at all. A simple approach to do that: Require all engines to provide a gameid-table.h file of a consistent format. That table can then be shared by the engine plugin, and X. Where X is: a separate plugin with the list of gameids; or the main executable could include them; or a separate tool could turn them into text files (resp. conversely: A separate tool could create the gameid-table.h files from a text file).
> All of these could be implemented in very short time.
>    


Whoops yeah my fault. I forgot that we already have such a list in each 
engine (also of course for the --list-games feature :-).


> This approach is not beautiful, but it's simple, easy to implement, and has very low overhead. And it helps a lot to ease transition to the "engine=" config key system.
>
>    

Actually if we just want that for a transition it seems to me that a 
mapping file sounds easiest to implement. A special plugin, would 
require another plugin type etc. etc. :-). Building into the main 
ScummVM for a transition only might be fine too, at least we don't 
support out of source plugin builds right now anyway, in this case it 
should only be for the transition though.


>>
>> This one kind of destroys the possibility of third party plugins, thus I
>> am really against it. It will also add "useless" memory overhead for all
>> those gameid ->  engine entries IMHO.
>>      
> I agree on your first argument (though I don't consider the memory overhead as such a big problem -- the mini-plugins will take up a lot more memory, too ;). Anyway, I mostly listed it for completeness :).
>    

Yeah but we could unload the mini-plugins on game run unlike the mapping 
in the binary :-).


// Johannes