Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DIRECTOR: Even more fixes #4755

Merged
merged 18 commits into from
Mar 6, 2023
Merged

Conversation

moralrecordings
Copy link
Contributor

No description provided.

@moralrecordings moralrecordings force-pushed the director_fixes branch 5 times, most recently from 34717b6 to f05e830 Compare March 4, 2023 11:23
pathMakeRelative will attempt to look for files in locations listed in
the search path. This can include directories that don't end with a
separator. As such, ensure that paths being tested have a separator so
they don't get accidentally merged with the target filename.

Fixes the loading sequence in The Dark Eye.
Previously, pathMakeRelative didn't make a distinction between relative
and absolute paths. The input would always be converted to a relative
path, then checked against a number of places (e.g. the current
working directory, all the locations in the search path...).

This causes an issue for functions such as getNthFileNameInFolder. Quite
often, this is used for absolute paths constructed with "the pathName"
as a base. ScummVM assumes the game root is also the root of the volume
(e.g. "C:\"). As such, when you would call getNthFileNameInFolder("C:\",
1), this would be translated to a relative path of "", and then match
with the current directory provided from getCurrentPath; clearly not the
same thing.

To fix this, we detect if the input to pathMakeRelative is an absolute path,
and if so avoid checking every location in the search path and prefixing
the result with getCurrentPath().

Fixes "I am unable to save or retrieve games" error message in The Dark Eye.
SpaceMgr::m_parseText can be called multiple times, and assume the context
from the previous call exists. For example, ml.dir in The Dark Eye will
call it twice; the first time with a full hierarchy starting with a
SPACECOLLECTION, the second time with a hierarchy starting with a SPACE
and assuming _curSpaceCollection was set by the previous call.

We should be able to reuse the object's global "_cur" values for this,
as the game will override them soon after loading the model with a
SpaceMgr::m_setCurData call.

Fixes loading the scene graph data in The Dark Eye.
Fixes visiting Mount Olympus in Wrath of the Gods.
b_go() should only reset the custom event handlers when switching
movies, not just frames.

Fixes firing the bow at the dragon in Wrath of the Gods.
Fixes palette glitches when rotating the probe in the Mac version
of Majestic: Part 1.
The CLUT in the resource fork can contain palettes unused by Director;
the key is that all of the ones used for cast members have a palette ID
of the cast ID   1024. This is effectively what the D2 branch was
relying on, so we can clean up the logic and use it for D3 as well.

Fixes the Invictus logo in Over-Ring-Under, the colour cycling in The 7
Colors, and countless others.
Fixes the colors in Gahan Wilson's The Ultimate Haunted House.
Partially rolls back a change introduced in
2caa338; Director relies on the notion
of updateScreen() being near-zero cost if there are no changes.
For example, starting director:henachoco05 went from taking a few
milliseconds to taking 11 seconds, and opening the top menu in
director:easternmind only works on 10% of the screens.
stream.readByte();
index = 3;
if (increment)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a superfluous if statement as increment is set to true and never to false.
Is this intended?

Copy link
Member

@sev- sev- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent progress!

Common::String Archive::formatArchiveInfo() {
Common::String result = "unknown, ";
if (_isBigEndian)
result = "big endian,";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use our widespread abbreviations 'BE' and 'LE'.

In Director, palette IDs in the palette channel apply to all subsequent
frames. If we seek to an arbitrary frame, the correct palette
should be applied.

Fixes the introductory cutscene in Majestic: Part 1.
@sev-
Copy link
Member

sev- commented Mar 6, 2023

Okay, merging this before it went bitrot :)

@sev- sev- merged commit 585ec55 into scummvm:master Mar 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants