Opened 17 years ago

Closed 16 years ago

Last modified 16 years ago

#1098 closed defect (fixed)

SAM: Screen Saver generates warnings

Reported by: SF/torbenb Owned by: Kirben
Priority: low Component: Engine: SCUMM
Keywords: Cc:
Game: Sam and Max

Description

When you leave the game running for about 5 minutes a screen saver (I guess) kicks in.

ScummVM generates these warnings:

WARNING: o6_roomops:180: partially unimplemented! WARNING: stub o6_kernelSetFuctions_117()! WARNING: Player::addParameterFader(): Unknown parameter 15!

Ticket imported from: #781683. Ticket imported from: bugs/1098.

Change History (14)

comment:1 by SF/torbenb, 17 years ago

Component: Engine: SCUMM
Game: Sam and Max
Priority: normallow

comment:2 by SF/torbenb, 17 years ago

Summary: Screen Saver (?) generates warningsSAM: Screen Saver (?) generates warnings

comment:3 by fingolfin, 17 years ago

Owner: set to SF/jamieson630

comment:4 by fingolfin, 17 years ago

Not really a bug, more like a missing feature, but still would be nice to fix this.

I wonder, are the parameter fader numbers the same as in getParam? Would seem to make sense, wouldn't it? In that case, 15 probably refers to "part volume". Also in IMuseInternal::doCommand(), the case 16 (the cmds seems to be +1 here) invokes set_volchan. Alas, Jamieson is the iMuse Voodoo Master, and I am sure he'll know much better than me about this.

Hm, looking again, getParam has param 4 as "_detune" (whatever that is?) and 5 as speed, while the pfSpeed = 4... does that means they have different ideas about what a param ID means, after all, or is this a bug?

Unrelated to fixing this or not, might be a nice idea to use an enum to give symbolic names to all these iMuse commands

comment:5 by eriktorbjorn, 17 years ago

If you want to speed up the debugging of the screensaver, try setting variable 132 to 0. That's the number of minutes before the screensaver starts, so it will make the game pretty unplayable of course.

See my comment in the o6_kernelSetFunctions_117() stub for some more details.

comment:6 by SF/jamieson630, 17 years ago

I tried reconciling the parameter numbers in addParameterFader()/transitionParameters() and getParam(), and I have to conclude that they are different. For starters, I'm willing to assume that the numbers in getParam() are not incorrect, since they've been there forever and seem to work fine. As far as the parameter faders, some numbers in getParam() don't make any sense for a continuous transition (e.g. priority and parse position), and other parameters are specified differently. For instance, I am reasonably certain that transpose (3) and detune (4) are doubled up in one fade parameter (3), which is measured in cents (1/100 of a half step). So transpose is based on (param / 100), while detune is based on (param % 100). Since detune does not have its own parameter, then, speed (and anything above it, presumably) was bumped down from 5 to 4.

This is still all speculation, though. I just tried matching up the parameter numbers and testing the Tunnel of Love (our best test case for parameter fade behavior), and the music changes when shorting out the fuse box don't work correctly at all.

As for parameter 15 (the original topic of this bug), getParam () would say that's a part volume fade, which doesn't make any sense for a screensaver. (Why fade out, or in, only one instrument?) I'll have to look at the original distro to see how the music changes when the screensaver kicks in. I frankly don't remember a thing about the screensaver from that game.

comment:7 by SF/jamieson630, 17 years ago

Summary: SAM: Screen Saver (?) generates warningsSAM: Screen Saver generates warnings

comment:8 by SF/jamieson630, 17 years ago

Owner: changed from SF/jamieson630 to eriktorbjorn

comment:9 by SF/jamieson630, 17 years ago

First, in the original distro, no audible music changes occur when a screensaver kicks in.

Second, when testing the screensaver in ScummVM, I did not get any addParameterFader() errors. Consequently, I think that was incidental to the particular music being played at the time torbenb's screensaver test produced the reported warnings.

eriktorbjorn, I'll kick this over to you since you no more about o6_kernelSetFunctions_117() than I do.

NOTE: When testing the screensaver, I found that it takes 6 minutes of inactivity to activate the screensaver in the original distro. On my 366 Mhz Celeron, it took about 5 minutes and 40 seconds in ScummVM. Is one of our timers running a tad fast? Do we care?

comment:10 by SF/ender, 17 years ago

I wouldn't say we care, it's a small difference and getting the timing EXACTLY the same as the original (in a cross-platform way) isn't as easy as it sounds.

comment:11 by fingolfin, 16 years ago

Erik, did you try if using the V7 code for op 117 helps? That is, freezeScripts(2);

comment:12 by eriktorbjorn, 16 years ago

As far as I could tell, adding freezeScripts(2) to that kernel function stub didn't do any good.

Anyway, I thought the background animations were pretty much on "auto-pilot" anyway, i.e. once they've started the scripts can pretty much forget about them.

comment:13 by Kirben, 16 years ago

I added code for missing case from disasm. Basically it just freezes all scripts, even if they are freeze resistant.

comment:14 by Kirben, 16 years ago

Owner: changed from eriktorbjorn to Kirben
Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.