[Scummvm-devel] Re: [Scummvm-cvs-logs] CVS: scummvm/scumm actor.cpp,1.150,1.151 script_v5.cpp,1.147,1.148 scummvm.cpp,2.323,2.324

Max Horn max at quendi.de
Sat Aug 23 11:32:03 CEST 2003


Am Samstag, 23.08.03 um 18:01 Uhr schrieb J.Brown (Ender):

>>> I do not know what the purpose of actor 0 was in the original. Either
>>> it's a no-op, or it's used to store actor default values.
>>
>> I know of exactly one "legal" use of actor 0: the default actor talk
>> color is set by the actor-set opcode, specifying actor 0. All other
>> occurrences are most likely to be caused by a script bug. Note that in
>> the past we discovered several script & ScummVM bugs via this check.
>
> Yes, but many of these fixes have involved simply ignoring the problem.

Yup, indeed. But in each case, we carefully verified that ignoring the 
problem is safe - the game works correctly. With the global approach of 
just always ignoring the problem, we have no way to deal with cases for 
which "just ignoring" is not enough. Which makes finding the cause of 
such bugs much harder.


> The original interpreters, at least until FOA, simply store any 
> requested
> value for Actor 0 without complaint. So whilst, yes, these problems are
> all script bugs... these bugs are in some cases also present in the
> original engine, and are treated as completely valid cases. (All the 
> actor
> draw loops operation from 1 > MAX_ACTORS, so actor 0 is just treated 
> as a
> dummy case)
Hum, so what? Don't see how that makes any difference... Yeah of course 
this "actor 0 allowed" change works, but my point is not that I am 
saying it's not working. My point is that I think it's a bad solution 
to the problem.


> I discussed this for some time with him, and after examining this and
> several other Actor 0 bugs found over time, this modification was added
> with my permission.
Fine, I didn't expect Erik to make such a change w/o discussing it 
first anyway, he's a very considerate & careful coder after all, who 
prefers to double check his solutions before he applies them, something 
which I appreciate a lot :-) However, it is irrelevant and doesn't 
influence my opinion about the change :-)

> As long as it's a low-level debug (level 1), than I
> see no reason why we should not behave as th original interpreter 
> did...
See above. Another reason: it caused us lots of pain in the past, when 
we accidentally made a change to the save game system which broke save 
game compatibility, and we didn't notice it. With the current code, it 
would have immediately lead to an error, and the bug would have been 
fixed on the spot. As it was, the bug was rather silent (i.e. at first 
you didn't notice any problems) but after some playing usually would 
render your savegames unusable. Was very nasty, and I had to add some 
ugly work around hack to the save/load game to be able to at least 
partially rescue those save games.

I guess in the end, though, my opinion on this is strongly influence by 
the fact that I don't like ignoring possible problems, and rather try 
to tackle them proactively. Like, by adding assert() all over the 
place, and coding defensively.


> as at least in some cases (although of course some Actor 0's will be a
> valid bug) these conditions are hit with that interpterer as well.
>
Hm, I don't agree with this reasoning. Besides the actor 0 bugs, there 
are various other bugs in Scumm scripts which did work in the original 
engine "by chance", but which we *have* to actively work around. Unlike 
the original Scumm engine, for us accessing invalid memory may cause 
seg faults. And our memory is not always set to 0. Also our memory 
layout (of structs etc.) is different. Some buggy scripts only worked 
in the original engine, becausethe memory which was incorrectly 
accessed had a "good" value by chance. Not something one should rely 
on, IMHO.


Cheers,

Max





More information about the Scummvm-devel mailing list