RFC: Fix Confusion compiler warnings

[eriktorbjorn]

This pull request is not intended to merge wholesale - at least not without further discussion and investigation - but represents my attempt at fixing or silencing GCC compiler warnings when compiling Confusion.

The most intrusive change is that I've replaced __gnu_cxx::hash_set with std::unordered_set, without really knowing much about either. It seems to work. I played through the game far enough to get 400+ points, and then I ran into https://github.com/heasm66/mdlzork/issues/45 which probably wasn't caused by my changes.

(I wonder if it would be possible to add some sort of semi-automated regression testing, i.e. setting the random seed and feeding it a walkthrough.)

[heasm66]

Great work! I'll look over the changes as good as I can but it would be nice to include your modifications. Would you like me to report the PR upstream (I'm thinking of an issue with s link on the GitLab project) or do you do one yourself?

I'm not familiar with MDL32 but guess it compiles a 32-bit version. PDP-10 ITS was a 36-bit computer so a WORD in it had some extra bits. I know Zork used all bits for, for example, the flags so I guess it could run into trouble pretty quickly.

[larsbrinkhoff]

I checked what MDL32 does: it makes both ints and floats 32 bits.

[heasm66]

Yeah, that will cause problems when Zork tries to fit its 34 flags for objects into a 32-bit word.

[eriktorbjorn]

Would you like me to report the PR upstream (I'm thinking of an issue with s link on the GitLab project)

Please do. (Or perhaps I should say "Feel Free".)

[heasm66]

I reviewed the changes and leaning on merging them in. What's your biggest worry?

The ones I personally had to look up were:

• Using std::unordered_set instead of __gnu_cxx::hash_set seems as far as I can read be the recommended way.
• memcpy() instead of strncpy() looks ok.

Both seems ok. The other changes are unproblematic in my opinion.

[eriktorbjorn]

The unordered_set / hash_set change is definitely the one I was most nervous about. I saw some mention that they don't guarantee the same order when iterating over them, but I don't think Confusion depends on that. The memcpy() one, I think is safe and fairly clear about what the intention is.

What I was mostly worried about for the whole series of patches was that I might be breaking some subtle behavior like, say, the thief's path through the game. But that seems to be unchanged.

That said, the path thief's does not seem to match the FORTRAN version, nor Jeff Claar's C++ port. If my debug messages are correct:

Confusion (810722): CP, CPOUT, BKEXE, BKVE, BKBOX, BKVW, BKTE, BKTW, ALISM, ALITR, TWELL, CAGED, CAGER, ... C++: TOMB, SLIDE, PRM, CP, CPOUT, CPANT, BKEXE, BKBOX, BKVE, BKTE, BKTW, BKENT, ALITR, ALISM, ALICE, ... FORTRAN: CPOUT, CPANT, 156, BKBOX, BKVE, BKVW, 150, 149, BKENT, ALITR, ALISM, ALICE, ...

So the FORTRAN and C++ versions seem to be pretty close, but Confusion deviates? Is this a Confusion bug?

[heasm66]

I've made a little test-function:

<DEFINE ATLAS ("AUX" (RMTMP ,ROOMS) RM)
    <REPEAT ()
    <SET RM <1 .RMTMP>>
        <PRIN1 <1 .RM>>
    <COND (<NOT <OR <RTRNN .RM ,RSACREDBIT>
         <RTRNN .RM ,RENDGAME>
         <NOT <RTRNN .RM ,RLANDBIT>>>>
           <PRINTSTRING " *">)>
     <CRLF>
       <SET RMTMP <REST .RMTMP>>
        <COND (<EMPTY? .RMTMP> <RETURN>)>
        <AGAIN>>>

Running this on Zork both on PDP-10 ITS and Confusion produces this identical output from the ROOMS-list (asterix means eligable for the thief):

#PSTRING PCELL
#PSTRING PARAP
#PSTRING NIRVA
#PSTRING NCELL
#PSTRING CELL
#PSTRING WCORR
#PSTRING ECORR
#PSTRING SCORR
#PSTRING NCORR
#PSTRING CRYPT
#PSTRING MRANT
#PSTRING TSTRS
#PSTRING MRAW
#PSTRING MRAE
#PSTRING MRBW
#PSTRING MRBE
#PSTRING MRCW
#PSTRING MRCE
#PSTRING MRGW
#PSTRING MRGE
#PSTRING MRDW
#PSTRING MRDE
#PSTRING MREYE
#PSTRING FDOOR
#PSTRING INMIR
#PSTRING MRA
#PSTRING MRB
#PSTRING MRC
#PSTRING MRG
#PSTRING MRD
#PSTRING SPAL
#PSTRING SLEDG
#PSTRING SLID3
#PSTRING SLID1
#PSTRING SLID2
#PSTRING PALAN
#PSTRING CP *
#PSTRING CPOUT *
#PSTRING BKEXE *
#PSTRING BKVAU
#PSTRING BKTWI
#PSTRING BKVE *
#PSTRING BKBOX *
#PSTRING BKVW *
#PSTRING BKTE *
#PSTRING BKTW *
#PSTRING ALISM *
#PSTRING ALITR *
#PSTRING TWELL *
#PSTRING CAGED *
#PSTRING CAGER *
#PSTRING ALICE *
#PSTRING CMACH *
#PSTRING SAFE *
#PSTRING LIBRA *
#PSTRING VAIR4
#PSTRING LEDG4 *
#PSTRING VAIR3
#PSTRING VAIR2
#PSTRING LEDG2 *
#PSTRING VAIR1
#PSTRING VLBOT *
#PSTRING CLMID
#PSTRING CLBOT
#PSTRING POG *
#PSTRING RAINB
#PSTRING FALLS
#PSTRING FANTE
#PSTRING FCHMP *
#PSTRING BEACH
#PSTRING RIVR5
#PSTRING WCLF2
#PSTRING RCAVE
#PSTRING WCLF1
#PSTRING RIVR4
#PSTRING RIVR3
#PSTRING RIVR2
#PSTRING RIVR1
#PSTRING MAINT *
#PSTRING LOBBY *
#PSTRING DOCK
#PSTRING TEMP2
#PSTRING TEMP1
#PSTRING TOMB *
#PSTRING LLD2 *
#PSTRING BWELL *
#PSTRING MPEAR *
#PSTRING RIDDL *
#PSTRING DEAD6 *
#PSTRING DEAD5 *
#PSTRING TCAVE *
#PSTRING CAVE4 *
#PSTRING PRM *
#PSTRING MTORC *
#PSTRING MINE6 *
#PSTRING MINE3 *
#PSTRING MINE5 *
#PSTRING MINE2 *
#PSTRING MINE4 *
#PSTRING MACHI *
#PSTRING BSHAF
#PSTRING TIMBE
#PSTRING DEAD7 *
#PSTRING TLADD *
#PSTRING MINE7 *
#PSTRING BLADD *
#PSTRING BOOM
#PSTRING MINE1 *
#PSTRING SMELL *
#PSTRING TUNNE *
#PSTRING BATS
#PSTRING ENTRA *
#PSTRING TSHAF *
#PSTRING SQUEE *
#PSTRING SLIDE *
#PSTRING MGRAI *
#PSTRING LLD1 *
#PSTRING MIRR2 *
#PSTRING CAVE2 *
#PSTRING CRAW3 *
#PSTRING PASS4 *
#PSTRING MIRR1 *
#PSTRING CRAW2 *
#PSTRING PASS3 *
#PSTRING ECHO *
#PSTRING CAVE3 *
#PSTRING PASS5 *
#PSTRING CHAS3 *
#PSTRING CAROU *
#PSTRING DAM *
#PSTRING CAVE1 *
#PSTRING LAVA *
#PSTRING RUBYR *
#PSTRING LEDG3 *
#PSTRING ICY *
#PSTRING ATLAN *
#PSTRING INSTR
#PSTRING RESEN *
#PSTRING STREA *
#PSTRING CANY1 *
#PSTRING RESER
#PSTRING DOME *
#PSTRING EGYPT *
#PSTRING RAVI1 *
#PSTRING CRAW1 *
#PSTRING CHAS1 *
#PSTRING RESES *
#PSTRING CPANT *
#PSTRING TREAS *
#PSTRING CYCLO *
#PSTRING DEAD4 *
#PSTRING MAZ12 *
#PSTRING MAZ13 *
#PSTRING MAZ10 *
#PSTRING MAZ11 *
#PSTRING DEAD3 *
#PSTRING MAZ15 *
#PSTRING MAZE8 *
#PSTRING MAZ14 *
#PSTRING MAZE9 *
#PSTRING MAZE7 *
#PSTRING MAZE6 *
#PSTRING DEAD2 *
#PSTRING DEAD1 *
#PSTRING MAZE5 *
#PSTRING MAZE3 *
#PSTRING MAZE4 *
#PSTRING MAZE2 *
#PSTRING BKENT *
#PSTRING STUDI *
#PSTRING GALLE *
#PSTRING MAZE1 *
#PSTRING PASS1 *
#PSTRING CRAW4 *
#PSTRING CHAS2 *
#PSTRING MTROL *
#PSTRING MGRAT *
#PSTRING FORE5
#PSTRING CLTOP
#PSTRING TREE *
#PSTRING FORE4
#PSTRING CELLA *
#PSTRING BLROO *
#PSTRING ATTIC *
#PSTRING LROOM
#PSTRING CLEAR
#PSTRING KITCH
#PSTRING FORE2
#PSTRING FORE3
#PSTRING EHOUS
#PSTRING FORE1
#PSTRING SHOUS
#PSTRING NHOUS
#PSTRING !
#PSTRING MAGNE *
#PSTRING WHOUS
#PSTRING BDOOR

So I would say that both the C++ and FORTRAN handles the thief "wrong". Why the ROOMS-list looks like it does is another question. One would think that it would be a LIFO-list where the last added room is the first in the list.

[heasm66]

Mystery solved (I think). Room-slots in the ROOMS-list are created by the function GET-ROOM. The first call to this function creates the room that is last in the list, and so on. Before the rooms are created there are some calls to ADD-ACTOR, that creates BDOOR, WHOUS and MAINT. Then when the rooms are created they have exits defined and each call to EXIT creates a slot for that room (if it's not already in the list). This, I think, accounts for the "random" order of the ROOMS-list.

[eriktorbjorn]

Ok, I guess I can probably stop worrying about the thief. At least in this particular sense. Thanks!