Bocfel: Crashes when robot finds kitten

[angstsmurf]

When you finally find the kitten in the Z-machine port of Robot Finds Kitten, Gargoyle just crashes instead of displaying the victory animation, which is kind of a bummer if you've just spent hours looking for it on higher difficulties. It seems to work in any other Z-machine interpreter I've tried.

The problem seems to manifest on line 2024 in bocfel/screen.c, in the read_handler() function. Bocfel asserts with fatal error: too many preloaded characters: 157 when max is 5 (pc = 0x4906).

I have a hunch that the solution can be found in the Z-Machine Standards Document, in the documentation of the @read opcode, or perhaps in the Frotz implementation of the same opcode, but I haven't been able to work it out.

I also don't quite understand the line (704) in the Robot Finds Kitten Inform source (available from the Ifdb page linked above) that seems to cause this: @aread junk 0 10 pause -> junk; You don't input any lines into this game, so why use @aread at all, rather than @read_char?

Remember to set difficulty to 1 when testing this!

EDIT: Right, it seems like the "junk" variable in the Inform source is used uninitialised, which I suppose in Inform means that it is set to 0, so when the interpreter tries to read bytes 0 and 1 from the text-buffer given by the first argument, it just gets random stuff from address 0. Frotz does not seem to be doing much checking at all, so it is just lucky, I guess. If I add guards for (zargs[0] == 0) to the code, the animation runs without crashing, but then we arrive at a different bug: the kitten and robot are not erased, so they leave trails when moving toward each other.

EDIT 2: The reason for the trails is that the game calls @erase_window -1, but because of the way Bocfel handles size changes of the upper window, that is, only after user input, the window is never cleared. This should probably be an issue of its own.

[cspiegel]

I think adding the zero check to read is worthwhile, even if it only affects one game (there's already code that affects single games anyway).

As for the bug regarding trails being left, it's because Bocfel won't consider read/read_char to have “seen input” if they timed out or the routine returns true (i.e. "cancel input"), and the input used by Robot Finds Kitten is only used for the timeout side effect, and its routine always returns true to cancel. This could theoretically be tested by hitting ENTER while the kitten and robot approach each other: that would generate a "seen input" event and clear the screen. In reality it crashes because junk is 0, and read will try to store the typed-in values at an address based on junk. So, passing 0 as the first argument to read causes cascading problems in this particular game. I'm thinking about making read 0 ... be a shorthand for pausing without reading input, since it's undefined behavior anyhow and this would at least fix Robot Finds Kitten.

I'm attaching a simple patch which unconditionally sets saw_input to true if an input routine was called, regardless of how it ends. I tested this with Trinity and Curses, and the quote boxes seem to work fine. If you have a minute, could you give it a test?

[angstsmurf]

Well, I tried out the patch on macOS, and the quote boxes in Trinity and Curses still work.

Of course, Robot Finds Kitten still crashes because zargs[0] is 0, so I added a simple check (changed line 2021 in screen.c to if(zversion >= 5 && text > 0)) and then everything works as it should. Robot and kitten happily ever after.

[cspiegel]

After further review, this patch has an unwanted effect in Robot Finds Kitten: starting the game (hitting "F") doesn't cause the picture of the cat and robot to disappear until a key is pressed.

I'll work on an improved (i.e. correct) solution.

[cspiegel]

Actually, that's not true: I committed a slightly tweaked version of this patch to my local codebase and that tweaking caused the problem. I'll go back to the version I attached here and all will hopefully be right.

[cspiegel]

I fixed the "zargs[0] is 0" bug using the same idea I had earlier (0 means sleep) but without making it global, i.e. without having it apply to all games. Instead of special-casing the read opcode, I used the patching system to patch the story file so that it uses read_char instead of aread. This'll be in the next release, but I'm attaching a patch if you want to apply it to your codebase in the meantime.

[angstsmurf]

Excellent! David Griffith in particular will be happy when RFK runs in Gargoyle.

[angstsmurf]

Closed by #401

[DavidGriffith]

Hi there! I had no idea there was a problem with this until @curiousdannii referenced patches.c while pondering a solution for Bureaucracy running too fast (https://gitlab.com/DavidGriffith/frotz/-/issues/234)