Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bare UNLOCK produces no response #16

Open
apthorpe opened this issue Apr 14, 2021 · 3 comments
Open

Bare UNLOCK produces no response #16

apthorpe opened this issue Apr 14, 2021 · 3 comments

Comments

@apthorpe
Copy link

I'm not sure this is of interest for your goals of preserving the original gameplay but here goes:

If you enter a bare verb (single word) for a verb that expects an accompanying noun, you generally get a response like You don't have it with you. or That would be a neat trick.

UNLO (unlock) doesn't produce any output. Hitting return will give you the expected I didn't get that!! response. The game isn't hung but you'd never know from looking at it.

I'm not sure if this is a bug in the transcription or just an oversight in the original coding. I tested this on the patches branch, latest commit (ca1aac9) with and without asa. I'm building with gfortran 10.2.0 on both OSX and Windows 10, and 9.3.0 on Linux; I see the same behavior on all three.

tl; dr This came up during testing while modernizing the code from F66 (with a sprinkling of F77) to F2018. Eleven bare verbs were causing segfaults by setting ACTION(2) to zero and passing it upstream to main. In main, ACTION(2) is aliased to OBJECT and in some cases this is used to index an array. Fortran array indices start at 1 (unless otherwise changed but that's F90 onward). I believe legacy behavior is to return a 0 or blank for an index of zero which might be why the code isn't segfaulting (I may have broken something while refactoring fread and input) The other bare verbs that I've seen segfault are

         11 T/TAKE
         12 DROP
         17 THRO
         20 SHOW/WAVE
         24 DRIN
         30 UNLO
         32 LIGH
         34 EXTI
         38 WATE
         39 POUR
         49 FILL

In your unmodified code, the only issue I found was with UNLO; the others seem ok.

@apthorpe
Copy link
Author

A little more info: If you start a fresh game and enter UNLO (with no noun), you come out of INPUT() with the following state:

  • ACTION(1) = 30 "UNLO"
  • OBJECT -> ACTION(2) = 0
  • ROOM = 1
  • DOOR(ROOM) = DOOR(1) = D(1) = -2

J gets set to ACTION(1) and the GOTO switchboard sends you to line label 130, the start of the "UNLO" resolution fragment.

OBJECT is set to 47 "DOOR" and we jump to line label 271, the middle of the "OPEN" resolution fragment.

Since DOOR(ROOM) is -2, nothing is printed and we're directed back line label 25, the INPUT() routine where we started.

This all seems to match the original code in the PDF so it's not a transcription issue.

tl;dr As to the segfaults I'm seeing, they are reproducible by enabling -fcheck=all (specifically -fcheck=bounds). Segfaults magically go away when bounds-checking is disabled. At least that suggests what guard code is necessary in order to maintain the original behavior while enforcing array boundaries. If you're curious, my fork is at https://gitlab.com/apthorpe/Castlequest and I'm actively working on the cmakify2 branch.

@Quuxplusone
Copy link
Owner

Neat — thanks for the writeup and diagnosis. As you surmise, I plan to leave the code as-is since it's not a transcription error and not a fatal bug (in the sense of "fatal to the player's experience," even if it is technically undefined behavior).

If I ever get around to porting to C, I'll definitely want to plug these holes in some aesthetically pleasing way. I did a lot of that kind of thing with my ports of Adventure variants. Sample anecdote: Quuxplusone/Advent@39f13ab

I believe legacy behavior is to return a 0 or blank for an index of zero which might be why the code isn't segfaulting

I suspect that gfortran's default behavior is the same as if you said arr[-1] in C: you just grab whatever's in that particular memory. Most likely it happens to be zero, or maybe it's some garbage like 238972375 which doesn't compare equal to any other object, or very rarely maybe you do get bizarre behavior. In C we say "may cause demons to fly out of your nose."

@apthorpe
Copy link
Author

If we were really bored, we could look up the behavior of the OS/360 Fortran H Extended compiler (bitsavers.org is a godsend) though honestly, who knows what options they might have passed along when building this in 1980. This code may be working out of sheer luck. :)

I doubt that anyone would actually come across this in normal play so maybe the resolution is just to eventually document this as known and expected behavior. It's less for users than as a reminder of what to revise in an updated version.

This is actually a pretty interesting edge case. It's not exactly a bug - it's not game-breaking and it's not an error with an obvious solution. It's more of an oversight or inconsistency in the UI. It's likely not intended behavior, but it's also not clear what the behavior was intended to be.

Also, if you're interested I can send you a minimal PR to patch out the out-of-bounds array accesses in main to allow bounds checking while preserving original behavior. It's about 9 lines of IF(OBJECT < 1) GOTO ... scattered around. I've already patched it in my fork and there's no reason you should duplicate that tedious effort. It's maybe 5 minutes additional work on my part; writing this took longer...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants