angstsmurf / spatterlight

Updated fork of Spatterlight
GNU General Public License v3.0
105 stars 5 forks source link

Scott Adams terp fails to LOOK after room-swap #54

Closed ahope1 closed 2 years ago

ahope1 commented 2 years ago

Testing Spatterlight 0.9.0 on macOS Big Sur 11.6. I think the Scott Adams terp fails in the same way that ScottKit fails in the following Issue report:

https://github.com/MikeTaylor/scottkit/issues/42#issue-786364764

To reproduce the issue in Spatterlight, you can load the attached test file cases.DAT (which I recently created in order to test certain edge-cases in Scott Adams terps), and type in the command AR2.

A shell/commandline version of ScottFree that I compiled myself in the Terminal app will load the .DAT file and will behave correctly — i.e. when you enter the command AR2 the room description will change briefly and then revert to what it was before. But when you enter AR2 in Spatterlight the room description doesn't seem to change at all (which is incorrect behaviour).

cases.DAT.zip

angstsmurf commented 2 years ago

Thanks for reporting this!

It is not surprising that this happens. Spatterlight limits how often it updates the screen in order to reduce flickering, and here the game tries to do deliberate flickering.

I've done a lot of work on the ScottFree interpreter lately, even if nothing is ready for release, and this is definitely something I will take a look at.

ahope1 commented 2 years ago

Don't know if it's of any use to you, but in my prototype Scott Adams terp ("BeebScott") written in BBC BASIC for the 8-bit BBC Micro computer, I've buffered all the top-window (room-description) updates and only allow the room-description to be updated when it's necessary. I've tested the algorithm on a few converted Scott Adams TRS-80 .DAT games, and it seems to work.

In my code, B% is the flag that determines whether the room-description should be updated (which is done in the procedure PROCdRM). Here's the line that enables the AR2 command in my test case to work:

https://github.com/ahope1/BeebScott/blob/9ab9206dec00db60ff909c37cd2db594dc5b9d43/terp.bas#L289

Here's the test game, cases.DAT, converted and loaded into BeebScott, running in a BBC Micro emulator:

http://bbcmicro.co.uk//jsbeeb/play.php?autoboot&disc=https://raw.githubusercontent.com/ahope1/BeebScott/master/test/cases.ssd

angstsmurf commented 2 years ago

That's interesting. I'm afraid I'm not fluent in BBC BASIC.

Is there a simple way to explain how your implementation determines when an update of the top window is necessary?

ahope1 commented 2 years ago

In my interpreter there are functions ("command handlers") that may move an object into or out of the current room, and there are other functions that may move the player to another room, and I've implemented those functions in such a way that if an object is indeed moved into or out of the current room, or if the player is in fact moved to a different room, then the "Top-Window Update Pending" flag ("B%") is set. Then, just before control is returned to the user-command prompt, the status of that flag is checked, and if (and only if) the flag is set, the top window is updated.

Additionally, whenever the function that handles the DELAY command (command 88) is called, that function checks the status of the Top-Window Update Pending flag and if the flag is set then the function immediately updates the top window without waiting for the flag-check that happens just before control is returned to the command prompt.

The code that updates the top window also clears the flag.

angstsmurf commented 2 years ago

I experimented with this, and the simplest solution seems to be to update the room description and have a small pause whenever the game automatically changes the room (and not when a player-initiated GO action changes the room.)

I seem to recall that one of the original Scott Adams uses this effect, is that right?

Also, this is off-topic and should perhaps be its own issue, but another one of the tests in cases.DAT made me curious: The GET FOX test says: "Now you still shouldn’t be able to GET FOX _ because an explicit GET FOX action hasn’t been coded!". But I would have thought that unless there is an explicit action to prevent this, the player would be able to take any item that has an AutoGet word.

EDIT: Right, I found the relevant discussion at the Infiction.org forum. I wonder if there is a simple way to fix this in ScottFree without breaking anything else.

ahope1 commented 2 years ago

I seem to recall that one of the original Scott Adams uses this effect, is that right?

I thought at least one did, but I've now "decompiled" most of them with ScottKit, searching for swap_room and swap_specific_room commands, in combination with pause, but none of the original Scott Adams games seem to do exactly what the AR2 example in cases.DAT does. (Btw, the AR2 example was taken from the Adventure System manual.)

Also, this is off-topic and should perhaps be its own issue, but another one of the tests in cases.DAT made me curious: The GET FOX test says: "Now you still shouldn’t be able to GET FOX _ because an explicit GET FOX action hasn’t been coded!". But I would have thought that unless there is an explicit action to prevent this, the player would be able to take any item that has an AutoGet word.

EDIT: Right, I found the relevant discussion at the Infiction.org forum. I wonder if there is a simple way to fix this in ScottFree without breaking anything else.

I don't know. I've scanned through the ScottFree code in the past, but I'm not very good at C unfortunately.

angstsmurf commented 2 years ago

I've looked into this, and it is tricky. Many games, in particular The Sorcerer Of Claymorgue Castle, do a lot of room-swapping that they expect to be invisible to the player. We also don't want delays every time the player goes somewhere.

I'm currently experimenting with a flag that is set after a room swap (which includes commands 80, 87, and 54) and is reset at the start of every turn, and having a delay after changing the player room (with command 54) if this flag is set, i.e. if there has already been at least one room swap that turn.

This makes both of the room swaps in AR1 visible to the player. I'm just worried that it will cause problems in some other game.

angstsmurf commented 2 years ago

This should be fixed in release 0.9.1.

ahope1 commented 2 years ago

Interesting! AR2 now seems to work as expected, but, as you implied, AR1 behaves differently in Spatterlight 0.9.1 than it does in ADVENTUR/CMD. See the attached screen recording, where AR1 causes the top window to update so fast in ADVENTUR/CMD that the human eye is unable to read the successive room-descriptions properly. (ScottFree is even faster!) I'm not sure if any games actually use this AR1 "quick change" effect, however.

https://user-images.githubusercontent.com/11441088/151468524-51f981b1-4943-4cf5-b9c9-e787d51f74d2.mov

angstsmurf commented 2 years ago

On a modern computer, that will be too fast to be noticeable (and for the screen refresh to catch it), so there has to be an artificial delay, but perhaps I made it too long. I've mostly compared it to ZX Spectrum games which update the room description slower because they draw pictures as well.

ahope1 commented 2 years ago

I think that in the AR1 example the intention is that the top-window updates should be too fast to be read individually — or, ideally (as in ScottFree), invisible.

I think a similar effect occurs (at least sometimes) when you enter a dark room: the full room-description is displayed on screen and then it gets cleared and overwritten with “It’s too dark — I can’t see”. Even in ADVENTUR/CMD that happens too quickly for the eye to see (which is the correct behaviour). I think. I’ll try and pin down an example later.

angstsmurf commented 2 years ago

Right, so I guess it would be better if AR1 showed no changes at all, like it did originally.

ahope1 commented 2 years ago

I only recently found this comment about ScottFree's top-window update algorithm in the ScottKit repo. Maybe useful?:

https://github.com/MikeTaylor/scottkit/issues/3#issuecomment-337858358

angstsmurf commented 2 years ago

Thanks for the link!

This is somewhat orthogonal to the bug in question, but there seems to be a general conflict between split-screen mode, where we want to make sure that the info in the top window always is accurate, and what they call the conversational style, where you want to minimise the number of repeated room descriptions.

Currently the non-split-screen mode can't actually be activated in Spatterlight, but it is used for the transcript output, so it really needs more testing.

ahope1 commented 2 years ago

Many games, in particular The Sorcerer Of Claymorgue Castle, do a lot of room-swapping that they expect to be invisible to the player.

The Count seems to do that, but the effect isn't exactly "invisible"! Perhaps the TRS-80 just wasn't a fast enough machine to be able to process SLEEP quickly enough for the "closet" text to be invisible to the human eye:

https://user-images.githubusercontent.com/11441088/155823324-1e136a85-e253-4811-b591-7de5bd900936.mov

angstsmurf commented 2 years ago

I tried to fix this once again in release 0.9.2.

angstsmurf commented 2 years ago

Closing this for now. Let me know if it still doesn't work as it should.

ahope1 commented 2 years ago

All looks good!

EDIT: Well, there's still the issue of the ”ANY” noun, exposed by cases.DAT. But then BeebScott too is guilty of implementing the same expedient (“shortcut”), as is ScottFree.

EDIT2: Correction -- I think Spatterlight actually behaves in the same way as ADVENTUR/CMD with regard to the aforementioned "ANY" noun issue. So you're all good.