[Scummvm-devel] Augmented target name

[Max Horn]

Am Mo, 7.04.2008, 14:42, schrieb Johannes Schickel:
> On Monday 07 April 2008 00:07:10 Max Horn wrote:
>> Hi folks,
>>
>> [...]
>> Yes, that's it. :-)
>>
>> My stance on this is:
>>
>> * This is a SILLY debate :-). I am sorry for having started it, but I
>> think it's my responsibility now to handle it. Sorry.
>
> Agreed.
>
>> * I agree that the "monkey-vga-xmas-it" is not god given, and IMO it
>> has no strict advantage of the others -- it's just a pure matter of
>> taste.
>
> Agreed.
>
>> * In fact, I believe that *any* of the three is far superior to non-
>> augmented target names. Augmenting helps power users, and doesn't
>> hurt novices.
>
> Agreed.
>
>> * Making this changeable via an option is a total NO. This is not a
>> solution, it's chickening out
>
> Fine with me too.

OK, I am glad we agree so far *g*

>
>> One reason given *against* changing kyra to use the "augmented
>> targets" was that it would break savestates when re-adding games.
>> True. But changing the 11 other engines now (to disable it, or to use
>> a different style) would also break savestates. Compared to them,
>> Kyra has not been around for long. And not being consistent breaks
>> user experience. As such, my stance is that we should keep
>> augmenting, and should change Kyra to do it. And possible more
>> engines which don't do it right now and for which there are tons of
>> game variants.
>
> As I said I don't have any problems with changing kyra to use arugmented
> target names in general. But if we (yes we not (only) I) do it, I myself
> do not really feel like thinking of an automated way to rename save files.

I agree that you (i.e. engine authors) should not have to worry about it
-- however, I am not quite sure what we are talking / worrying about here?

What I am proposing is that we change Kyra (and other engines, maybe
future engines only) to use augmented target names for newly added games.
Existing targets would not be affected.

This would cause a problem if a user re-adds a game, because the savegames
couldn't be found anymore. Annoying -- *but* I am not sure we can do
anything about it. I mean, it's trivial for "kyra1-3" to also look for
"kyra1-cd-en.*" savestates -- but the other way around it is impossible,
because the numbering depends on the order in which games were added.

A gradual shift is not really possible either, because the only effect
waiting would have here is that more and more people get configs with
"bad" target names, and thus savestates with "bad" filenames. So the
sooner we switch the Kyra (and other) detectors to generate the augmented
target names, the better.

We could maybe automatically change target names to the "new" ones if they
match the pattern "gameid-NUM", and rename savestates while doing so. But
is this really a good idea, I wonder?

I can't really think of a good transition system. In the past, we didn't
bother about this either, esp. since people with many game variants tend
to be power users who can rename savestates manually if they really want
to (admittedly, though, it is annoying).

If there are suggestions as to how to tackle this, I would like to hear
them, and I am willing to (help) implement them. In a generic fashion, if
possible.

> Also I will not tolerate any hacks to do that in the kyra engine itself, I
> believe
> AdvancedDetector should do that itself with some hints for obsolet target
> names
> though, right?
>
> Also can I somehow make AdvancedDetector use "kyra1" as a gameid for all
> games, while using for example kyra1-cd for CD version or something?
> If that isn't working I would personally wait to change kyra to use
> arugmented
> names, since I think it's rather silly to have kyra1-de or something for
> both
> floppy and cd version if they got some differences.

I agree that it should be "kyra1-cd-de" (not sure whether to mark floppy
versions via "disk", "floppy", or just by omitting this?).
What you ask for (same gameid, but an extra variant tag added to the
target name) is not possible with the AdvancedDetector right now (SCUMM
does it with it in its custom detector). BTW: We really need better
documentation for the AdvancedDetector ;-).
That is, there is one exception, we have a "demo" flag which inserts
"-demo". I see the following ways"
* Add some more flags for CD/floppy/??? variants. This would require the
least code changes, but would be rather inflexible
* switch to multiple gameids after all. another simple "solution", but ugly.
* add a new field "const char *variant" to ADGameDescription, set to 0 by
default. Most flexible, but requires updating all engines which use the
AdvDetector (not hard, though, just tedious ;)

So far, the third option seems to me by far the best. But maybe somebody
has a better idea?


>
> Also about multilanguage games, HoF supports language selection via
> ingame menu. How do I update the language without the need to fear
> that the target id was kyra1-cd-de in the beginning and now stays the same
> though I changed it to french?

Hm, normally, the detector should detect this as a game with language
"unknown" (we could add a "multi-lingual" flag in the future, though i see
little use for that). The generated target name hence would not carry any
language. Switching the "language" setting would not affect this, so I see
no risk.
Or maybe I misunderstood you?

>
> I didn't look at the AdvancedDetector code since ages. If somebody cries
> now that I should've looked at it, great, but I didn't want to change kyra
> to use it right now, so I will just ignore such comments.

Agreed -- in fact, I think no engine author should have to look at the
AdvancedDetector *code* at all -- it should be made self-explanatory /
well-documented. So glancing at the header file, and maybe looking how
other engines use it, should ideally be sufficient for 99% of all cases. I
will try to improve its docs, but I'd greatly appreciate if others could
help with that (in the end, the AdvDetector is not my baby, after all -- I
wrote the SCUMM detector, its friendly competitor ;-)

>
>> Ideally using the same style everywhere (let's keep BS1&BS2 as they
>> are, though -- these target name have been in use for ages now,
>> nothing to be gained by changing them).
>
> Whoops.. I believe if we use argumented target names with a certain
> style we should use them everywhere, but that's just my two cents.

I am torn in this regard. My hacker ego screems "YEESSS, he is right!",
and I want the tidiness. OTOH, my pragmatic part says "why bother for
those few targets which are good enough and long established -- the
pay-off will be low, and won't offset the potential suffering by some
users"...

*sigh* we should decide this, but it's a bit less important then the
other, it hink

>
>> Anyway, since we didn't ask democratically the last time, I will do
>> it now (well, we aren't a democracy anyway, but it won't hurt to ask
>> for opinions -- we aren't coloring a bikeshed here afte rall, are
>> we??? (*)):
>
> Yes, with all what I said before in my mail.
>
> // Johannes
>
> PS: Is me the only one who wonders that kyra seems to be of much
> interest when it comes to 'non standard' use of things, though other
> engines did (and some are doing) it the same?

Hmm, I can only speak for myself, and I can't say that the Kyra engine
caught my eye more than other engines. In this specific case, I noticed
the problem only because I decided to finally try to mass-add all my Kyra
variants and wondered about the result. That urge to add them all was in
turn a result of the announcement that HoF is now supported :-).
Oh, and I think the whole "augmented target" thing is not *that*
important, I just want it settled. Normally, I would have just changed it,
but I thought that maybe there are some good reasons that the Kyra engines
does what it does. And also, there was/is a *flag* for augmenting targets,
and you used to have to turn it on explicitly -- I was wondering about
that, too.

So, in this case, it was really more the advanced detector code which
(once again *g*) drew my attention, Kyra only made me notice the issue :)

> I just remember that
> FS discussion back in the days when I changed some Kyra code
> after looking at another (older) engine.

I don't claim to recall this in full detail (and don't have my email
archives with me here at work), so please bear with me if I don't recall
this correctly: But I think back then I simply noticed something "odd" (to
my eyes) in a commit to one of the more actively developed engines, and
started a discussion about it. Yes, I should have already argued about
this when the old engine introduced its bad behavior, but I simply didn't
notice it (I try to check all commits, but these days, I usually only
glance over engine specific commits, trusting engine authors to know what
they are doing ;-).


> Also I remember about a
> discussion 'caused by our event recoder not supporting kyra, just
> because kyra used the OSystem event API in a valid way, which
> was not expected when writing that event recorder code.
> ('valid' as in conforming to the documentation).

Can't comment on that, as I was not privy to this (or if I was, I totally
forgot). Can you point me to an IRC log or so?

> I don't want to complain about that feedback, but I find it rather
> strange right now... but maybe that's because of lack of sleep and
> my mood caused by this.


Well, regarding part 1 & 2, all i can say is that it was pure coincidence
that Kyra was involved, and neither was directed against Kyra, nor you, if
that's what you are hinting at. In particular, I think you know more about
our "core/common" code than most other engine authors, having contributed
substantial additions to it. And I have no particular agenda to check your
commits more than those of others (in fact, these days I really am slack
when it comes to checking any commits inside engines/ *sigh*).


Cheers,
Max