[Scummvm-devel] Re: [Scummvm-cvs-logs] CVS: scummvm/simon simon-md5.h,NONE,1.7.2.1

[Max Horn]

Am 14.08.2004 um 09:13 schrieb Travis Howell:

> Max Horn wrote:
>> Am 14.08.2004 um 06:37 schrieb Travis Howell:
>>
>>> Update of /cvsroot/scummvm/scummvm/simon
>>> In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv14407/simon
>>>
>>> Added Files:
>>>       Tag: branch-0-6-0
>>> simon-md5.h
>>> Log Message:
>>>
>>> Back port Simon engine detection changes
>>>
>> Uhm,
>>
>> Kirben, for my taste those recent backports (HE, Windows platform,
>> Simon MD5) are going a bit to far -- those are big feature backports,
>> not just small isolated bug fixes, some with rather deep impact (Simon
>> MD5 removes targets, for example). The main idea of having a 0.6.x
>> branch is to be able to backport important bug fixes, and then be able
>> to make a bug fix release, *if desired*.
>> Neither of those "backports" qualifies in that category, though....
>>
>> If we end up backporting all our 0.7.x changes, there is no point in
>> keeping a separate branch...
>>
>> In any case, I'd prefer to see a 0.7.0 release next over a 0.6.2
>> release, so maybe this discussion is moot :-)
>
> What HE changes ? I made no HE specific changes in the last series of 
> commits.

Well, I thought Actor::offs_x, offs_y, actorDrawVirScr, flip, skipLimb 
were HE specific... ?


> The last series of commits to 0.6.0 branch were mainly to sync changes 
> between save game versions on two branches. Since  a few bug fixes 
> required save game to be increased in main branch and were worth back 
> porting. Is always good to have save game version the same between 
> release version and CVS when possible too.

Sure, that's ideally the case, but since the release version is 0.6.1b, 
you didn't achieve it anyway :-). Furthermore, while it's a worthy 
goal, I judge having a stable release branch a more worthy goal; and 
some of the changes you ported are in my eyes controversial. For 
example, the Actor::boxScale change, which i still don't like very 
much. Several SCUMM variables were added (like VAR_VOICE_MODE / 
VAR_ROOM_FLAG); changes which may improve compatibility, but which are 
also virtually untested, so they can also introduce regressions. So 
while they may turn out to be very very good improvements, they are 
untested, can potentially cause regressions, and hence should IMNHSO 
not be applied to a release branch. Unless they received proper 
testing... did they?


> The reason I back ported the Simon detection changes (Along with the 
> required Windows platform), was for the Dreamcast port, where those 
> particular games have auto detection issues. So they would be classed 
> as a bug fix, although mainly specific to that port.

Fair enough. As long as upgrading is smooth for existing users (which I 
believe your patch now ensures?) that's acceptable.


Bye,

Max