re-implement pull req #334 - sorting discoveries

The pull request changed \ and ` output to unconditionally show
discoveries in alphabetical order.  That's nearly useless except
when looking at prediscovered weapons and armor that fighter
types start out knowing.

This allows the player to choose sorting order via the new
'sortdiscoveries' option.  In addition to setting it via
config file or 'O', it can be set via 'm' prefix for \ and `.
Choices are:
 o - sort by class, by order of discovery in class (default);
 s - sort by 'sortloot' classification which groups sub-class
     items (so all helmets before any other armor, then all
     gloves, then boots, and so on); within each sub-class, or
     whole class for classes which don't subdivide so usefully,
     partly-discovered types (where a name has been assigned)
     come before fully ID'd types;
 c - sort by class, alphabetically within class;
 a - sort alphabetically across all classes.

Turned out to be a large amount of work for fairly little gain,
although I suspect that 'sortdiscoveries:s' will eventually be
more popular than the default.

Invalidates existing save files so that current sort setting can
persist across save/restore cycles.

Closes #334

src/cmd.c

@@ -3236,6 +3236,8 @@ int NDECL((*cmd_func));
         /* 'm' for removing saddle from adjacent monster without checking
            for containers at <u.ux,u.uy> */
         || cmd_func == doloot
+        /* offer menu to choose discoveries sort order */
+        || cmd_func == dodiscovered || cmd_func == doclassdisco
         /* travel: pop up a menu of interesting targets in view */
         || cmd_func == dotravel
         /* wait and search: allow even if next to a hostile monster */

src/invent.c

@@ -13,7 +13,6 @@
 #define CONTAINED_SYM '>' /* designator for inside a container */
 #define HANDS_SYM '-'
 
-static void FDECL(loot_classify, (Loot *, struct obj *));
 static char *FDECL(loot_xname, (struct obj *));
 static int FDECL(invletter_value, (CHAR_P));
 static int FDECL(CFDECLSPEC sortloot_cmp, (const genericptr,
@@ -54,7 +53,7 @@ static char FDECL(obj_to_let, (struct obj *));
 static const char venom_inv[] = { VENOM_CLASS, 0 }; /* (constant) */
 
 /* sortloot() classification; called at most once [per sort] for each object */
-static void
+void
 loot_classify(sort_item, obj)
 Loot *sort_item;
 struct obj *obj;

src/o_init.c

@@ -9,6 +9,8 @@ static void FDECL(setgemprobs, (d_level *));
 static void FDECL(shuffle, (int, int, BOOLEAN_P));
 static void NDECL(shuffle_all);
 static boolean FDECL(interesting_to_discover, (int));
+static int FDECL(CFDECLSPEC discovered_cmp, (const genericptr,
+                                             const genericptr));
 static char *FDECL(oclass_to_name, (CHAR_P, char *));
 
 #ifdef USE_TILES
@@ -465,37 +467,167 @@ static const short uniq_objs[] = {
     BELL_OF_OPENING,
 };
 
+/* discoveries qsort comparison function */
+static int CFDECLSPEC
+discovered_cmp(v1, v2)
+const genericptr v1;
+const genericptr v2;
+{
+    const char *s1 = *(const char **) v1;
+    const char *s2 = *(const char **) v2;
+    /* each element starts with "* " or "  " but we don't sort by those */
+    int res = strcmpi(s1 + 2, s2 + 2);
+
+    if (res == 0) {
+        ; /* no tie-breaker needed */
+    }
+    return res;
+}
+
+static char *
+sortloot_descr(otyp, outbuf)
+int otyp;
+char *outbuf;
+{
+    Loot sl_cookie;
+    struct obj o;
+
+    o = cg.zeroobj;
+    o.otyp = otyp;
+    o.oclass = objects[otyp].oc_class;
+    o.dknown = 1;
+    o.known = (objects[otyp].oc_name_known || !objects[otyp].oc_uses_known)
+              ? 1 : 0;
+    o.corpsenm = NON_PM; /* suppress statue and figurine details */
+    /* but suppressing fruit details leads to "bad fruit #0" */
+    if (otyp == SLIME_MOLD)
+        o.spe = g.context.current_fruit;
+
+    (void) memset((genericptr_t) &sl_cookie, 0, sizeof sl_cookie);
+    sl_cookie.obj = (struct obj *) 0;
+    sl_cookie.str = (char *) 0;
+
+    loot_classify(&sl_cookie, &o);
+    Sprintf(outbuf, "%02d%02d%1d ",
+            sl_cookie.orderclass, sl_cookie.subclass, sl_cookie.disco);
+    return outbuf;
+}
+
+#define DISCO_BYCLASS      0 /* by discovery order within each class */
+#define DISCO_SORTLOOT     1 /* by discovery order within each subclass */
+#define DISCO_ALPHABYCLASS 2 /* alphabetized within each class */
+#define DISCO_ALPHABETIZED 3 /* alphabetized across all classes */
+/* also used in options.c (optfn_sortdiscoveries) */
+const char disco_order_let[] = "osca";
+const char *const disco_orders_descr[] = {
+    "by order of discovery within each class",
+    "sortloot order (by class with some sub-class groupings)",
+    "alphabetical within each class",
+    "alphabetical across all classes",
+    (char *) 0
+};
+
+int
+choose_disco_sort(mode)
+int mode; /* 0 => 'O' cmd, 1 => full discoveries; 2 => class discoveries */
+{
+    winid tmpwin;
+    menu_item *selected;
+    anything any;
+    int i, n, choice;
+
+    tmpwin = create_nhwindow(NHW_MENU);
+    start_menu(tmpwin, MENU_BEHAVE_STANDARD);
+    any = cg.zeroany; /* zero out all bits */
+    for (i = 0; disco_orders_descr[i]; ++i) {
+        any.a_int = disco_order_let[i];
+        add_menu(tmpwin, NO_GLYPH, &any, (char) any.a_int, 0, ATR_NONE,
+                 disco_orders_descr[i],
+                 (disco_order_let[i] == flags.discosort)
+                    ? MENU_ITEMFLAGS_SELECTED
+                    : MENU_ITEMFLAGS_NONE);
+    }
+    if (mode == 2) {
+        /* called via 'm `' where full alphabetize doesn't make sense
+           (only showing one class so can't span all classes) but the
+           chosen sort will stick and also apply to '\' usage */
+        any = cg.zeroany;
+        add_menu(tmpwin, NO_GLYPH, &any, 0, 0, ATR_NONE,
+                 "", MENU_ITEMFLAGS_NONE);
+        add_menu(tmpwin, NO_GLYPH, &any, 0, 0, ATR_NONE,
+                 "Note: full alphabetical and alphabetical within class",
+                 MENU_ITEMFLAGS_NONE);
+        add_menu(tmpwin, NO_GLYPH, &any, 0, 0, ATR_NONE,
+                 "      are equivalent for single class discovery, but",
+                 MENU_ITEMFLAGS_NONE);
+        add_menu(tmpwin, NO_GLYPH, &any, 0, 0, ATR_NONE,
+                 "      will matter for future use of total discoveries.",
+                 MENU_ITEMFLAGS_NONE);
+    }
+    end_menu(tmpwin, "Ordering of discoveries");
+
+    n = select_menu(tmpwin, PICK_ONE, &selected);
+    destroy_nhwindow(tmpwin);
+    if (n > 0) {
+        choice = selected[0].item.a_int;
+        /* skip preselected entry if we have more than one item chosen */
+        if (n > 1 && choice == (int) flags.discosort)
+            choice = selected[1].item.a_int;
+        free((genericptr_t) selected);
+        flags.discosort = choice;
+    }
+    return n;
+}
+
 /* the '\' command - show discovered object types */
 int
 dodiscovered() /* free after Robert Viduya */
 {
-    register int i, dis;
-    int ct = 0;
-    char *s, oclass, prev_class, classes[MAXOCLASSES], buf[BUFSZ];
     winid tmpwin;
+    char *s, *p, oclass, prev_class,
+         classes[MAXOCLASSES], buf[BUFSZ],
+         *sorted_lines[NUM_OBJECTS]; /* overkill */
+    int i, j, sortindx, dis, ct, uniq_ct, arti_ct, sorted_ct;
+    boolean alphabetized, alphabyclass, lootsort;
+
+    if (!flags.discosort || !(p = index(disco_order_let, flags.discosort)))
+        flags.discosort = 'o';
+
+    if (iflags.menu_requested) {
+        if (choose_disco_sort(1) < 0)
+            return 0;
+    }
+    alphabyclass = (flags.discosort == 'c');
+    alphabetized = (flags.discosort == 'a' || alphabyclass);
+    lootsort = (flags.discosort == 's');
+    sortindx = index(disco_order_let, flags.discosort) - disco_order_let;
 
     tmpwin = create_nhwindow(NHW_MENU);
-    putstr(tmpwin, 0, "Discoveries");
+    Sprintf(buf, "Discoveries, %s", disco_orders_descr[sortindx]);
+    putstr(tmpwin, 0, buf);
     putstr(tmpwin, 0, "");
 
     /* gather "unique objects" into a pseudo-class; note that they'll
        also be displayed individually within their regular class */
+    uniq_ct = 0;
     for (i = dis = 0; i < SIZE(uniq_objs); i++)
         if (objects[uniq_objs[i]].oc_name_known) {
             if (!dis++)
                 putstr(tmpwin, iflags.menu_headings, "Unique items");
+            ++uniq_ct;
             Sprintf(buf, "  %s", OBJ_NAME(objects[uniq_objs[i]]));
             putstr(tmpwin, 0, buf);
-            ++ct;
         }
     /* display any known artifacts as another pseudo-class */
-    ct += disp_artifact_discoveries(tmpwin);
+    arti_ct = disp_artifact_discoveries(tmpwin);
 
     /* several classes are omitted from packorder; one is of interest here */
     Strcpy(classes, flags.inv_order);
     if (!index(classes, VENOM_CLASS))
         (void) strkitten(classes, VENOM_CLASS); /* append char to string */
 
+    ct = uniq_ct + arti_ct;
+    sorted_ct = 0;
     for (s = classes; *s; s++) {
         oclass = *s;
         prev_class = oclass + 1; /* forced different from oclass */
@@ -504,21 +636,63 @@ dodiscovered() /* free after Robert Viduya */
             if ((dis = g.disco[i]) != 0 && interesting_to_discover(dis)) {
                 ct++;
                 if (oclass != prev_class) {
-                    putstr(tmpwin, iflags.menu_headings,
-                           let_to_name(oclass, FALSE, FALSE));
-                    prev_class = oclass;
+                    if ((alphabyclass || lootsort) && sorted_ct) {
+                        /* output previous class */
+                        qsort(sorted_lines, sorted_ct, sizeof (char *),
+                              discovered_cmp);
+                        for (j = 0; j < sorted_ct; ++j) {
+                            p = sorted_lines[j];
+                            if (lootsort) {
+                                p[6] = p[0]; /* '*' or ' ' */
+                                p += 6;
+                            }
+                            putstr(tmpwin, 0, p);
+                            free(sorted_lines[j]), sorted_lines[j] = 0;
+                        }
+                        sorted_ct = 0;
+                    }
+                    if (!alphabetized || alphabyclass) {
+                        /* header for new class */
+                        putstr(tmpwin, iflags.menu_headings,
+                               let_to_name(oclass, FALSE, FALSE));
+                        prev_class = oclass;
+                    }
                 }
-                Sprintf(buf, "%s %s",
-                        (objects[dis].oc_pre_discovered ? "*" : " "),
-                        obj_typename(dis));
-                putstr(tmpwin, 0, buf);
+                Strcpy(buf,  objects[dis].oc_pre_discovered ? "* " : "  ");
+                if (lootsort)
+                    (void) sortloot_descr(dis, &buf[2]);
+                Strcat(buf, obj_typename(dis));
+
+                if (!alphabetized && !lootsort)
+                    putstr(tmpwin, 0, buf);
+                else
+                    sorted_lines[sorted_ct++] = dupstr(buf);
             }
         }
     }
     if (ct == 0) {
         You("haven't discovered anything yet...");
-    } else
+    } else {
+        if (sorted_ct) {
+            /* if we're alphabetizing by class, we've already shown the
+               relevant header above; if we're alphabetizing across all
+               classes, we normally don't need a header; but it we showed
+               any unique items or any artifacts then we do need one */
+            if ((uniq_ct || arti_ct) && !alphabyclass)
+                putstr(tmpwin, iflags.menu_headings, "Discovered items");
+            qsort(sorted_lines, sorted_ct, sizeof (char *), discovered_cmp);
+            for (j = 0; j < sorted_ct; ++j) {
+                p = sorted_lines[j];
+                if (lootsort) {
+                    p[6] = p[0]; /* '*' or ' ' */
+                    p += 6;
+                }
+                putstr(tmpwin, 0, p);
+                free(sorted_lines[j]), sorted_lines[j] = 0;
+            }
+        }
         display_nhwindow(tmpwin, TRUE);
+    }
     destroy_nhwindow(tmpwin);
 
     return 0;
@@ -547,13 +721,24 @@ doclassdisco()
         havent_discovered_any[] = "haven't discovered any %s yet.",
         unique_items[] = "unique items",
         artifact_items[] = "artifacts";
-    char *s, c, oclass, menulet, allclasses[MAXOCLASSES],
-        discosyms[2 + MAXOCLASSES + 1], buf[BUFSZ];
-    int i, ct, dis, xtras;
-    boolean traditional;
     winid tmpwin = WIN_ERR;
-    anything any;
     menu_item *pick_list = 0;
+    anything any;
+    char *p, *s, c, oclass, menulet, allclasses[MAXOCLASSES],
+         discosyms[2 + MAXOCLASSES + 1], buf[BUFSZ],
+         *sorted_lines[NUM_OBJECTS]; /* overkill */
+    int i, ct, dis, xtras, sorted_ct;
+    boolean traditional, alphabetized, lootsort;
+
+    if (!flags.discosort || !(p = index(disco_order_let, flags.discosort)))
+        flags.discosort = 'o';
+
+    if (iflags.menu_requested) {
+        if (choose_disco_sort(2) < 0)
+            return 0;
+    }
+    alphabetized = (flags.discosort == 'a' || flags.discosort == 'c');
+    lootsort = (flags.discosort == 's');
 
     discosyms[0] = '\0';
     traditional = (flags.menu_style == MENU_TRADITIONAL
@@ -670,9 +855,9 @@ doclassdisco()
                upstart(strcpy(buf, unique_items)));
         for (i = 0; i < SIZE(uniq_objs); i++)
             if (objects[uniq_objs[i]].oc_name_known) {
+                ++ct;
                 Sprintf(buf, "  %s", OBJ_NAME(objects[uniq_objs[i]]));
                 putstr(tmpwin, 0, buf);
-                ++ct;
             }
         if (!ct)
             You(havent_discovered_any, unique_items);
@@ -685,20 +870,40 @@ doclassdisco()
         break;
     default:
         oclass = def_char_to_objclass(c);
-        Sprintf(buf, "Discovered %s", let_to_name(oclass, FALSE, FALSE));
-        putstr(tmpwin, iflags.menu_headings, buf);
-        for (i = g.bases[(int) oclass];
-             i < NUM_OBJECTS && objects[i].oc_class == oclass; ++i) {
+        Sprintf(buf, "Discovered %s in %s", let_to_name(oclass, FALSE, FALSE),
+                (flags.discosort == 'o') ? "order of discovery"
+                : (flags.discosort == 's') ? "'sortloot' order"
+                  : "alphabetical order");
+        putstr(tmpwin, 0, buf); /* skip iflags.menu_headings */
+        sorted_ct = 0;
+        for (i = g.bases[(int) oclass]; i < g.bases[oclass + 1] - 1; ++i) {
             if ((dis = g.disco[i]) != 0 && interesting_to_discover(dis)) {
-                Sprintf(buf, "%s %s",
-                        objects[dis].oc_pre_discovered ? "*" : " ",
-                        obj_typename(dis));
-                putstr(tmpwin, 0, buf);
                 ++ct;
+                Strcpy(buf,  objects[dis].oc_pre_discovered ? "* " : "  ");
+                if (lootsort)
+                    (void) sortloot_descr(dis, &buf[2]);
+                Strcat(buf, obj_typename(dis));
+
+                if (!alphabetized && !lootsort)
+                    putstr(tmpwin, 0, buf);
+                else
+                    sorted_lines[sorted_ct++] = dupstr(buf);
             }
         }
-        if (!ct)
+        if (!ct) {
             You(havent_discovered_any, oclass_to_name(oclass, buf));
+        } else if (sorted_ct) {
+            qsort(sorted_lines, sorted_ct, sizeof (char *), discovered_cmp);
+            for (i = 0; i < sorted_ct; ++i) {
+                p = sorted_lines[i];
+                if (lootsort) {
+                    p[6] = p[0]; /* '*' or ' ' */
+                    p += 6;
+                }
+                putstr(tmpwin, 0, p);
+                free(sorted_lines[i]), sorted_lines[i] = 0;
+            }
+        }
         break;
     }
     if (ct)

src/options.c

@@ -3256,6 +3256,66 @@ char *op;
     return optn_ok;
 }
 
+int
+optfn_sortdiscoveries(optidx, req, negated, opts, op)
+int optidx;
+int req;
+boolean negated;
+char *opts;
+char *op;
+{
+    if (req == do_init) {
+        flags.discosort = 'o';
+        return optn_ok;
+    }
+    if (req == do_set) {
+        op = string_for_env_opt(allopt[optidx].name, opts, FALSE);
+        if (negated) {
+            flags.discosort = 'o';
+        } else if (op != empty_optstr) {
+            switch (lowc(*op)) {
+            case '0':
+            case 'o': /* order of discovery */
+                flags.discosort = 'o';
+                break;
+            case '1':
+            case 's': /* sortloot order (subclasses for some classes) */
+                flags.discosort = 's';
+                break;
+            case '2':
+            case 'c': /* alphabetical within each class */
+                flags.discosort = 'c';
+                break;
+            case '3':
+            case 'a': /* alphabetical across all classes */
+                flags.discosort = 'a';
+                break;
+            default:
+                config_error_add("Unknown %s parameter '%s'",
+                                 allopt[optidx].name, op);
+                return optn_silenterr;
+            }
+        } else
+            return optn_err;
+        return optn_ok;
+    }
+    if (req == get_val) {
+        extern const char *const disco_orders_descr[]; /* o_init.c */
+        extern const char disco_order_let[];
+        const char *p = index(disco_order_let, flags.discosort);
+
+        if (!p)
+            flags.discosort = 'o', p = disco_order_let;
+        Strcpy(opts, disco_orders_descr[p - disco_order_let]);
+        return optn_ok;
+    }
+    if (req == do_handler) {
+        /* return handler_sortdiscoveries(); */
+        (void) choose_disco_sort(0); /* o_init.c */
+    }
+    return optn_ok;
+}
+
 int
 optfn_sortloot(optidx, req, negated, opts, op)
 int optidx;