My commit 0e734722 causes lockups in SCUMM (sometimes) and SCI (very often). I didn't like the way I had fixed this before, but in the end I now had to do it in a similar way.
The engine ID identifies which engine should be used to launch the target.
Also remove the 'single ID' system. Different games from engines that used
that system now have different game IDs.
Also-By: Matthew Hoops <clone2727@gmail.com>
- At at least one place in INDY4 the game attempts to send a Roland MT-32 patch to the Amiga driver. This will fail and thus not cause any harm, but I have added a check nonetheless. The original Amiga driver ignores sysex messages that don't have the iMuse manufacturer id 7D.
- In INDY4 the iMuse player thinks it has a percussion channel due to the fact that it plays ROL resources. This should also be irrelevant for the actual playback, but I have added a check for that, too.
FOA Amiga uses 'ROL ' resources (unlike MI2 which has 'AMI ' resources). So our imuse player treated those as MT32 tracks playing on a non-MT32 device and applied GM mapping. Which of course messed up the instruments.
This warning will not only show up if a tag is actually unrecognized but also in cases where the tag is recognized, but the resource size is 0. This happens quite a lot in the Amiga version of MI2 with 'SOU ' tags.
The audio track is not broken. LEC simply didn't make good euphony music tracks for the FM-Towns. There is nothing we can do about that.
The TODO implies that someone should mess around with the audio data (change the composition?).
- move mac, pc speaker and fm-towns ims sound drivers into separate directory
(AdLib and MT32/GM drivers are still too entangled with common code to be moved so easily, especially MT32/GM. It would require lots of changes to the common code and possibly to all engines using the MidiDriver class. So I leave that for now.)
Recent GCC versions complain if you memset() a class or struct that
contain non-POD data types. Get around that by either initializing
the object when created, or by adding a reset() method.
What:
The bug is reproducible in the following ways:
1. quiting the game
2. enter challenge mode state 2, when the game starts move mouse around
the menu buttons (choose building or weapons)
Observed behavior: In he/wiz_he.cpp:2839, the function failed to return since
the stack around variable 'color' was corrupted. The game will crash then.
Analysis:
Since other function will modify memory area around local variable 'color',
the bug shoud be caused by memory overwritten. The memory write happens
in this modified file. From the code, it only wants to write a certain amount
of pixels. So I found 2 places where more pixels are written. This causes
stack memory corruption.
Fix:
Add checking. If we have written enough pixels then break.
Testing:
The game UI looks correct. Single player mode game is tested.
Bug no longer observable. Tested for both cases mentioned above.
The speech sample at VCTL offset 0x76ccbca ("Hey you!") which is used
when Indy gets caught on the German submarine seems to not be a VOC
but raw PCM s16be at (this is a guess) 44.1 kHz with a bogus VOC header.
To work around this we skip the VOC header and decode the raw PCM data.
Fixes Trac#10559
This commit introduces the following (seemingly non-invasive) changes
to make the 'imuse play' debugger command more useful (i.e. don't crash
when trying to load the wrong kind of (sound) resource.
* ScummEngine::readSoundResource()
Instead of fatally error()'ing upon hitting a non-sound resource type,
e.g. a room header (0x524d4844 aka RMHD), we now only issue a warning().
This enables the already existing dead code which properly returns 0
(aka no resource loaded).
* ResourceManager::validateResource()
Instead of fatally error()'ing upon hitting an illegal glob type we now
only issue a warning() and return false (also existing dead code).
All methods calling validateResource() check its return value so this
seems like the right thing to do anyway.
* ScummDebugger::Cmd_IMuse()
Instead of directly calling ensureResourceLoaded() we now call
getResourceAddress() instead (which in turn calls ensureResourceLoaded()
and handles other edge cases) and only attempt to play a sound if the
returned pointer actually is valid.
Fixes Trac#10527.
In some cases the pointer returned is used directly without further
error checking.
As most instances already assert() in this case this commit simply
adds asserts where missing and deemed appropriate.
v5+ scripts can request saving/loading of savegames, this type of
savegame is internally called a "temporary" savegame
(_saveStateTemporary == true) which is invisible to the user (I'm not
sure whether this is by design or not).
Currently the savegame handling in scummLoop_handleSaveLoad() doesn't
distinguish between such temporary savegames and normal autosaves and
unconditionally resets _lastSaveTime (even after loading).
This has the unwanted side effect of potentially delaying the creation
of normal autosaves which are supposed to be created in accordance with
the autosave period setting in the GUI.
This commit makes sure that _lastSaveTime only gets updated if and only
if saving a (non-temporary) autosave.
Both means to calculate `diff` are essentially equal because
wraparound of unsigned integers is well-defined and does what
the 'else' branch is simulating manually. Because of this,
gcc complains when compiling with -Wduplicated-branches.
This commit introduces the following changes:
1. Graphics::loadThumbnail()
Now returns a boolean and takes a new argument skipThumbnail which
defaults to false. In case of true, loadThumbnail() reads past the
thumbnail data in the input stream instead of actually loading the
thumbnail. This simplifies savegame handling where, up until now,
many engines always read the whole savegame metadata (including
the thumbnail) and then threw away the thumbnail when not needed
(which is in almost all cases, the most common exception being
MetaEngine::querySaveMetaInfos() which is responsible for loading
savegame metadata for displaying it in the GUI launcher.
2. readSavegameHeader()
Engines which already implement such a method (name varies) now take
a new argument skipThumbnail (default: true) which is passed
through to loadThumbnail(). This means that the default case for
readSavegameHeader() is now _not_ loading the thumbnail from a
savegame and just reading past it. In those cases, e.g.
querySaveMetaInfos(), where we actually are interested in loading
the thumbnail readSavegameHeader() needs to explicitely be called
with skipThumbnail == false.
Engines whose readSavegameHeader() (name varies) already takes an
argument loadThumbnail have been adapted to have a similar
prototype and semantics.
I.e. readSaveHeader(in, loadThumbnail, header) now is
readSaveHeader(in, header, skipThumbnail).
3. Error handling
Engines which previously did not check the return value of
readSavegameHeader() (name varies) now do so ensuring that possibly
broken savegames (be it a broken thumbnail or something else) don't
make it into the GUI launcher list in the first place.
On my AZERTY keyboard the period is obtained using SHIFT + ; and
the code checking the PERIOD keycode and no modifier was failing
on both account. The manual for my French DOTT mentions the period
key, but I have not actually checked how it worked with the original
executable and if using the semicolon key without shift or using the
colon key (which would be the period key on a English-US layout)
works.
Drawing nows happens directly when the Dialog or Widget draw methods are
called. This makes it easy to debug why a particular low level draw
method was called, by inspecting the call stack.
This replaces the notion of "buffering" by two independant ways to
control what is drawn and where:
- The active layer is used to select whether the foreground or
background part of the dialogs are rendered by the draw calls.
- The active surface is used to select if the draw calls affect the back
buffer or the screen.
The foreground layer of the active dialog is drawn directly to the
screen. Its background layer is drawn to the back buffer. This way
widgets can restore the back buffer in order to update without having to
redraw the dialog's background.
Dialogs lower in the dialog stack are drawn entirely to the back buffer.
Rewrote the detection matching to use the extra field like the other
two.
This requires extensive testing due to touching code shared for a lot of
games and being sensitive to individual versions of games.
This happens when clicking on the triangular button in room 27 in
The Dig.
There are probably several other places where this overflow
happens, since there are several different `int args[16]` in
the code (and many more `int args[` of various sizes, not all of
which are at least NUM_SCRIPT_LOCAL).
It looks like the code was there, but it was never fully implemented
because _curSoundPos was never being incremented. Experimentally,
it looks like it works if it is a 60FPS counter.
This situation is triggered normally when _currentScript is 0xFF,
but it could potentially also happen if _currentScript is some
other number >= NUM_SCRIPT_SLOT, so the check is a bit more
conservative than it might appear to need to be.
This flag is removed for a few reasons:
* Engines universally set this flag to true for widths > 320,
which made it redundant everywhere;
* This flag functioned primarily as a "force 1x scaler" flag,
since its behaviour was almost completely undocumented and users
would need to figure out that they'd need an explicit non-default
scaler set to get a scaler to operate at widths > 320;
* (Most importantly) engines should not be in the business of
deciding how the backend may choose to render its virtual screen.
The choice of rendering behaviour belongs to the user, and the
backend, in that order.
A nearby future commit restores the default1x scaler behaviour in
the SDL backend code for the moment, but in the future it is my
hope that there will be a better configuration UI to allow users
to specify how they want scaling to work for high resolutions.
Caught by GCC 7 (though the warning seemed misleading to me). Since
SoundDesc->name can potentially be 14 characters "%s_reg%03d.fla"
can be 25 characters, plus the terminating \0.
Since case 64 was added for "MM C64 Costume Animation", and
considering the way it's written to only affect game version 0,
it is clearly an intentional fall through.
Fix the "MI2: Two soundtracks playing at once" bug from the bug tracker.
It's triggered when the player enters the kiosk for the second time and
leaves before tune 113 is started.
Fix the "MI2: Two soundtracks playing at once" bug from the forum. It's
triggered when the player is moving faster then the scripts expect causing
the "stop tune 117" command be executed before the start of the tune.
Effectily creating a hanging tune 107.
This patch kills the tune when this sitation is detected.
We have translations enabled for the warning created
by advancedDetector.cpp, so I think it would be neat
to have translations enabled here too.
This also enables the message about the "Lite" version
of Putt-Putt Saves the Zoo being unsupported to the
translation database.
For some games the Steam Mac version and Mac CD version have the
same executable name and since we use the executable name to
detect the Steam version it becomes a candidate for the CD version
as well. Moving it at the end of the list ensures that the CD version
entry takes precedence. This fixes bug #7060.
If we don't provide the platform and the candidate is a Steam version
it errors out with 'Unable to find Steam executable from detection
pattern!' message.
