heasm66 / mdlzork

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

Strange line break in endgame score display #52

Closed eriktorbjorn closed 3 months ago

eriktorbjorn commented 1 year ago

I haven't been able to figure out why there is a line break (and a space at the beginning of the next line) when the endgame score is displayed:

>SCORE

Your score in the end game would be 0 [total of 100 points], in 1112
 moves.
This score gives you the rank of Cheater.

I don't see anything in the MDL source code to explain it to me:

    <CRLF>
    <PRINC "Your score ">
    <COND (.EG
           <PRINC "in the end game ">)>
    <COND (.ASK? <PRINC "would be ">)
          (<PRINC "is ">)>
    <COND (.EG
           <PRIN1 <SET SCOR ,EG-SCORE>>)
          (<PRIN1 <SET SCOR <ASCORE ,WINNER>>>)>
    <PRINC " [total of ">
    <PRIN1 <SET SMAX <COND (.EG ,EG-SCORE-MAX) (,SCORE-MAX)>>>
    <PRINC " points], in ">
    <PRIN1 ,MOVES>
    <COND (<1? ,MOVES> <PRINC " move.">)
          (<PRINC " moves.">)>
    <CRLF>

It can't be PRIN1... can it? Is Confusion doing line breaking on its own?

heasm66 commented 1 year ago

Hmm, there's something funny going on in Confusion when the line hits a certain length:

Your score in the end game would be 15 [total of 100 points], in 94 moves.
This score gives you the rank of Cheater.
Your score in the end game would be 15 [total of 100 points], in 101
 moves.
This score gives you the rank of Cheater.
heasm66 commented 1 year ago

Made a stripped down test-program... Funny enough it hasn't any problem with LF...

<DEFINE SCORE ("OPTIONAL" (SCORE 0) (MOVES 1121) (ASK? T) (EG T) "AUX" SCOR SMAX (OUTCHAN .OUTCHAN) PCT) 
    #DECL ((ASK?) <OR ATOM FALSE> (SCOR SMAX) FIX (OUTCHAN) CHANNEL (PCT) FLOAT
           (EG) <OR ATOM FALSE>)
    <CRLF>
    <PRINC "Your score ">
    <COND (.EG
           <PRINC "in the end game ">)>
    <COND (.ASK? <PRINC "would be ">)
          (<PRINC "is ">)>
    <COND (.EG
           <PRIN1 <SET SCOR .SCORE>>)
          (<PRIN1 <SET SCOR .SCORE>>)>
    <PRINC " [total of ">
    <PRIN1 <SET SMAX <COND (.EG 100) (616)>>>
    <PRINC " points], in ">
    <PRIN1 .MOVES>
    <COND (<1? .MOVES> <PRINC " move.">)
          (<PRINC " moves.">)>
    <CRLF>
    <PRINC "This score gives you the rank of cheater.">
    <CRLF>
>
heasm66 commented 1 year ago

Ok. The culprit is this (in the function START)

           <COND (<NOT ,TENEX?>
              <AND <0? <13 ,OUTCHAN>>
               <PUT ,OUTCHAN 13 80>>)>)>

Position 13 in OUTCHAN is, according to MDL Programming Language is "number of characters per line of output". On PDP-10 ITS it's set to the actual width of the terminal (131 on the emulator) but in Confusion it's set to 0 initially. Therefore the code sets it to 80 characters.

Fix 1: Change 80 to a higher value in the above line. Fix 2: Change Confusion to have another initial value here (actual width, or 131?).

heasm66 commented 1 year ago

Fix 2 (mdl_output.cpp):

  mdl_value_t *mdl_create_default_outchan()
  {
      mdl_value_t *chan = mdl_internal_create_channel();
      int chnum = mdl_new_chan_num(stdout);
      mdl_set_chan_mode(chan, "PRINT");
      *VITEM(chan,CHANNEL_SLOT_CHNUM) = *mdl_new_fix(chnum);
+     *VITEM(chan,CHANNEL_SLOT_LINEWIDTH) = *mdl_new_fix(131);

      if (!isatty(fileno(stdout)))
          *VITEM(chan,CHANNEL_SLOT_DEVN) = *(mdl_new_string(3, "DSK"));
      else
          *VITEM(chan,CHANNEL_SLOT_DEVN) = *(mdl_new_string(3, "TTY"));
      return chan;
  }
eriktorbjorn commented 1 year ago

I noticed the OUTCHAN thing earlier today, but didn't have my GitHub login with me.

Since the resulting line is still less than 80 characters, I think there may be a bug in mdl_print_string_to_chan(). It gets a string (str) and its length (len), and then - for whatever reason - it counts most of the length a second time:

    olen = len;
    if (!binary)
    {
        tlen = olen;
        s = str;

        while (tlen--)
        {
            ch = *s++;
            if (!isspace(ch) && !iscntrl(ch)) len++;
        }
    }

And that length is then passed on to mdl_need_line_break(). I have no idea what the idea here is.

eriktorbjorn commented 1 year ago

Looking a bit closer at the context, mdl_print_string_to_chan() goes on to do this:

    s = str;
    tlen = olen;
    while (tlen--)
    {
        ch = *s++;
        if (!binary && !isspace(ch) && iscntrl(ch))
        {
            mdl_print_char_to_chan(chan, '^', MDL_PF_NONE, f);
            ch = ch + 0x40;
        }
        mdl_print_char_to_chan(chan, ch, MDL_PF_NONE, f);
    }

Which I guess means that control characters like Ctrl+A, Ctrl+B, etc. are turned into "^A", "^B", etc for non-binary streams. So it's probably just a case of the check being accidentally inverted in the first loop. In which case I guess this should be enough to fix it:

diff --git a/confusion_patched/mdl_output.cpp b/confusion_patched/mdl_output.cpp
index 9d56539..cd34611 100644
--- a/confusion_patched/mdl_output.cpp
+++ b/confusion_patched/mdl_output.cpp
@@ -358,7 +358,7 @@ void mdl_print_string_to_chan(mdl_value_t *chan,
         while (tlen--)
         {
             ch = *s++;
-            if (!isspace(ch) && !iscntrl(ch)) len++;
+            if (!isspace(ch) && iscntrl(ch)) len++;
         }
     }
     else if (!mdl_chan_mode_is_output(chan))

I.e. use the same condition for the extra character in both loops. With that change:

<SCORE 99>

Your score in the end game would be 99 [total of 100 points], in 1121 moves.
This score gives you the rank of cheater.
T
<SCORE 100>

Your score in the end game would be 100 [total of 100 points], in 1121 moves.
This score gives you the rank of cheater.
T
<SCORE 1000>

Your score in the end game would be 1000 [total of 100 points], in 1121 moves.
This score gives you the rank of cheater.
T
<SCORE 10000>

Your score in the end game would be 10000 [total of 100 points], in 1121 moves.
This score gives you the rank of cheater.
T
<SCORE 100000>

Your score in the end game would be 100000 [total of 100 points], in 1121 moves.
This score gives you the rank of cheater.
T
<SCORE 1000000>

Your score in the end game would be 1000000 [total of 100 points], in 1121
 moves.
This score gives you the rank of cheater.
T
eriktorbjorn commented 1 year ago

I didn't realize GitHub would snitch on me for committing that to my fork. :-)

I'm still not sure if it's worth it to submit a pull request for a one-character fix, and it would be interesting to know how it compares to the original. I don't have the know-how to test that, though.

heasm66 commented 1 year ago

I havn't had the time to test it yet, but soon...

heasm66 commented 1 year ago

The ITS/MDL handles it correctly:

<PUT ,OUTCHAN 13 80>$
#CHANNEL [1 "PRINT" "INPUT" ">" "TTY" " " "INPUT" ">" "TTY" " " 5186 23748385836
80 3 48 48 1713 10 ""]
<SCORE>$

Your score in the end game would be 0 [total of 100 points], in 1121 moves.
This score gives you the rank of cheater.
T
<PUT ,OUTCHAN 13 70>$
#CHANNEL [1 "PRINT" "INPUT" ">" "TTY" " " "INPUT" ">" "TTY" " " 5186
23748385836 70 15 48 7 1949 10 ""]
<SCORE>$

Your score in the end game would be 0 [total of 100 points], in 1121
 moves.
This score gives you the rank of cheater.
T

I'm gonna use your pull request, but also set an initial value other than 0 for <13 ,OUTCHAN> in TELL-REPL.

heasm66 commented 3 months ago

Tested this on PDP-10 ITS:

MUDDLE 56 IN OPERATION.
LISTENING-AT-LEVEL 1 PROCESS 1
<DEFINE TEST (X) <PRINC .X> <PRINC .X> <>>$
TEST
<TEST "HELLO">$
HELLOHELLO#FALSE ()
<PUT ,OUTCHAN 13 10>$
#CHANNEL [
1 "PRINT"
"INPUT"
">" "TTY"
" "
"INPUT"
">" "TTY"
" " 5186
23748405326
10 3 48 16
177 10 ""]
<TEST "HELLO">$
HELLOHELLO
#FALSE ()

This is in Confusion before @eriktorbjorn fix:

LISTENING-AT-LEVEL 1 PROCESS 1
<DEFINE TEST (X) <PRINC .X> <PRINC .X> <>>
TEST
<TEST "HELLO">
HELLOHELLO#FALSE ()
<PUT ,OUTCHAN 13 10>
#CHANNEL
[2
"PRINT"
"" "" ""
"" "" ""
"TTY" ""
0 0 10 6
0 0 0 10
0]
<TEST "HELLO">
HELLO
HELLO
#FALSE ()

And after the fix:

LISTENING-AT-LEVEL 1 PROCESS 1
<DEFINE TEST (X) <PRINC .X> <PRINC .X> <>>
TEST
<TEST "HELLO">
HELLOHELLO#FALSE ()
<PUT ,OUTCHAN 13 10>
#CHANNEL [
2 "PRINT"
"" "" ""
"" "" ""
"TTY" "" 0
0 10 4 0 0
0 10 0]
<TEST "HELLO">
HELLOHELLO
#FALSE ()