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

Travis Howell kirben at optusnet.com.au
Sat Aug 14 07:27:01 CEST 2004


Max Horn wrote:
> 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... ?

They were just part of back porting the save game version changes, only 
actorDrawVirScr and skipLimb are HE specific.
Actor::offs_x, offs_y and flip are referenced in all later games (scumm7/8), 
although I'm not sure where they are used in those games.

>> 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?

I'm getting a bit lost here, some of the changes you mention aren't recent 
at all:
VAR_ROOM_FLAG has been around a long time, the minor changes I back ported 
were small and verified via disasm. of games from each SCUMM version.
VAR_VOICE_MODE was in 0.6.1 too and in main branch for long time now, it has 
not been recently changed. It solved missing subtitles in several scenes of 
COMI and the Dig, although it depends on the bug fixes to subtitles 
translation code you made.
The only SCUMM variable I added during recent back ports was 
VAR_NUM_GLOBAL_OBJS and I enabled the use of VAR_DEFAULT_TALK_DELAY. These 
two SCUMM variable changes were originally verified via disasm. too and any 
benefits would be positive (Since we previously just completely ignored 
them).
The actor scale changes for scumm4 are straight from monkeyvga disasm., I 
just mapped out that area the other day to be sure. So they are correct and 
unlikely to cause any regressions.The original monkeyvga used a single actor 
scale variable, checking it against 0x8000 in only two functions (As 
mentioned in comment for original patch).
The back ports to keep save game up to date in 0.6.0 branch included (In 
addition to changes mentioned above):
Cleanup of V1 Maniac Mansion speech array
Cleanup of extra palette used in Sam & Max (Which was just _shadowPalette)
Fix for CD music continuing in Monkey Island 1 (CD) when it should stop.

Overall quite minor changes, none of those should cause any regressions. 
They just made for one large change when back porting to 0.6.0 branch.
I think having save game versions in sync is always worth it, when possible. 
Since many users use CVS version and are annoyed to find the next ScummVM 
version released isn't compatbile with the saved games.

>> 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.

Yes I added and tested the auto upgrading of older targets, as you 
suggested. The other benefit is users will usually have correct target auto 
choosen now, many users previously found the choice of multiple targets 
confusing too. 





More information about the Scummvm-devel mailing list