heasm66 / mdlzork

Different versions of original mainframe Zork reconstructed and patched to run under Confusion.
15 stars 6 forks source link

#58 - proposed changes #59

Closed AlexProudfoot closed 2 weeks ago

AlexProudfoot commented 8 months ago

Proposed changes for issue #58.

larsbrinkhoff commented 8 months ago

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

CC @eswenson1; also https://github.com/heasm66/mdlzork/issues/58

AlexProudfoot commented 8 months ago

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

larsbrinkhoff commented 8 months ago

@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 commented 8 months ago

@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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

@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 commented 8 months ago

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 https://github.com/heasm66/mdlzork/pull/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 commented 8 months ago

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

AlexProudfoot commented 8 months ago

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 commented 8 months ago

To recompile Zork, do:

:xxfile tty:_lcf;comp xxfile

then do:

:xxfile tty:_lcf;zork xxfile

larsbrinkhoff commented 8 months ago

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

AlexProudfoot commented 8 months ago

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 commented 8 months ago

Finished now @heasm66.

eswenson1 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

What happens if you try?

eswenson1 commented 8 months ago

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 commented 8 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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.

eswenson1 commented 7 months ago

Well that value of GLOHI is mighty big and multiplying it by 2 causes an arithmetic overflow. Might want to look for the definition and any GSETs of that variable.

Can you post your LOADAL source?

AlexProudfoot commented 7 months ago

LOADAL

<DEFINE VFLOAD (X) <FLOAD .X> <PRINTSTRING <STRING .X " loaded">> <TERPRI>>
<VFLOAD "prim">
<VFLOAD "defs">
<VFLOAD "makstr">
<VFLOAD "tell-r">
<VFLOAD "act1">
<VFLOAD "act2">
<VFLOAD "act3">
<VFLOAD "act4">
;<VFLOAD "b">
<VFLOAD "disp1">
<VFLOAD "parser">
<VFLOAD "melee">
<VFLOAD "rooms">
<VFLOAD "dung">
<VFLOAD "impl">
<VFLOAD "sr">
<VFLOAD "syntax">
<VFLOAD "typhak">
<VFLOAD "util">
AlexProudfoot commented 7 months ago
*: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

*ERROR*
OVERFLOW
*
LISTENING-AT-LEVEL 2 PROCESS 1
eswenson1 commented 7 months ago

And can you post your RUN source?

larsbrinkhoff commented 7 months ago

Isn't it the case that Zork is too big to load uncompiled?

AlexProudfoot commented 7 months ago

RUN

<FLOAD "loadal">
<SAVE-IT>
AlexProudfoot commented 7 months ago

Oh. I didn't consider that but you may well be right. That would make my bug hunt a bit difficult.

Do you have any suggestions?

eswenson1 commented 7 months ago

Isn't it the case that Zork is too big to load uncompiled?

I thought that was the case. I do note that @AlexProudfoot is using a different version of TELL -- that I don't have.

When I updated LOADAL to load TELL NBIN, I can get the same error that @AlexProudfoot does with the overflow error attempting to multiply GLOHI by 2. GLOHI has the GVAL of 17179869184, which is mighty big. I'll have to find out what that variable is for, and what values are acceptable.

eswenson1 commented 7 months ago

Oh. I didn't consider that but you may well be right. That would make my bug hunt a bit difficult.

Do you have any suggestions?

What I've done is loaded everything compiled, except what I needed for debugging. Sometimes loading individual functions in interpreted on top of a load of the compiled NBINs.

eswenson1 commented 7 months ago

Here are the references to GLOHI in the code:

dung.355:<SETG GLOHI 1>
makstr.44:<GDECL (GLOHI STAR-BITS) FIX>
makstr.44:                   (<SETG GLOHI <SET BITS <* ,GLOHI 2>>>
makstr.44:            (<SETG GLOHI <SET BITS <* ,GLOHI 2>>>

It seems pretty unreasonable that GLOHI is getting multiplied by 2 from 1 so many times that it gets this high. But I suppose that we may have pushed it over the edge with another GOBJECT. Will have to investigate.

AlexProudfoot commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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?

AlexProudfoot commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

@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 commented 7 months ago

Its been a while since I looked at the code and I don't even remember if Zork runs in 32-bit Confusion. I know there are a couple of places that uses all 36 bits for flags and such. This could be another case where the number of global objects are limited by 36 bits and Zork uses every slot already.

Den lör 18 nov. 2023 11:40Alex Proudfoot @.***> skrev:

@eswenson1 https://github.com/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.

— Reply to this email directly, view it on GitHub https://github.com/heasm66/mdlzork/pull/59#issuecomment-1817472519, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOJZOMQ2A4WE2T745FU5FGLYFCGAXAVCNFSM6AAAAAA6ZCZTJCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJXGQ3TENJRHE . You are receiving this because you were mentioned.Message ID: @.***>

eswenson1 commented 7 months ago

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 commented 7 months ago

@eswenson1 - Ok Eric. Please do that.

eswenson1 commented 7 months ago

I've submitted this PR: https://github.com/PDP-10/its/pull/2257

eswenson1 commented 7 months ago

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 commented 7 months ago

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?

AlexProudfoot commented 7 months ago

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.