revisit shop_keeper() readiness

During a restore from a savefiles is not the only
time that levels are processed by getlev() in NetHack.
They are read back in as the hero moves up and down
between levels and dungeons.

The previous fix checked for program_state.beyond_savefile_load,
but once set, that remains static through level changes.

It would be better to check the status of the level
being read, because those operations do placements as well.

Add the following:
    struct levelstatus level_status;
    level_status.making   - in the midst of makelevel processing
    level_status.loading  - in the midst of loading a level via getlev()
    level_status.ready    - the level is fully ready

    (all 3 of the above status settings are mutually exclusive)

    level_status.shkready - the level processing is far enough
                            along to allow shop keeper tests and actions

This also relocates the find_lev_obj() call in getlev() down several
lines, so that it falls after any set_residency() calls, so that it
has a better chance of carrying out what it was intending to do with
the shop_keeper() checks made by its subfunctions.

doc/fixes5-0-1.txt

@@ -46,6 +46,10 @@ replace the hashtable used for parsing glyphnames with sorted indices and
 	of pr #1548 by ingpaschke)
 for SELECTSAVE, allow utd flags for uptodate() to be passed down through
 	the small set of callers that eventually call uptodate()
+avoid calling costly_spot(), costly_adjacent(), find_byowner(), which
+	all rely on shop_keeper() under the hood, in the midst of the level
+	loading process before all the required data structures have
+	been finalized
  
 
 Platform- and/or Interface-Specific Fixes

include/decl.h

@@ -1288,6 +1288,11 @@ extern struct instance_globals_saved_w svw;
 extern struct instance_globals_saved_x svx;
 extern struct instance_globals_saved_y svy;
 extern struct sinfo program_state; /* flags describing game's current state */
+/* flags describing current level's loading/making/readiness status;
+ * restlevelstate() already associated term 'levelstate' for a different
+ * purpose, so attempt to avoid confusion
+ */
+extern struct levelstatus level_status;
 
 struct const_globals {
     const struct obj zeroobj;      /* used to zero out a struct obj */

include/extern.h

@@ -520,6 +520,7 @@ extern void destroy_drawbridge(coordxy, coordxy);
 /* ### decl.c ### */
 
 extern void program_state_init(void);
+extern void level_status_init(void);
 extern void decl_globals_init(void);
 extern void sa_victual(volatile struct victual_info *);
 

include/hack.h

@@ -821,6 +821,14 @@ struct sinfo {
 #endif
 };
 
+/* structure for current 'level_status'; not saved and restored */
+struct levelstatus {
+    int making;                 /* makelevel has begun */
+    int loading;                /* level loading has begun */
+    int shkready;               /* shops ready */
+    int ready;                  /* level is ready */
+};
+
 /* value of program_state.input_state, significant during readchar();
    get_count() expects digits then a command so sets it to commandInp */
 enum InputState {

src/decl.c

@@ -1002,6 +1002,7 @@ static const struct instance_globals_saved_y init_svy = {
 };
 
 static const struct sinfo init_program_state = { 0 };
+static const struct levelstatus init_level_status = { 0 };
 
 #if 0
 struct instance_globals g;
@@ -1054,6 +1055,7 @@ struct instance_globals_saved_w svw;
 struct instance_globals_saved_x svx;
 struct instance_globals_saved_y svy;
 struct sinfo program_state;
+struct levelstatus level_status;
 
 const struct const_globals cg = {
     DUMMY, /* zeroobj */
@@ -1079,6 +1081,12 @@ program_state_init(void)
     program_state = init_program_state;
 }
 
+void
+level_status_init(void)
+{
+    level_status = init_level_status;
+}
+
 void
 decl_globals_init(void)
 {

src/mklev.c

@@ -1261,6 +1261,9 @@ makelevel(void)
         impossible("makelevel() called when dungeon not yet initialized.");
         init_dungeons();
     }
+    level_status_init();
+    level_status.making = 1;
+
     oinit(); /* assign level dependent obj probabilities */
     clear_level_structures();
 
@@ -1416,7 +1419,7 @@ makelevel(void)
     for (i = 0; i < svn.nroom; ++i) {
         fill_special_room(&svr.rooms[i]);
     }
-
+    level_status.shkready = 1;
     themerooms_post_level_generate();
 
     if (gl.luacore && nhcb_counts[NHCB_LVL_ENTER]) {
@@ -1425,6 +1428,7 @@ makelevel(void)
         nhl_pcall_handle(gl.luacore, 1, 0, "makelevel", NHLpa_panic);
         lua_settop(gl.luacore, 0);
     }
+    level_status.making = 0, level_status.ready = 1;
 }
 
 /* return TRUE if water location at (x,y) should have kelp. */

src/mkobj.c

@@ -2354,9 +2354,7 @@ place_object(struct obj *otmp, coordxy x, coordxy y)
     otmp->where = OBJ_FLOOR;
 
     /* if placed outside of shop, no_charge is no longer applicable */
-    if (program_state.beyond_savefile_load
-        && otmp->no_charge
-        && !costly_spot(x, y)
+    if (level_status.shkready && otmp->no_charge && !costly_spot(x, y)
         && !costly_adjacent(find_objowner(otmp, x, y), x, y))
         otmp->no_charge = 0;
 

src/restore.c

@@ -1068,6 +1068,8 @@ getlev(NHFILE *nhfp, int pid, xint8 lev)
 #endif
 
     program_state.in_getlev = TRUE;
+    level_status_init();
+    level_status.loading = 1;
 #ifndef SFCTOOL
 
     if (ghostly)
@@ -1170,9 +1172,11 @@ getlev(NHFILE *nhfp, int pid, xint8 lev)
     dealloc_trap(trap);
 
     fobj = restobjchn(nhfp, FALSE);
-#ifndef SFCTOOL
-    find_lev_obj();
-#endif  /* !SFCTOOL */
+    /* more work needs to be done on fobj in find_lev_obj() further down,
+     * but that needs to happen after set_residency() so that shop_keeper()
+     * will return correct results during the processing.
+     */
+
     /* restobjchn()'s `frozen' argument probably ought to be a callback
        routine so that we can check for objects being buried under ice */
     svl.level.buriedobjlist = restobjchn(nhfp, FALSE);
@@ -1202,7 +1206,6 @@ getlev(NHFILE *nhfp, int pid, xint8 lev)
             if (hides_under(mtmp->data) && mtmp->mundetected)
                 (void) hideunder(mtmp);
         }
-
         /* regenerate monsters while on another level */
         if (!u.uz.dlevel || program_state.restoring == REST_LEVELS)
             continue;
@@ -1225,6 +1228,11 @@ getlev(NHFILE *nhfp, int pid, xint8 lev)
         if (ghostly || (elapsed > 0L && elapsed > (long) rnd(10)))
             hide_monst(mtmp);
     }
+    level_status.shkready = 1;
+    /* post-5.0.0: this is now postponed until here so that it takes place
+       after set_residency() has been called */
+    find_lev_obj();
+
 #endif /* !SFCTOOL */
 
     restdamage(nhfp);
@@ -1308,12 +1316,13 @@ getlev(NHFILE *nhfp, int pid, xint8 lev)
 
     if (ghostly)
         clear_id_mapping();
+#endif
+    level_status.loading = 0, level_status.ready = 1;
     program_state.in_getlev = FALSE;
-#else
+#ifdef SFCTOOL
     nhUse(pid);
     nhUse(lev);
 #endif /* !SFCTOOL */
-    program_state.in_getlev = FALSE;
 }
 
 void

src/shk.c

@@ -1075,6 +1075,17 @@ shop_keeper(char rmno)
                correct the underlying svr.rooms[].resident issue but... */
             return (struct monst *) 0;
         }
+    } else {
+        if (!level_status.shkready) {
+            int hmm UNUSED = 1;
+#if (NH_DEVEL_STATUS != NH_STATUS_RELEASED \
+     && NH_DEVEL_STATUS != NH_STATUS_POSTRELEASE)
+                impossible("untrustworthy null shkp; level_status.shkready"
+                           " is FALSE (%d, %d, %d, &d)",
+                           level_status.making, level_status.loading,
+                           level_status.shkready, level_status.ready);
+#endif
+        }
     }
     return shkp;
 }