ILLUSIONS: Duckman - Crash after option menu "times out"

[raziel-]

ScummVM 2.2.0git (Jun 28 2020 09:39:26)
Features compiled in: Vorbis FLAC MP3 RGB zLib MPEG2 Theora AAC A/52 FreeType2 FriBiDi JPEG PNG cloud (servers, local)

Crash after intro has played and Option menu "times out".
Sorry for not being clearer, i don't know if at that point a demo should play or something else should happen, but the Option text (Start New Game etc.) is removed when it happens.

Steps to reproduce:

1. Start Duckman
2. Let the credits roll completely until you reach the Options (Start New Game etc.)
3. Let the game sit there until the second Duckman animation has played (where he rolls up a poster of presumably James Dean)
4. Duckman will make an angry face, the options will vanish, the cursor will jump to the top left and then the crash happens.

This is what i get in the console:
User picked target 'duckman' (engine ID 'illusions', game ID 'duckman')...

Looking for a plugin supporting this target... Illusions

WARNING: Unsupported Text stream detected!
WARNING: Unsupported Text stream detected!
_midiMusicCount: 18; midiMusicOffs: 00000010
WARNING: makeWAVStream: Trying to play a WAVE file with an incomplete PCM packet!
WARNING: makeWAVStream: Trying to play a WAVE file with an incomplete PCM packet!
WARNING: makeWAVStream: Trying to play a WAVE file with an incomplete PCM packet!
MidiPlayer::play(000B000D)

If more information is needed, just let me know.

Crashlog and serial log attached.
Sorry for the incomplete report, but i have neither found a "Duckman" entry in the "Game" field nor a "Engine: Duckman" entry in the "Component" field.

Duckman (Windows/English)

AmigaOS4 - PPC - SDL - BE
gcc (adtools build 8.3.0) 8.3.0

[raziel-]

Actually you can skip the intro with SPACE and start right at the options.

And i'm not sure if that might be of help, but here are the last two opcode outputs gathered with -d9:

execOpcode([00020341] 56) opStartTalkThread
ARG_UINT32(objectId = 00040008)
ARG_UINT32(talkId = 000F00E1)
ARG_UINT32(sequenceId1 = 00060042)
ARG_UINT32(sequenceId2 = 00060016)
Starting talk thread

execOpcode([00020341] 2) opSuspend
VoicePlayer::cue(A00A040)
000206D8 000F00E1 [What the hell are you starin' at?]

There is no "talking" and no text output (see screenshot)

[sev-]

Does it still happen? For me it plays a video and then loops back.

[raziel-]

Yep, same place, same crash (with a fresh build from today)

[sev-]

Any way of producing at least some kind of backtrace or name of the method where it crashes?

[raziel-]

The attached crashlog with stacktrace isn't sufficient then, i guess? :-(

Sorry, gdb is totally broken here, it will crash when running scummvm from within it, not even the launcher window will come up, so unusable, i'm afraid.

I'm guessing here it has to do with this issue:
https://bugs.scummvm.org/ticket/11236

BE issue here aswell, i suppose

[sev-]

Could you please put a debug output to getCharWidth(uint16 c) function in engines/illusions/textdrawer.cpp:211 like the following

int16 TextDrawer::getCharWidth(uint16 c) {
	warning("C: %d", c);
	return _font->getCharInfo(c)->_width + _font->_widthC;
}

Run the game and provide me with the output near the crash.

[raziel-]

Will do asap

[raziel-]

@sev-

The second to last number (C: 32!) is when the cursor changes from the hand icon to something else (which i cannot make out, because it is teleported to the top left) and the last (exceptionally high) number (C: 22272!) is when it crashes.

User picked target 'duckman' (engine ID 'illusions', game ID 'duckman')...

Looking for a plugin supporting this target... Illusions

WARNING: Unsupported Text stream detected!
WARNING: Unsupported Text stream detected!
_midiMusicCount: 18; midiMusicOffs: 00000010
WARNING: makeWAVStream: Trying to play a WAVE file with an incomplete PCM packet!
WARNING: makeWAVStream: Trying to play a WAVE file with an incomplete PCM packet!
WARNING: makeWAVStream: Trying to play a WAVE file with an incomplete PCM packet!
MidiPlayer::play(000B000D)
WARNING: C: 32!
WARNING: C: 83!
WARNING: C: 116!
WARNING: C: 97!
WARNING: C: 114!
WARNING: C: 116!
WARNING: C: 32!
WARNING: C: 78!
WARNING: C: 101!
WARNING: C: 119!
WARNING: C: 32!
WARNING: C: 71!
WARNING: C: 97!
WARNING: C: 109!
WARNING: C: 101!
WARNING: C: 76!
WARNING: C: 111!
WARNING: C: 97!
WARNING: C: 100!
WARNING: C: 32!
WARNING: C: 83!
WARNING: C: 97!
WARNING: C: 118!
WARNING: C: 101!
WARNING: C: 100!
WARNING: C: 32!
WARNING: C: 71!
WARNING: C: 97!
WARNING: C: 109!
WARNING: C: 101!
WARNING: C: 79!
WARNING: C: 112!
WARNING: C: 116!
WARNING: C: 105!
WARNING: C: 111!
WARNING: C: 110!
WARNING: C: 115!
WARNING: C: 81!
WARNING: C: 117!
WARNING: C: 105!
WARNING: C: 116!
WARNING: C: 32!
WARNING: C: 71!
WARNING: C: 97!
WARNING: C: 109!
WARNING: C: 101!
WARNING: C: 32!
WARNING: C: 22272!

[raziel-]

This is still crashing with
ScummVM 2.3.0git (Apr 5 2021 14:24:06)
Features compiled in: Vorbis FLAC MP3 RGB zLib MPEG2 Theora AAC A/52 FreeType2 FriBiDi JPEG PNG cloud (servers, local) TinyGL OpenGL

Isn't the list printed font width freakischly big in my debug log?
Maybe that is where it crushes?
Anything i could do to prevent it growing so big?

Thank you

[dwatteau]

I can confirm the same bug on Debian 11 PowerPC.

I've attached some gdb and debug logs, if this helps understanding what is happening.

@sev- You can ping me over Discord (as we did for Full Pipe) if you want to do some new big-endian tests over here.

[dwatteau]

Hi,

The diff that I've just attached appears to fix the problem here.

@raziel-: Could you apply this patch on a new AmigaOS build of yours, and test it ? If it appears to be good to you, I'll submit a PR for it.

Thanks!

[raziel-]

@dwatteau

With your changes in place i'm unfortunately still getting the crash at the same position.

Though i manually edited the files from your diff...not sure if sgit supports merging in diffs, so there might have been a mistake on my side, but i'm pretty sure i covered all files and changes.

[dwatteau]

@raziel-

As yes, there were at least two mistakes of mine in my patch above, sorry.

I've pushed a new version on the fix/illusions-fixes-for-big-endian-systems branch of my ​https://github.com/dwatteau/scummvm fork (using this branch in particular is important).

Could try rebuilding from that fork and checkout'd branch, please? Please *don't* build with dynamic plugins if that's possible, because it seems that dynamic plugins on AmigaOS4 make the crashlog even less useful, unfortunately.

On my side, the patch still unbreaks the game on Debian 11 powerpc and OpenBSD/macppc. (I tried testing a PS3 build too, but I couldn't get any working ScummVM binary with the current scummvm/dockerized-toolchains:ps3 environment).

Thanks!

[raziel-]

@dwatteau

Will do asap.

Thank you very much for trying to fix this, greatly appreciated.

I'll do a static build, one engine is not a problem.

[raziel-]

@dwatteau

With your above fork compiled i get the same crash at the same position in the intro.

crahslog attached

log window:
User picked target 'duckman' (engine ID 'illusions', game ID 'duckman')...

Looking for a plugin supporting this target... Illusions

WARNING: Unsupported Text stream detected!
WARNING: Unsupported Text stream detected!
_midiMusicCount: 18; midiMusicOffs: 00000010
WARNING: makeWAVStream: Trying to play a WAVE file with an incomplete PCM packet!
WARNING: makeWAVStream: Trying to play a WAVE file with an incomplete PCM packet!
WARNING: makeWAVStream: Trying to play a WAVE file with an incomplete PCM packet!
MidiPlayer::play(000B000D)

[dwatteau]

@raziel-: Thanks for your new test!

Indeed, there are various problems in my previous patches, but now I've think I've found the real culprit. And of course it was a one-liner :)

Forget my Github fork, just go back to the official ScummVM sources, but apply this change in engines/illusions/resources/talkresource.cpp:

 #if defined(SCUMM_BIG_ENDIAN)
-       for (byte *ptr = (byte *)_text; ptr != _tblPtr; ptr += 2) {
+       for (byte *ptr = (byte *)_text; *ptr != 0; ptr += 2) {
                WRITE_UINT16(ptr, SWAP_BYTES_16(READ_UINT16(ptr)));
        }
 #endif

and I think that should be it.

The reason is that, on big-endian systems, the original line above reversed the internal wide strings until reaching _tblPtr, but TalkEntry::load() is called for every piece of text in a talk entry, and _tblPtr is way past all the text parts:

TalkEntry::load() _talkId: 000F00C8; textOffs: 0000065C; tblOffs: 0000285A; voiceNameOffs: 00003C04
TalkEntry::load() _talkId: 000F00C9; textOffs: 000006E8; tblOffs: 00002896; voiceNameOffs: 00003C0C
TalkEntry::load() _talkId: 000F00CA; textOffs: 000007A8; tblOffs: 000028E8; voiceNameOffs: 00003C14
TalkEntry::load() _talkId: 000F00CB; textOffs: 0000080C; tblOffs: 0000290E; voiceNameOffs: 00003C1C

so, every piece of text was reversed multiple times, sometimes being in native endianness, and sometimes not, hence the crashes (that's why c appeared as 22272 on big-endian PowerPC instead of 87 on Intel: 0x0057 vs. 0x5700). Well, at least, that's what I understood from my debugging session :)

I'll continue my testing a bit, but I think that's the fix I'm going to submit.

[raziel-]

What line would that be?

[dwatteau]

Around line 55, in the TalkEntry::load() function at the top of engines/illusions/resources/talkresource.cpp.

diff --git a/engines/illusions/resources/talkresource.cpp b/engines/illusions/resources/talkresource.cpp
index 83ff98a9a7..776f89e15a 100644
--- a/engines/illusions/resources/talkresource.cpp
+++ b/engines/illusions/resources/talkresource.cpp
@@ -52,7 +52,7 @@ void TalkEntry::load(byte *dataStart, Common::SeekableReadStream &stream) {
 		_talkId, textOffs, tblOffs, voiceNameOffs);
 
 #if defined(SCUMM_BIG_ENDIAN)
-	for (byte *ptr = (byte *)_text; ptr != _tblPtr; ptr += 2) {
+	for (byte *ptr = (byte *)_text; *ptr != 0; ptr += 2) {
 		WRITE_UINT16(ptr, SWAP_BYTES_16(READ_UINT16(ptr)));
 	}
 #endif

(n.b. "ptr" also becomes "*ptr", don't forget that added symbol.)

[raziel-]

Because there is no such line in the main tree

[dwatteau]

It's there:
​https://github.com/scummvm/scummvm/blob/master/engines/illusions/resources/talkresource.cpp

Please wait a few days for the fix to be committed to the official repo, otherwise.

[raziel-]

I added it manually, will give feedback soon :-)

[dwatteau]

I think that you're maybe still using my earlier Github fork, instead of going back to the official ScummVM Git repository, as I said above.

Go back to the official ScummVM Git repository, and apply my one-liner line on top of that. Otherwise, it's still not going to work.

[raziel-]

I have a full updated checkout, just done.
Then added the lines like in your last post...am I still missing something?

[dwatteau]

You just need to delete the previous local repo (because I think you still have mine), re-clone the official github.com/scummvm/scummvm repository from scratch and apply the diff above.

I don't know how this translates in AmigaOS, but in Unix it's something along the lines of:

rm -rf scummvm
git clone -b master https://github.com/scummvm/scummvm
cd scummvm
sed -i -e 's/ptr != _tblPtr;/*ptr != _0;/' engines/illusions/resources/talkresource.cpp
./configure --disable-all-engines --enable-engine=illusions --enable-debug
make

[raziel-]

Yes, I know how to clone :-)

I meant that I already have a clean master tree, cloned from a few days ago.
This I simply updated with fetch and merge a few minutes ago and then applied your change (because the ifdef big Endian was not there.

[dwatteau]

OK, but that's strange, because the SCUMM_BIG_ENDIAN line does exist in the official and up-to-date repo:

​https://github.com/scummvm/scummvm/blob/master/engines/illusions/resources/talkresource.cpp

and it's been there since 2020-03-01. So if you don't have it, something is wrong.

Aren't you using sgit? It's advertised as a prototype that's likely buggy and not fully working (​http://aminet.net/package/dev/misc/sgit). So I'd suggest really doing a fresh reclone instead of anything else.

Anyway, I can't help you more there, sorry...

[raziel-]

@dwatteau

Confirmed fixed in both game and demo, thank you very much :-)

I'm going to open one other issue though.
The subtitles are vanishing far too fast.
Not conforming to the spoken line and, with long text, even too fast to read.

[dwatteau]

@raziel-: Thanks for your test! Cool if it works, now.

I've submitted an official PR here:
​https://github.com/scummvm/scummvm/pull/3125

As for the subtitles, yes they're way too fast by default, although if you press Escape and go to the Options menu, there's an option to change their speed. I also found some other bugs in the game, but they're not big-endian related so I'll open new tickets for that as well.

[dwatteau]

Hi,

PR 3125 has been merged, and the problem appears to be have been solved.

I believe that this issue can be closed, then. Thanks.

[raziel-]

@dwatteau

You may close tickets too :-)