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

#58 - proposed changes #59

Merged
merged 2 commits into from
Jun 25, 2024
Merged

#58 - proposed changes #59

merged 2 commits into from
Jun 25, 2024

Conversation

AlexProudfoot
Copy link
Contributor

Proposed changes for issue #58.

@larsbrinkhoff
Copy link

Maybe this is the upstream for "Mainframe"/"MDL" Zork now: https://github.com/PDP-10/its/tree/master/src/lcf

CC @eswenson1; also #58

@AlexProudfoot
Copy link
Contributor Author

@larsbrinkhoff
I can also make my change requests on the PDP-10/its repo if that would help.

@larsbrinkhoff
Copy link

@AlexProudfoot, if it's not too much trouble, yes please!

(By the way, no need to make both an issue and a pull request for the ITS repository. The latter is enough by itself.)

@AlexProudfoot
Copy link
Contributor Author

AlexProudfoot commented Nov 5, 2023

@larsbrinkhoff
So, having made my first change to the contents of src/lcf, how do I rebuild Zork to check it?
I've tried just re-making from scratch and it doesn't seem to work.
I mean the Zork executable seems unchanged.

@larsbrinkhoff
Copy link

Re-making from scratch should work. Did you include clean in your make targets? The Makefile dependencies do not cover the ITS source files, unfortunately.

@AlexProudfoot
Copy link
Contributor Author

Hmmm. Remaking works. My solution to make the windows visible using Confusion doesn't work when using Muddle. I'll need to try something different.

@eswenson1
Copy link

@AlexProudfoot Can you explain how your change to add the window function when one tries to open that window is related to your comment about making windows visible under Muddle? I’m confused!

@AlexProudfoot
Copy link
Contributor Author

Sure. I was referring to a previous related change. If you start Zork and go north, you will be at “North of House”. If you then type “look at window”, the answer will be “I don’t see any window here.” Although there is a global object representing windows in the sides of the house and that object is a local-global in NHOUSE. This was fixed by #30. The response becomes “I don’t see anything special about the window.” The window object is now “visible” i.e. in scope. Now you can attempt to open the window and will notice the incorrect response that the window function fixes.

@AlexProudfoot
Copy link
Contributor Author

What I meant to say was that #30 doesn’t work when applied to https://github.com/PDP-10/its.

@AlexProudfoot
Copy link
Contributor Author

Hmmm. No success yet. I need to shorten the turnaround time on my experimental changes. How do I rebuild Zork from inside the PDP-10 simulation.

@eswenson1
Copy link

To recompile Zork, do:

:xxfile tty:_lcf;comp xxfile

then do:

:xxfile tty:_lcf;zork xxfile

@larsbrinkhoff
Copy link

For operating ITS in general, I'll point to this page: https://github.com/larsbrinkhoff/its-manual/wiki/HACTRN-basics

@AlexProudfoot
Copy link
Contributor Author

AlexProudfoot commented Nov 9, 2023

I just added a GOBJECT based on the HOUSEBIT object and that's not visible either. That is, the parser recognises the name but it's not in scope. I don't understand what's going on. Is there a limit to the number of GOBJECTs that can be declared?

@AlexProudfoot
Copy link
Contributor Author

Finished now @heasm66.

@eswenson1
Copy link

eswenson1 commented Nov 13, 2023

As you've said, the fix, as outlined here, doesn't work on ITS (MUDDLE) Zork. The reason is that the "NHOUS" room needs to have a " (<GET-OBJ "DWIND">)" in the list of objects for the room. If you add that, as well as the DWIND-FUNCTION and "DWIND", then the fix works on ITS Zork.

Here is the complete diff of DUNG:

ES>:srccom lcf;dung 355,lcf;dung 356

;COMPARISON OF DSK:LCF;DUNG 355 AND DSK:LCF;DUNG 356
;OPTIONS ARE    /3

**** FILE DSK:LCF;DUNG 355, 10-282 (50191)
         ["WINDO"]
**** FILE DSK:LCF;DUNG 356, 10-282 (50191)
         ["DWIND" "WINDO"]
***************

**** FILE DSK:LCF;DUNG 355, 10-286 (50233)
         <>>
**** FILE DSK:LCF;DUNG 356, 10-286 (50241)
         DWIND-FUNCTION>
***************

**** FILE DSK:LCF;DUNG 355, 14-21 (52571)
       ()
**** FILE DSK:LCF;DUNG 356, 14-21 (52591)
       (<GET-OBJ "DWIND">)
***************


:KILL  ZORK◊J
ES>

With these changes in place, I get:

zork↑K--Clobber Existing Job--
This Zork created November 13, 2023.
West of House
This is an open field west of a white house, with a boarded front door.
There is a small mailbox here.
A rubber mat saying 'Welcome to Zork!' lies by the door.
>n
North of House
You are facing the north side of a white house.  There is no door here,
and all the windows are barred.
>open window
The window cannot be opened.
>

@eswenson1
Copy link

Shouldn't the same thing be done for "SHOUS"? I get this:

>s
South of House
You are facing the south side of a white house. There is no door here,
and all the windows are barred.
>open window
I can't see any window here.
>look at windows
I can't see any windows here.
>

@AlexProudfoot
Copy link
Contributor Author

What happens if you try?

@eswenson1
Copy link

Adding the dwindow object to SHOUS object fixes it there too:

>s
South of House
You are facing the south side of a white house. There is no door here,
and all the windows are barred.
>look at windows
I see nothing special about the window.
>open windows
The window cannot be opened.
>

Change is:

:srccom lcf;dung 355,lcf;dung 356

;COMPARISON OF DSK:LCF;DUNG 355 AND DSK:LCF;DUNG 356
;OPTIONS ARE    /3

**** FILE DSK:LCF;DUNG 355, 10-282 (50191)
         ["WINDO"]
**** FILE DSK:LCF;DUNG 356, 10-282 (50191)
         ["DWIND" "WINDO"]
***************

**** FILE DSK:LCF;DUNG 355, 10-286 (50233)
         <>>
**** FILE DSK:LCF;DUNG 356, 10-286 (50241)
         DWIND-FUNCTION>
***************

**** FILE DSK:LCF;DUNG 355, 14-21 (52571)
       ()
**** FILE DSK:LCF;DUNG 356, 14-21 (52591)
       (<GET-OBJ "DWIND">)
***************

**** FILE DSK:LCF;DUNG 355, 14-32 (52954)
       ()
**** FILE DSK:LCF;DUNG 356, 14-32 (52991)
       (<GET-OBJ "DWIND">)
***************


:KILL  EMACS◊J
ES>

@AlexProudfoot
Copy link
Contributor Author

After making both changes, have you navigated to both rooms and obtained the same result. I ask because I thought GET-OBJ was exclusively used for local objects, not global ones.

@eswenson1
Copy link

eswenson1 commented Nov 13, 2023

Works fine. I can go to north or south of house and get the right behavior on “open window”. I went back and forth several times.

Feel free to try it out on ES yourself.

@eswenson1
Copy link

I ask because I thought GET-OBJ was exclusively used for local objects, not global ones.

As far as I know, GET-OBJ is used to get the pointer to any object on the global object list. And in order to have a room “contain” an object that can be referenced from the room, you need to put the object on the list of room objects. That allows the parser to find the object when you refer to that object.

@AlexProudfoot
Copy link
Contributor Author

AlexProudfoot commented Nov 14, 2023

That’s great news.

I have to admit to still being confused because the situation with all other RGLOBALs, the house for example, is that if it’s on the RGLOBAL list it’s also in scope and GET-OBJ is not needed.

Do you have an explanation for why the global window object is different (currently unique) in this respect?

@eswenson1
Copy link

I always thought that either the object had to be on the "objects in room" slot of the ROOM object (or in your inventory) in order to be used as a direct or indirect object in a command. If an object is neither in the room or in your inventory, you get the "I can't see any XXX there." message.

@AlexProudfoot
Copy link
Contributor Author

There also has to be a way of making “backdrop” objects. These are in scope in multiple rooms but are not separately listed in the room descriptions and may not be taken. That’s what the RGLOBAL section is for.

@eswenson1
Copy link

Yes, GOBJECT is a function for defining global objects (not necessarily in a single room or in inventory, and accessible as a direct or indirect object from multiple rooms), and that RGLOBAL was a means for specifying, by adding together bits (e.g. ,DWINDOW, ,HOUSEBIT, etc.) and then adding this bitmask to a ROOM to indicate that you could reference the global object "from this room". But it does seem a little more complicated than that, because you can access some global objects (e.g. "sailor") from any room without having an RGLOBAL reference in each room. However, I think for those kinds of objects, any verb used with them results in a word-specific function being invoked, as opposed to getting a "I don't see xxx here" message when you invoke a command that references the word. If no word-specific function is defined for the GOBJECT, however, you get "Nothing happens" as a default for whatever you try to do with the object.

@AlexProudfoot
Copy link
Contributor Author

Another problem is that, despite the naming, HOUSEBIT has nothing to do with bit masks. The item ,HOUSEBIT is the GVAL of the object called HOUSEBIT which evaluates to the address of the object. I can only assume that, in this context, <+ …> constructs a list of object addresses.

Have you tried your fixes in the interpreter or just in the compiled version?

@AlexProudfoot
Copy link
Contributor Author

Hey @eswenson1, here's an interesting result from trying to load zork into the muddle 5.5 interpreter.

Screenshot from 2023-11-17 12-32-16

It seems that the declaration of DWINDOW causes an overflow.

@AlexProudfoot
Copy link
Contributor Author

AlexProudfoot commented Nov 17, 2023

You were right @larsbrinkhoff. If I comment out DWINDOW, loading continues but there is not enough memory to finish loading DUNG.

*:mud55
MUDDLE 55 IN OPERATION.
LISTENING-AT-LEVEL 1 PROCESS 1
<FLOAD "run">$
prim loaded
/LSRTNS defs loaded
makstr loaded
tell-r loaded
act1 loaded
act2 loaded
act3 loaded
act4 loaded
disp1 loaded
parser loaded
melee loaded
rooms loaded
:$ FATAL ERROR AGC--NO CORE AVAILABLE TO SATISFY REQUESTS $

@eswenson1
Copy link

eswenson1 commented Nov 17, 2023

Well, it turns out there is a real limit to the number of GOBJECTs you can define. Each new one increases GLOHI by a factor of 2. So we have:

"random object" => 2
"free brochure" => 4
"cretin" => 8
"wish" => 16
"everything" => 32
"possessions" => 64
"valuables" => 128
"sailor" => 256
"set of teeth" => 512
"wall" => 1024
"granite wall" => 2048
"ground" => 4096
"lurking grue" => 8192
"pair of hands" => 16384
"breath" => 32768
"flyer" => 65536
"moby lossage" => 131072
"well" => 262144
"piece of rope" => 524288
"chute" => 1048576
"eastern wall" => 2097152
"western wall" => 2097152
"southern wall" => 2097152
"northern wall" => 2097152
"ladder" => 4194304
"bird" => 8388608
"white house" => 16777216
"tree" => 33554432
"Guardian of Zork" => 67108864
"compass rose" => 134217728
"dungeon master" => 268435456
"mirror" => 536870912
"panel" => 1073741824
"stone channel" => 2147483648
"eastern wall" => 4294967296
"southern wall" => 4294967296
"western wall" => 4294967296
"northern wall" => 8589934592
"water" => 17179869184

And then the definition of DWINDOW ("window") attempts to multiply that last value by 2 and gets an arithmetic overflow.

Simply put:

<SETG FOO 17179869184>◊
17179869184
<* ,FOO 2>◊

*ERROR*
OVERFLOW
*

But the fix doesn't add a new GOBJECT -- this overflow already exists (when you load the code interpreted). When you compile it and load compiled, the overflow is not caught. Presumably the value of GLOHI simply overflows without error.

Since we didn't add this GOBJECT, and the original code had it and worked, I don't think we should worry about the overflow.

@larsbrinkhoff
Copy link

larsbrinkhoff commented Nov 18, 2023 via email

@AlexProudfoot
Copy link
Contributor Author

AlexProudfoot commented Nov 18, 2023

Another problem is that, despite the naming, HOUSEBIT has nothing to do with bit masks. The item ,HOUSEBIT is the GVAL of the object called HOUSEBIT which evaluates to the address of the object. I can only assume that, in this context, <+ …> constructs a list of object addresses.

So, HOUSEBIT really is to do with a bit. Each global object is associated with a power of 2 starting at 1 and ending, in practice, at 35. Calculation of the next power of 2 produces an integer overflow.

I guess the problem doesn’t exist in Confusion because the integers have 64 bits as opposed to 40.

@heasm66 - Do you know how this problem was avoided in 32-bit Confusion?

@AlexProudfoot
Copy link
Contributor Author

AlexProudfoot commented Nov 18, 2023

Since we didn't add this GOBJECT, and the original code had it and worked, I don't think we should worry about the overflow.

It depends what you mean by “worked”. We are still left with the original problem of not being able to refer to the windows in NHOUSE and SHOUSE. It’s clear that the implementation of global objects will not correctly support this.

@AlexProudfoot
Copy link
Contributor Author

@eswenson1

As you've said, the fix, as outlined here, doesn't work on ITS (MUDDLE) Zork. The reason is that the "NHOUS" room needs to have a " (<GET-OBJ "DWIND">)" in the list of objects for the room. If you add that, as well as the DWIND-FUNCTION and "DWIND", then the fix works on ITS Zork.

Maybe this is the way to go after all. If it works for me, I can submit a pull request if you like.

@heasm66
Copy link
Owner

heasm66 commented Nov 18, 2023 via email

@eswenson1
Copy link

Since we didn't add this GOBJECT, and the original code had it and worked, I don't think we should worry about the overflow.

It depends what you mean by “worked”. We are still left with the original problem of not being able to refer to the windows in NHOUSE and SHOUSE. It’s clear that the implementation of global objects will not correctly support this.

What I meant was that the fix (augmented with the GET-OBJ calls) works fine on ITS compiled. That we get an integer overflow when interpreted doesn’t affect the normal compiled use case). So we should commit this fix (I can do that).

In your original fix, you didn’t add the offending GOBJECT — it was already there. So you didn’t introduce the overflow. We never noticed the issue because we don’t ever run interpreted — we can’t because there isn’t enough memory to load it all interpreted.

If you agree, I’ll commit the fix since I updated it and have tested it on ITS and found that it fixes the original problem. Note: the addition of the “DWIND” string ID is required — the fix doesn’t work using the “WINDO” ID.

@AlexProudfoot
Copy link
Contributor Author

@eswenson1 - Ok Eric. Please do that.

@eswenson1
Copy link

I've submitted this PR: PDP-10/its#2257

eswenson1 added a commit to PDP-10/its that referenced this pull request Nov 18, 2023
…ional windows.

Addresses this issue:  heasm66/mdlzork#58
And this fix: heasm66/mdlzork#59

Also updated LCF;COMP XXFILE and LCF;ZORK XXFILE in an attempt to make them a
little more robust.
eswenson1 added a commit to PDP-10/its that referenced this pull request Nov 19, 2023
…ional windows.

Addresses this issue:  heasm66/mdlzork#58
And this fix: heasm66/mdlzork#59

Also updated LCF;COMP XXFILE and LCF;ZORK XXFILE in an attempt to make them a
little more robust.
eswenson1 added a commit to PDP-10/its that referenced this pull request Nov 19, 2023
…ional windows.

Addresses this issue:  heasm66/mdlzork#58
And this fix: heasm66/mdlzork#59

Also updated LCF;COMP XXFILE and LCF;ZORK XXFILE in an attempt to make them a
little more robust.
@eswenson1
Copy link

I observe the first set of four walls share a vaule. Then there's another which has two new values. What's up with that?

I didn’t understand your observation (can’t figure out what you mean).

@eswenson1
Copy link

eswenson1 commented Nov 19, 2023

So, HOUSEBIT really is to do with a bit. Each global object is associated with a power of 2 starting at 1 and ending, in practice, at 35. Calculation of the next power of 2 produces an integer overflow.

I guess the problem doesn’t exist in Confusion because the integers have 64 bits as opposed to 40.

How does this work at all on PDP-10s? As far as I know, FIX values are limited to 36 bits. There are 40 GOBJECTS defined and multiplying 1 by 2 40 times should overflow a FIX (WORD) variable. Why does the overflow only occur at the 40th object and not the 36th object?

eswenson1 added a commit to PDP-10/its that referenced this pull request Nov 19, 2023
…ional windows.

Addresses this issue:  heasm66/mdlzork#58
And this fix: heasm66/mdlzork#59

Also updated lcf/comp.xxfile so that it doesn't fail during ACT2 build.  Seems
the occasional failures during zork build can be mitigated by breaking up some
of the compiles into separate jobs.
@AlexProudfoot
Copy link
Contributor Author

Please be patient if I seem a bit dense at any point.

Wasn’t the PDP-10 a 36-bit machine so why would FIX values be limited to 32 bits? Also, if you look over your number doubling list, you can see that the numbers don’t double for every object. As @larsbrinkhoff said “What’s up with that?”

I believe the numbers double to the point where the result would overflow 36 bits. Why the numbers don’t always double, I don’t know.

@eswenson1
Copy link

Wasn’t the PDP-10 a 36-bit machine so why would FIX values be limited to 32 bits? Also, if you look over your number doubling list, you can see that the numbers don’t double for every object. As @larsbrinkhoff said “What’s up with that?”

I believe the numbers double to the point where the result would overflow 36 bits. Why the numbers don’t always double, I don’t know.

Sorry. I typed 32 when I meant 36. The point I was trying to make -- but perhaps I'm the one who is being dense here -- is that doubling 1 36 times should overflow, and yet we don't see the overflow until the 40th object. However, I didn't realize that the numbers didn't double each time. I thought I saw code that did the equivalent of <SETG GLOHI <* ,GLOHI 2>> each time a new GOBJECT is defined.

If they don't double each time, of course, then we wouldn't die at 36 objects. I don't see how we can't be doubling though -- especially since we are trying to compute a bit value for each GOBJECT defined.

eswenson1 added a commit to PDP-10/its that referenced this pull request Nov 19, 2023
…ional windows.

Addresses this issue:  heasm66/mdlzork#58
And this fix: heasm66/mdlzork#59

Also updated lcf/comp.xxfile so that it doesn't fail during ACT2 build.  Seems
the occasional failures during zork build can be mitigated by breaking up some
of the compiles into separate jobs.

Updated zork.tcl to look for :KILL rather than .VALUE first.
eswenson1 added a commit to PDP-10/its that referenced this pull request Nov 19, 2023
…ional windows.

Addresses this issue:  heasm66/mdlzork#58
And this fix: heasm66/mdlzork#59

Also updated lcf/comp.xxfile so that it doesn't fail during ACT2 build.  Seems
the occasional failures during zork build can be mitigated by breaking up some
of the compiles into separate jobs.

Updated zork.tcl to look for :KILL rather than .VALUE first.
@AlexProudfoot
Copy link
Contributor Author

I’ll try to more precisely decipher the MDL code today but I do remember seeing conditionals.

  1. Is the name non-null
  2. Is the name already declared

I can guess that these ensure that all GOBJECTS with a null name share a GVAL of 1 and all objects that share a non-null name share the GVAL that was established when the name was first declared.

@AlexProudfoot
Copy link
Contributor Author

@heasm66
An unrelated question. What is the source of the date in the name mdlzork_810722?

@heasm66
Copy link
Owner

heasm66 commented Nov 20, 2023

@eswenson1

I havn't studied the GOBJECTS but my guess is that objects with the same number are synonyms, i.e. EAST, WEST, NORTH and SOUTH WALL should all respond in the same way when refered to.

@heasm66
Copy link
Owner

heasm66 commented Nov 20, 2023

@AlexProudfoot

The date is from the newspaper (in the kitchen) that states:

		US NEWS & DUNGEON REPORT
7/22/81  				       Last G.U.E. Edition

This version of ZORK is no longer being supported on this or any other
machine.  In particular, bugs and feature requests will, most likely, be
read and ignored.  There are updated versions of ZORK, including some
altogether new problems, available for PDP-11s and various
microcomputers (TRS-80, APPLE, maybe more later).  For information, send
a SASE to:

                Infocom, Inc.
                P.O. Box 120, Kendall Station
                Cambridge, Ma. 02142

eswenson1 added a commit to PDP-10/its that referenced this pull request Nov 21, 2023
…ional windows.

Addresses this issue:  heasm66/mdlzork#58
And this fix: heasm66/mdlzork#59

Also updated lcf/comp.xxfile so that it doesn't fail during ACT2 build.  Seems
the occasional failures during zork build can be mitigated by breaking up some
of the compiles into separate jobs.

Updated zork.tcl to no longer use XXFILE to either compile or load/save zork.
XXFILE seems too brittle and doesn't function consistently.
larsbrinkhoff pushed a commit to PDP-10/its that referenced this pull request Nov 22, 2023
Updated Zork to address issue with bad message when opening
non-functional windows.

Addresses this issue:  heasm66/mdlzork#58
And this fix: heasm66/mdlzork#59
eswenson1 added a commit to PDP-10/its that referenced this pull request Nov 22, 2023
Updated Zork to address issue with bad message when opening
non-functional windows.

Addresses this issue:  heasm66/mdlzork#58
And this fix: heasm66/mdlzork#59
@heasm66 heasm66 merged commit 9fc5ba6 into heasm66:master Jun 25, 2024
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

Successfully merging this pull request may close these issues.

None yet

4 participants