ADL: HIRES5: Crash printing an apparently corrupt string

[eriktorbjorn]

This is about the version of the game included in the Roberta Williams Anthology.

When dumping the Time Zone scripts, using the "dump_scripts" debug command, I noticed that the following files (at least) contained corrupted strings:

• 032-031.ADL
• 040-073.ADL

The latter appears to be in a part of the game I haven't yet reached (way into the final time zone?), but the first one I have reached. Here's a quick walkthrough to get there:

14. N. GO MACHINE. GO MACHINE. SIT. SET BLUE. SA. SET ORANGE. 1000AD. PUSH LEVER. LEAVE MACHINE. S. D. D. D. S. S. S. U. OPEN DOOR. GO DOOR. CLOSE DOOR.

You should now be in a dark room in the pyramid where you could have found the torch if you had gone a bit further.

Trying to walk N, S, E, W, or D will now crash the game with an "ERROR: Word wrapping failed!" message.

I have not checked if this also crashes in an Apple II emulator.

[eriktorbjorn]

I found the second place. It happens if you try to lock this gate:

Which probably isn't something you ever need to do. This is, as I said, very late into the game, so if I do try to make a minimal walkthrough for it, it will still be pretty lengthy.

[eriktorbjorn]

For the second case, it actually prints two messages:

IF
	ROOM == 73
	&& SAID(42/LOCK    , 65/GATE    )
	&& GET_ITEM_ROOM(46/KEY     /A SKELETON KEY) == CARRYING
	&& VARS[13] == 1
THEN
	PRINT(172/<corrupted string>)
	PRINT(204/O.K. )
	VARS[13] = 0
END

So maybe something like this would be enough? I've never used SharedPtr myself, but it seems to work for me:

diff --git a/engines/adl/adl.cpp b/engines/adl/adl.cpp
index 79f55f782f4..c60be6419c3 100644
--- a/engines/adl/adl.cpp
+++ b/engines/adl/adl.cpp
@@ -444,6 +444,15 @@ Command &AdlEngine::getCommand(Commands &commands, uint idx) {
        error("Command %d not found", idx);
 }

+void AdlEngine::removeMessage(uint idx) {
+       if (_messages[idx]) {
+               _messages[idx].reset();
+               return;
+       }
+
+       error("Message %d not found", idx);
+}
+
 void AdlEngine::checkInput(byte verb, byte noun) {
        // Try room-local command list first
        if (doOneCommand(_roomData.commands, verb, noun))
diff --git a/engines/adl/adl.h b/engines/adl/adl.h
index a749c65a690..cea2313c8d5 100644
@@ -297,6 +297,7 @@ protected:
        void readCommands(Common::ReadStream &stream, Commands &commands);
        void removeCommand(Commands &commands, uint idx);
        Command &getCommand(Commands &commands, uint idx);
+       void removeMessage(uint idx);
        void checkInput(byte verb, byte noun);
        virtual bool isInputValid(byte verb, byte noun, bool &is_any);
        virtual bool isInputValid(const Commands &commands, byte verb, byte noun, bool &is_any);
diff --git a/engines/adl/hires5.cpp b/engines/adl/hires5.cpp
index 8a14b0ab24f..1f1b9de7d89 100644
--- a/engines/adl/hires5.cpp
+++ b/engines/adl/hires5.cpp
@@ -371,6 +371,14 @@ void HiRes5Engine::applyRegionWorkarounds() {
                // of the script.
                removeCommand(_roomCommands, 0);
                break;
+       case 40:
+               // Locking the gate prints a missing message, followed by
+               // "O.K.". Maybe there was supposed to be a more elaborate
+               // message, in the style of the one printed when you unlock
+               // the gate. But "O.K." should be enough, so we remove the
+               // first one.
+               removeMessage(172);
+               break;
        default:
                break;
        }

[eriktorbjorn]

The second case is worse since it's just prints one message, presumably telling you that you've died:

IF
	ROOM == 31
	&& SAID(11/N       , 60/        )
THEN
	PRINT(29/<corrupted string>)
	RESTART_GAME()
END

Which could be anything. It was probably supposed to tell you that you fell down the stairs, or something like that. But we know from Europe 50 BC that it could have been something more elaborate, like "THE CRAZY PEOPLE OF THE LABYRINTH SNEAK UP ON YOU AS YOU WANDER AIMLESSLY IN THE DARK, AND HACK YOU INTO SEVERAL TINY PIECES."

If we want to substitute our own message, one of my favorites comes from the very first text adventure I ever played: "Du dog nyss under mystiska omständigheter!" (translated: "You just died under mysterious circumstances!") :-)

But it would probably be more honest to just go with "INVALID STRING" or something like that.

[eriktorbjorn]

Maybe the first one - and, come to think of it, the second one as well - can be addressed simply like this:

diff --git a/engines/adl/adl_v2.cpp b/engines/adl/adl_v2.cpp
index c8a76c3e46d..91a79d28c48 100644
--- a/engines/adl/adl_v2.cpp
+++ b/engines/adl/adl_v2.cpp
@@ -364,6 +364,9 @@ Common::DataBlockPtr AdlEngine_v2::readDataBlockPtr(Common::ReadStream &f) const
        if (track == 0 && sector == 0 && offset == 0 && size == 0)
                return Common::DataBlockPtr();

+       if (track == 255 && sector == 255 && offset == 255 && size == 255)
+               return Common::DataBlockPtr();
+
        adjustDataBlockPtr(track, sector, offset, size);

        return _disk->getDataBlock(track, sector, offset, size);

That means nothing will be printed, because loadMessage() falls back to printing an empty string. That could be changed, but we don't want to print any additional message for locking the gate, I think?

[somaen]

Would be good to resolve this for the 2.9.0 release.

[bluegr]

I have tested the issue and I can confirm that eriktorbjorn's newest patch fixes it. Since it's just these two place, and the patch catches all such cases, it looks good to merge.

[eriktorbjorn]

To avoid confusion, I should point out that waltervn has a branch with ADL fixes. Perhaps someone who knows the engine better than I do could take a lot at those?

​https://github.com/waltervn/scummvm/commits/adl-fixes/