VCRUISE: Add V-Cruise engine #4710

elasota · 2023-02-13T14:20:24Z

This adds initial support for the V-Cruise engine used by Reah: Face the Unknown and Schizm: Mysterious Journey and initial detection for Reah. (I actually need to figure out the detection some more because the GOG version and DVD version only ship English VO.)

This is very preliminary right now, right now it boots Reah to the intro cinematic and then gets stuck at the opening screen, I'm working on getting navigation implemented.

Documentation of its formats (and a decoder/encoder for the scripts) currently resides at https://github.com/elasota/vcr, everything is documented except for yet-to-be-understood script ops and the "morph" format which was added in Schizm.

The biggest change to core is the addition of cursor masks. Reah uses a magnifying glass cursor where the lens part is inverted, unfortunately ScummVM only supports color key cursors. This tries to add the groundwork (i.e. mode values and feature flags) for fully supporting the Windows and classic MacOS cursor formats, although only total inversion is supported in OpenGL right now.

digitall · 2023-02-13T15:06:22Z

@elasota : One warning of missing gameID in the detection table entries:

In file included from engines/vcruise/detection.cpp:37:
./engines/vcruise/detection_tables.h:49:1: warning: missing initializer for member ‘VCruise::VCruiseGameDescription::gameID’ [-Wmissing-field-initializers]
 };

digitall · 2023-02-13T15:46:31Z

@elasota : A few more warnings (The first one is the need to avoid "??" and "???" as they have syntatic meaning in C/C++):

engines/vcruise/runtime.cpp:814:78: warning: trigraph ??) converted to ] [-Wtrigraphs]
                 warning("sanimL second operand wasn't zero (what does that do??)");
                                                                               
engines/vcruise/runtime.cpp: In member function ‘void VCruise::Runtime::scriptOpSoundL2(ScriptArg_t)’:
engines/vcruise/runtime.cpp:781:22: warning: variable ‘stackArgs’ set but not used [-Wunused-but-set-variable]
         StackValue_t stackArgs[n];                                                                \
                      ^~~~~~~~~
engines/vcruise/runtime.cpp:888:9: note: in expansion of macro ‘TAKE_STACK’
         TAKE_STACK(2);
         ^~~~~~~~~~
engines/vcruise/runtime.cpp: In member function ‘void VCruise::Runtime::scriptOpMusicUp(ScriptArg_t)’:
engines/vcruise/runtime.cpp:781:22: warning: variable ‘stackArgs’ set but not used [-Wunused-but-set-variable]
         StackValue_t stackArgs[n];                                                                \
                      ^~~~~~~~~
engines/vcruise/runtime.cpp:900:9: note: in expansion of macro ‘TAKE_STACK’
         TAKE_STACK(2);
         ^~~~~~~~~~
engines/vcruise/runtime.cpp: In member function ‘void VCruise::Runtime::scriptOpMusicDn(ScriptArg_t)’:
engines/vcruise/runtime.cpp:781:22: warning: variable ‘stackArgs’ set but not used [-Wunused-but-set-variable]
         StackValue_t stackArgs[n];                                                                \
                      ^~~~~~~~~
engines/vcruise/runtime.cpp:906:9: note: in expansion of macro ‘TAKE_STACK’
         TAKE_STACK(2);
         ^~~~~~~~~~
engines/vcruise/runtime.cpp: In member function ‘void VCruise::Runtime::scriptOpDrop(ScriptArg_t)’:
engines/vcruise/runtime.cpp:781:22: warning: variable ‘stackArgs’ set but not used [-Wunused-but-set-variable]
         StackValue_t stackArgs[n];                                                                \
                      ^~~~~~~~~
engines/vcruise/runtime.cpp:921:9: note: in expansion of macro ‘TAKE_STACK’
         TAKE_STACK(1);
         ^~~~~~~~~~
engines/vcruise/runtime.cpp: In member function ‘void VCruise::Runtime::scriptOpDisc1(ScriptArg_t)’:
engines/vcruise/runtime.cpp:781:22: warning: variable ‘stackArgs’ set but not used [-Wunused-but-set-variable]
         StackValue_t stackArgs[n];                                                                \
                      ^~~~~~~~~
engines/vcruise/runtime.cpp:968:9: note: in expansion of macro ‘TAKE_STACK’
         TAKE_STACK(1);
         ^~~~~~~~~~
engines/vcruise/runtime.cpp: In member function ‘void VCruise::Runtime::scriptOpDisc2(ScriptArg_t)’:
engines/vcruise/runtime.cpp:781:22: warning: variable ‘stackArgs’ set but not used [-Wunused-but-set-variable]
         StackValue_t stackArgs[n];                                                                \
                      ^~~~~~~~~
engines/vcruise/runtime.cpp:974:9: note: in expansion of macro ‘TAKE_STACK’
         TAKE_STACK(2);
         ^~~~~~~~~~
engines/vcruise/runtime.cpp: In member function ‘void VCruise::Runtime::scriptOpDisc3(ScriptArg_t)’:
engines/vcruise/runtime.cpp:781:22: warning: variable ‘stackArgs’ set but not used [-Wunused-but-set-variable]
         StackValue_t stackArgs[n];                                                                \
                      ^~~~~~~~~
engines/vcruise/runtime.cpp:980:9: note: in expansion of macro ‘TAKE_STACK’
         TAKE_STACK(3);
         ^~~~~~~~~~
engines/vcruise/script.cpp: In member function ‘void VCruise::ScriptCompiler::compileRoomScriptSet(VCruise::ScriptSet*)’:
engines/vcruise/script.cpp:256:14: warning: unused variable ‘roomID’ [-Wunused-variable]
         uint roomID = 0;
              ^~~~~~
engines/vcruise/script.cpp: In function ‘Common::SharedPtr<VCruise::ScriptSet> VCruise::compileLogicFile(Common::ReadStream&, uint, const Common::String&)’:
engines/vcruise/script.cpp:756:14: warning: unused variable ‘cipherOffset’ [-Wunused-variable]
         uint cipherOffset = 255u - (streamSize % 255u);
              ^~~~~~~~~~~~

digitall · 2023-02-13T18:47:23Z

@elasota : That gets rid of a fair amount of warnings. Only a few remain:

engines/vcruise/runtime.cpp: In member function ‘void VCruise::Runtime::scriptOpDisc1(ScriptArg_t)’:
engines/vcruise/runtime.cpp:781:22: warning: variable ‘stackArgs’ set but not used [-Wunused-but-set-variable]
         StackValue_t stackArgs[n];                                                                \
                      ^~~~~~~~~
engines/vcruise/runtime.cpp:972:9: note: in expansion of macro ‘TAKE_STACK’
         TAKE_STACK(1);
         ^~~~~~~~~~
engines/vcruise/runtime.cpp: In member function ‘void VCruise::Runtime::scriptOpDisc2(ScriptArg_t)’:
engines/vcruise/runtime.cpp:781:22: warning: variable ‘stackArgs’ set but not used [-Wunused-but-set-variable]
         StackValue_t stackArgs[n];                                                                \
                      ^~~~~~~~~
engines/vcruise/runtime.cpp:978:9: note: in expansion of macro ‘TAKE_STACK’
         TAKE_STACK(2);
         ^~~~~~~~~~
engines/vcruise/runtime.cpp: In member function ‘void VCruise::Runtime::scriptOpDisc3(ScriptArg_t)’:
engines/vcruise/runtime.cpp:781:22: warning: variable ‘stackArgs’ set but not used [-Wunused-but-set-variable]
         StackValue_t stackArgs[n];                                                                \
                      ^~~~~~~~~
engines/vcruise/runtime.cpp:984:9: note: in expansion of macro ‘TAKE_STACK’
         TAKE_STACK(3);
         ^~~~~~~~~~

sev- · 2023-02-14T00:26:00Z

Before going further into the engine review, I'd like to ask you to split the cursor changes into a separate PR so that it can be reviewed and merged before the engine.

digitall · 2023-02-14T01:52:03Z

@elasota : Now clean of warnings for me. Thanks for working on this. If you need playtesting, I have Reah and Schizm from CD.

digitall · 2023-02-14T02:02:41Z

@elasota : Further note on my 6 CD version of Reah. Missing detection entry:

Matched game IDs for the vcruise engine: reah-win

  {"Reah.exe", 0, "77bc7f7819cdd443f52b193529138c87", 305664},

For reference, full file md5sum is:

md5sum -b Reah/Reah.exe 
c8cce9a47fba364b9ec288e2e76bcc77 *Reah/Reah.exe

elasota · 2023-02-14T04:21:22Z

Before going further into the engine review, I'd like to ask you to split the cursor changes into a separate PR so that it can be reviewed and merged before the engine.

Moved this to pull #4713 but also fixed a bunch of issues, so will have to redo this PR after that's merged.

Further note on my 6 CD version of Reah. Missing detection entry:

One thing I would be interested with the 6CD version is what format the music is in. The DVD version uses MP3-in-WAV music but based on some things I've been reading the CD-ROM version may used uncompressed, so it would be nice to get an MD5 + file size of Music-00.wav too.

I am also not sure if there exist versions with other language VO. The DVD version only has English VO.

digitall · 2023-02-14T15:30:56Z

@elasota : Here are the required detection entries for my copies of Reah (6 CD) and Schizm (5 CD):

	{ // Reah: Face the Unknown, 6 CD Version
		{
			"reah",
			"CD",
			{
				{"Reah.exe", 0, "77bc7f7819cdd443f52b193529138c87", 305664},
				AD_LISTEND
			},
			Common::EN_ANY,
			Common::kPlatformWindows,
			ADGF_UNSTABLE,
			GUIO0()
		},
		GID_REAH,
	},
	{ // Schizm, 5 CD Version
		{
			"schizm",
			"CD",
			{
				{"Schizm.exe", 0, "296edd26d951c3bdc4d303c4c88b27cd", 364544},
				AD_LISTEND
			},
			Common::EN_ANY,
			Common::kPlatformWindows,
			ADGF_UNSTABLE,
			GUIO0()
		},
		GID_SCHIZM,
	},

digitall · 2023-02-14T15:34:42Z

@elasota : My copy of Reah has Music-00.wav in pcm_s16le 2ch 22050Hz format i.e. uncompressed.
File md5sum is af80fad4ec51b3467b43f4ca531b7788 *Music-00.wav with size of 14395864 bytes

digitall · 2023-02-14T15:36:50Z

@elasota : I can provide full file listing with MD5sum if required of both. Just a further note that currently when I try to start either, it immediately exits with a SDL_BlitSurface failed error:

Running Reah: Face the Unknown (CD/Windows/English)
Reah.exe: 77bc7f7819cdd443f52b193529138c87, 305664 bytes.
Surface::fillRect: bytesPerPixel must be 1, 2, or 4!
SDL_BlitSurface failed: Surfaces must not be locked during blit!
Debugger started, type 'exit' to return to the game.
Type 'help' to see a little list of commands and variables.
ERROR: SDL_BlitSurface failed: Surfaces must not be locked during blit!
Read 19 history entries
SDL_BlitSurface failed: Surfaces must not be locked during blit!

elasota · 2023-02-14T16:52:54Z

It looks like the problem there is the graphics mode selection in SurfaceSDL can select a 24-bit mode and fillScreen with a 24-bit mode crashes SurfaceSDL.

… stream names in Reah

elasota · 2023-02-14T18:26:39Z

Removed the cursor changes, fixed the crash in SurfaceSDL.

digitall · 2023-02-14T22:27:38Z

@elasota : That has fixed the crash for me. I get the Reah intro now up to the journalist outside of the walled city gate. I note that you didn't include the detection entries I provided yet :) Apart from that, the Schizm.exe is identical between the GOG release and the original 5 CD release so that may need another file in detection to distinguish if needed. You probably already know, but Schizm fails to start with "Couldn't figure out what screen to start on". Anyway, good luck on getting this working.

digitall · 2023-02-19T18:42:50Z

@elasota : Just a quick note against commit e7b0634, this warning turned up:
engines/vcruise/runtime.cpp: In member function ‘void VCruise::Runtime::continuePlayingAnimation(bool, bool&)’:
engines/vcruise/runtime.cpp:346:50: warning: comparison of integer expressions of different signedness: ‘uint’ {aka ‘unsigned int’} and ‘int’ [-Wsign-compare]
if (_animDisplayingFrame >= static_cast(_animLastFrame)) {

I would just change the type of one of those variables to match i.e. both signed or both unsigned instead and avoid the cast...

sev-

Finished my first review round

engines/vcruise/detection.cpp

engines/vcruise/detection_tables.h

engines/vcruise/metaengine.cpp

engines/vcruise/vcruise.cpp

engines/vcruise/vcruise.h

elasota · 2023-02-20T17:25:36Z

I think all issues should be cleaned up now except for the pixel format and expectLine implementation which I think there are valid technical reasons for doing the way they are, and the AVI copyRect handling which is intentional.

sev-

Whoa! You're fast. I have only one small issue/remark which could be resolved in-tree.

sev- · 2023-02-21T00:21:21Z

engines/vcruise/runtime.cpp

+					if (stream->read(interactionData, 12) != 12)
+						error("Error reading interaction data");
+
+					idef.rect = Common::Rect(READ_LE_INT16(interactionData + 0), READ_LE_INT16(interactionData + 2), READ_LE_INT16(interactionData + 4), READ_LE_INT16(interactionData + 6));


Why not use here a more intuitive stream->readUint16LE() for every variable?

C++ order of parameter evaluation is undefined (which I have been burned by before - VS optimizer can reorder execution of parameter expressions even if the expressions have side effects) so it would need to be separate read lines, plus I think this way makes it easier to detect a read failure.

sev- · 2023-02-21T00:31:00Z

engines/vcruise/textparser.cpp

+}
+
+bool TextParser::readOneChar(char &outC, TextParserState &outState) {
+	if (_returnedBufferPos == kReturnedBufferSize) {


I recommend using wrapBufferedReadStream() to achieve the same result. See common/bufferedstream.h

sev- · 2023-02-21T16:16:48Z

Time to merge...

elasota added 14 commits February 14, 2023 13:08

VCRUISE: Initial commit, boilerplate

6576998

VCRUISE: Add script compiler and initial loading things

3de928f

VCRUISE: Add map loader

c92c128

VIDEO: Tag V-Cruise as using AVIDecoder

d1701dd

VCRUISE: Add enough stuff to render opening cinematic

98672b9

VIDEO: Increase stream name capacity to deal with long (67 character)…

5a938db

… stream names in Reah

VCRUISE: Add more things to get Reah to the first screen

d722d46

VCRUISE: Rename detection entry from GOG to DVD

e2406e3

VCRUISE: Add game ID initializer for end marker.

0d7356e

VCRUISE: Remove unused variable

9b6ccf8

VCRUISE: Quiet some warnings

f5c1f98

VCRUISE: Remove unused variable

29b6629

VCRUISE: Quiet more unused variable warnings

f95e46d

VCRUISE: Only allow real 32-bit formats, permit 555 RGB.

30e2024

elasota force-pushed the vcruise branch from ffa81f4 to 30e2024 Compare February 14, 2023 18:23

elasota added 2 commits February 16, 2023 12:20

VCRUISE: Add Schizm and Reah CD

3c285fa

VCRUISE: Fix video frame overrun

2c20a8e

elasota added 4 commits February 17, 2023 21:13

VCRUISE: Fix formatting

b3c2af8

VCRUISE: Fix script miscompilation

18a765b

VCRUISE: Implement debug mode and some idle state functionality

1c013fa

VCRUISE: Combine env vars, make some tweaks for panorama input detection

e7b0634

elasota force-pushed the vcruise branch from 4bc997a to e7b0634 Compare February 18, 2023 23:58

sev- requested changes Feb 20, 2023

View reviewed changes

elasota added 9 commits February 20, 2023 10:34

VCRUISE: Clean up some code quality issues

b9bdbdf

VCRUISE: Clean up file access

166988c

VCRUISE: Stub unsupported opcodes

a5be1b1

VCRUISE: Clean up hash map types

5484c1c

VCRUISE: Remove input buffering from TextParser

b89aeee

VCRUISE: Use INI loader for index

e03014d

VCRUISE: Use sscanf for number parsing

2620289

VCRUISE: Cleanup

b717e84

VCRUISE: Use sscanf for TextParser number parsing

923295d

sev- approved these changes Feb 21, 2023

View reviewed changes

sev- reviewed Feb 21, 2023

View reviewed changes

VCRUISE: Initial panorama controls

8139cec

sev- merged commit 1f4f556 into scummvm:master Feb 21, 2023

Uh oh!

VCRUISE: Add V-Cruise engine #4710

VCRUISE: Add V-Cruise engine #4710

Uh oh!

Conversation

elasota commented Feb 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

digitall commented Feb 13, 2023

Uh oh!

digitall commented Feb 13, 2023

Uh oh!

digitall commented Feb 13, 2023

Uh oh!

sev- commented Feb 14, 2023

Uh oh!

digitall commented Feb 14, 2023

Uh oh!

digitall commented Feb 14, 2023

Uh oh!

elasota commented Feb 14, 2023

Uh oh!

digitall commented Feb 14, 2023

Uh oh!

digitall commented Feb 14, 2023

Uh oh!

digitall commented Feb 14, 2023

Uh oh!

elasota commented Feb 14, 2023

Uh oh!

elasota commented Feb 14, 2023

Uh oh!

digitall commented Feb 14, 2023

Uh oh!

digitall commented Feb 19, 2023

Uh oh!

sev- left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

elasota commented Feb 20, 2023

Uh oh!

sev- left a comment

Choose a reason for hiding this comment

Uh oh!

sev- Feb 21, 2023

Choose a reason for hiding this comment

Uh oh!

elasota Feb 21, 2023

Choose a reason for hiding this comment

Uh oh!

sev- Feb 21, 2023

Choose a reason for hiding this comment

Uh oh!

sev- commented Feb 21, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

elasota commented Feb 13, 2023 •

edited

Loading