Extensive cleanup and refactoring of the spawn command and obj-update functionality, as per #1879

CHANGELOG.md

@@ -54,6 +54,7 @@ without arguments starts a full interactive Python console.
   bugfixes.
 - Remove `dummy@example.com` as a default account email when unset, a string is no longer 
   required by Django.
+- Fixes to `spawn`, make updating an existing prototype/object work better.
 
 
 ## Evennia 0.9 (2018-2019)

evennia/commands/default/building.py

@@ -7,7 +7,7 @@ from django.db.models import Q, Min, Max
 from evennia.objects.models import ObjectDB
 from evennia.locks.lockhandler import LockException
 from evennia.commands.cmdhandler import get_and_merge_cmdsets
-from evennia.utils import create, utils, search
+from evennia.utils import create, utils, search, logger
 from evennia.utils.utils import (
     inherits_from,
     class_from_module,
@@ -3479,8 +3479,11 @@ class CmdSpawn(COMMAND_DEFAULT_CLASS):
                     "Use spawn/update <key> to apply later as needed.|n"
                 )
                 return
-            n_updated = spawner.batch_update_objects_with_prototype(
+            try:
-                prototype, objects=existing_objects)
+                n_updated = spawner.batch_update_objects_with_prototype(
+                    prototype, objects=existing_objects)
+            except Exception:
+                logger.log_trace()
             caller.msg(f"{n_updated} objects were updated.")
         return
 
@@ -3585,17 +3588,18 @@ class CmdSpawn(COMMAND_DEFAULT_CLASS):
             # check for existing prototype (exact match)
             old_prototype = self._search_prototype(prototype_key, quiet=True)
 
-            print("old_prototype", old_prototype)
-            print("new_prototype", prototype)
             diff = spawner.prototype_diff(old_prototype, prototype, homogenize=True)
-            print("diff", diff)
             diffstr = spawner.format_diff(diff)
             new_prototype_detail = self._get_prototype_detail(prototypes=[prototype])
 
             if old_prototype:
-                string = (f"|yExisting prototype \"{prototype_key}\" found. Change:|n\n{diffstr}\n"
+                if not diffstr:
-                          f"|yNew changed prototype:|n\n{new_prototype_detail}")
+                    string = f"|yAlready existing Prototype:|n\n{new_prototype_detail}\n"
-                question = "\n|yDo you want to apply the change to the existing prototype?|n [Y]/N"
+                    question = "\nThere seems to be no changes. Do you still want to (re)save? [Y]/N"
+                else:
+                    string = (f"|yExisting prototype \"{prototype_key}\" found. Change:|n\n{diffstr}\n"
+                              f"|yNew changed prototype:|n\n{new_prototype_detail}")
+                    question = "\n|yDo you want to apply the change to the existing prototype?|n [Y]/N"
             else:
                 string = f"|yCreating new prototype:|n\n{new_prototype_detail}"
                 question = "\nDo you want to continue saving? [Y]/N"
@@ -3682,7 +3686,7 @@ class CmdSpawn(COMMAND_DEFAULT_CLASS):
         try:
             for obj in spawner.spawn(prototype):
                 self.caller.msg("Spawned %s." % obj.get_display_name(self.caller))
-                if not noloc and "location" not in prototype:
+                if not prototype.get('location') and not noloc:
                     # we don't hardcode the location in the prototype (unless the user
                     # did so manually) - that would lead to it having to be 'removed' every
                     # time we try to update objects with this prototype in the future.

evennia/prototypes/menus.py

@@ -2131,12 +2131,13 @@ def _keep_diff(caller, **kwargs):
     tmp[path[-1]] = tuple(list(tmp[path[-1]][:-1]) + ["KEEP"])
 
 
-def _format_diff_text_and_options(diff, **kwargs):
+def _format_diff_text_and_options(diff, minimal=True, **kwargs):
     """
     Reformat the diff in a way suitable for the olc menu.
 
     Args:
         diff (dict): A diff as produced by `prototype_diff`.
+        minimal (bool, optional): Don't show KEEPs.
 
     Kwargs:
         any (any): Forwarded into the generated options as arguments to the callable.
@@ -2150,12 +2151,15 @@ def _format_diff_text_and_options(diff, **kwargs):
 
     def _visualize(obj, rootname, get_name=False):
         if utils.is_iter(obj):
+            if not obj:
+                return str(obj)
             if get_name:
                 return obj[0] if obj[0] else "<unset>"
             if rootname == "attrs":
                 return "{} |W=|n {} |W(category:|n {}|W, locks:|n {}|W)|n".format(*obj)
             elif rootname == "tags":
                 return "{} |W(category:|n {}|W)|n".format(obj[0], obj[1])
+
         return "{}".format(obj)
 
     def _parse_diffpart(diffpart, optnum, *args):
@@ -2166,17 +2170,33 @@ def _format_diff_text_and_options(diff, **kwargs):
             rootname = args[0]
             old, new, instruction = diffpart
             if instruction == "KEEP":
-                texts.append("   |gKEEP|W:|n {old}".format(old=_visualize(old, rootname)))
+                if not minimal:
+                    texts.append("   |gKEEP|W:|n {old}".format(old=_visualize(old, rootname)))
             else:
+                # instructions we should be able to revert by a menu choice
                 vold = _visualize(old, rootname)
                 vnew = _visualize(new, rootname)
                 vsep = "" if len(vold) < 78 else "\n"
-                vinst = "|rREMOVE|n" if instruction == "REMOVE" else "|y{}|n".format(instruction)
+
-                texts.append(
+                if instruction == "ADD":
-                    "   |c[{num}] {inst}|W:|n {old} |W->|n{sep} {new}".format(
+                    texts.append("   |c[{optnum}] |yADD|n: {new}".format(
-                        inst=vinst, num=optnum, old=vold, sep=vsep, new=vnew
+                        optnum=optnum, new=_visualize(new, rootname)))
+                elif instruction == "REMOVE" and not new:
+                    if rootname == "tags" and old[1] == protlib._PROTOTYPE_TAG_CATEGORY:
+                        # special exception for the prototype-tag mechanism
+                        # this is added post-spawn automatically and should
+                        # not be listed as REMOVE.
+                        return texts, options, optnum
+
+                    texts.append("   |c[{optnum}] |rREMOVE|n: {old}".format(
+                        optnum=optnum, old=_visualize(old, rootname)))
+                else:
+                    vinst = "|y{}|n".format(instruction)
+                    texts.append(
+                        "   |c[{num}] {inst}|W:|n {old} |W->|n{sep} {new}".format(
+                            inst=vinst, num=optnum, old=vold, sep=vsep, new=vnew
+                        )
                     )
-                )
                 options.append(
                     {
                         "key": str(optnum),
@@ -2203,11 +2223,8 @@ def _format_diff_text_and_options(diff, **kwargs):
     for root_key in sorted(diff):
         diffpart = diff[root_key]
         text, option, optnum = _parse_diffpart(diffpart, optnum, root_key)
-
         heading = "- |w{}:|n ".format(root_key)
-        if root_key in ("attrs", "tags", "permissions"):
+        if text:
-            texts.append(heading)
-        elif text:
             text = [heading + text[0]] + text[1:]
         else:
             text = [heading]

evennia/prototypes/prototypes.py

@@ -238,9 +238,6 @@ def save_prototype(prototype):
         )
 
     # make sure meta properties are included with defaults
-    stored_prototype = DbPrototype.objects.filter(db_key=prototype_key)
-    prototype = stored_prototype[0].prototype if stored_prototype else {}
-
     in_prototype["prototype_desc"] = in_prototype.get(
         "prototype_desc", prototype.get("prototype_desc", "")
     )
@@ -260,17 +257,16 @@ def save_prototype(prototype):
     ]
     in_prototype["prototype_tags"] = prototype_tags
 
-    prototype.update(in_prototype)
+    stored_prototype = DbPrototype.objects.filter(db_key=prototype_key)
-
     if stored_prototype:
         # edit existing prototype
         stored_prototype = stored_prototype[0]
-        stored_prototype.desc = prototype["prototype_desc"]
+        stored_prototype.desc = in_prototype["prototype_desc"]
         if prototype_tags:
             stored_prototype.tags.clear(category=_PROTOTYPE_TAG_CATEGORY)
-            stored_prototype.tags.batch_add(*prototype["prototype_tags"])
+            stored_prototype.tags.batch_add(*in_prototype["prototype_tags"])
-        stored_prototype.locks.add(prototype["prototype_locks"])
+        stored_prototype.locks.add(in_prototype["prototype_locks"])
-        stored_prototype.attributes.add("prototype", prototype)
+        stored_prototype.attributes.add("prototype", in_prototype)
     else:
         # create a new prototype
         stored_prototype = create_script(
@@ -280,7 +276,7 @@ def save_prototype(prototype):
             persistent=True,
             locks=prototype_locks,
             tags=prototype["prototype_tags"],
-            attributes=[("prototype", prototype)],
+            attributes=[("prototype", in_prototype)],
         )
     return stored_prototype.prototype
 

evennia/prototypes/spawner.py

@@ -138,6 +138,7 @@ from django.conf import settings
 
 import evennia
 from evennia.objects.models import ObjectDB
+from evennia.utils import logger
 from evennia.utils.utils import make_iter, is_iter
 from evennia.prototypes import prototypes as protlib
 from evennia.prototypes.prototypes import (
@@ -322,7 +323,7 @@ def prototype_from_object(obj):
     return prot
 
 
-def prototype_diff(prototype1, prototype2, maxdepth=2, homogenize=False):
+def prototype_diff(prototype1, prototype2, maxdepth=2, homogenize=False, implicit_keep=False):
     """
     A 'detailed' diff specifies differences down to individual sub-sections
     of the prototype, like individual attributes, permissions etc. It is used
@@ -336,6 +337,10 @@ def prototype_diff(prototype1, prototype2, maxdepth=2, homogenize=False):
             attr/tag (for example) are represented by a tuple.
         homogenize (bool, optional): Auto-homogenize both prototypes for the best comparison.
             This is most useful for displaying.
+        implicit_keep (bool, optional): If set, the resulting diff will assume KEEP unless the new
+            prototype explicitly change them. That is, if a key exists in `prototype1` and
+            not in `prototype2`, it will not be REMOVEd but set to KEEP instead. This is particularly
+            useful for auto-generated prototypes when updating objects.
 
     Returns:
         diff (dict): A structure detailing how to convert prototype1 to prototype2. All
@@ -346,6 +351,10 @@ def prototype_diff(prototype1, prototype2, maxdepth=2, homogenize=False):
             instruction can be one of "REMOVE", "ADD", "UPDATE" or "KEEP".
 
     """
+    class Unset:
+        def __bool__(self):
+            return False
+    _unset = Unset()
 
     def _recursive_diff(old, new, depth=0):
 
@@ -363,6 +372,9 @@ def prototype_diff(prototype1, prototype2, maxdepth=2, homogenize=False):
                     return {
                         part[0] if is_iter(part) else part: (part, None, "REMOVE") for part in old
                     }
+                if isinstance(new, Unset) and implicit_keep:
+                    # the new does not define any change, use implicit-keep
+                    return (old, None, "KEEP")
                 return (old, new, "REMOVE")
             elif not old and new:
                 if depth < maxdepth and new_type == dict:
@@ -376,7 +388,7 @@ def prototype_diff(prototype1, prototype2, maxdepth=2, homogenize=False):
         elif depth < maxdepth and new_type == dict:
             all_keys = set(list(old.keys()) + list(new.keys()))
             return {
-                key: _recursive_diff(old.get(key), new.get(key), depth=depth + 1)
+                key: _recursive_diff(old.get(key), new.get(key, _unset), depth=depth + 1)
                 for key in all_keys
             }
         elif depth < maxdepth and is_iter(new):
@@ -384,7 +396,7 @@ def prototype_diff(prototype1, prototype2, maxdepth=2, homogenize=False):
             new_map = {part[0] if is_iter(part) else part: part for part in new}
             all_keys = set(list(old_map.keys()) + list(new_map.keys()))
             return {
-                key: _recursive_diff(old_map.get(key), new_map.get(key), depth=depth + 1)
+                key: _recursive_diff(old_map.get(key), new_map.get(key, _unset), depth=depth + 1)
                 for key in all_keys
             }
         elif old != new:
@@ -468,7 +480,7 @@ def flatten_diff(diff):
     return flat_diff
 
 
-def prototype_diff_from_object(prototype, obj):
+def prototype_diff_from_object(prototype, obj, implicit_keep=True):
     """
     Get a simple diff for a prototype compared to an object which may or may not already have a
     prototype (or has one but changed locally). For more complex migratations a manual diff may be
@@ -482,6 +494,11 @@ def prototype_diff_from_object(prototype, obj):
         diff (dict): Mapping for every prototype key: {"keyname": "REMOVE|UPDATE|KEEP", ...}
         obj_prototype (dict): The prototype calculated for the given object. The diff is how to
             convert this prototype into the new prototype.
+        implicit_keep (bool, optional): This is usually what one wants for object updating. When
+            set, this means the prototype diff will assume KEEP on differences
+            between the object-generated prototype and that which is not explicitly set in the
+            new prototype. This means e.g. that even though the object has a location, and the
+            prototype does not specify the location, it will not be unset.
 
     Notes:
         The `diff` is on the following form:
@@ -494,7 +511,8 @@ def prototype_diff_from_object(prototype, obj):
 
     """
     obj_prototype = prototype_from_object(obj)
-    diff = prototype_diff(obj_prototype, protlib.homogenize_prototype(prototype))
+    diff = prototype_diff(obj_prototype, protlib.homogenize_prototype(prototype),
+                          implicit_keep=implicit_keep)
     return diff, obj_prototype
 
 
@@ -511,6 +529,7 @@ def format_diff(diff, minimal=True):
         texts (str): The formatted text.
 
     """
+
     valid_instructions = ("KEEP", "REMOVE", "ADD", "UPDATE")
 
     def _visualize(obj, rootname, get_name=False):
@@ -572,7 +591,7 @@ def format_diff(diff, minimal=True):
     return "\n ".join(line for line in texts if line)
 
 
-def batch_update_objects_with_prototype(prototype, diff=None, objects=None):
+def batch_update_objects_with_prototype(prototype, diff=None, objects=None, exact=False):
     """
     Update existing objects with the latest version of the prototype.
 
@@ -583,6 +602,12 @@ def batch_update_objects_with_prototype(prototype, diff=None, objects=None):
             If not given this will be constructed from the first object found.
         objects (list, optional): List of objects to update. If not given, query for these
             objects using the prototype's `prototype_key`.
+        exact (bool, optional): By default (`False`), keys not explicitly in the prototype will
+            not be applied to the object, but will be retained as-is. This is usually what is
+            expected - for example, one usually do not want to remove the object's location even
+            if it's not set in the prototype. With `exact=True`, all un-specified properties of the
+            objects will be removed if they exist. This will lead to a more accurate 1:1 correlation
+            between the  object and the prototype but is usually impractical.
     Returns:
         changed (int): The number of objects that had changes applied to them.
 
@@ -615,98 +640,108 @@ def batch_update_objects_with_prototype(prototype, diff=None, objects=None):
         old_prot_key = obj.tags.get(category=_PROTOTYPE_TAG_CATEGORY, return_list=True)
         old_prot_key = old_prot_key[0] if old_prot_key else None
 
-        for key, directive in diff.items():
+        try:
-            if directive in ("UPDATE", "REPLACE"):
+            for key, directive in diff.items():
 
-                if key in _PROTOTYPE_META_NAMES:
+                if key not in new_prototype and not exact:
-                    # prototype meta keys are not stored on-object
+                    # we don't update the object if the prototype does not actually
+                    # contain the key (the diff will report REMOVE but we ignore it
+                    # since exact=False)
                     continue
 
-                val = new_prototype[key]
+                if directive in ("UPDATE", "REPLACE"):
-                do_save = True
 
-                if key == "key":
+                    if key in _PROTOTYPE_META_NAMES:
-                    obj.db_key = init_spawn_value(val, str)
+                        # prototype meta keys are not stored on-object
-                elif key == "typeclass":
+                        continue
-                    obj.db_typeclass_path = init_spawn_value(val, str)
+
-                elif key == "location":
+                    val = new_prototype[key]
-                    obj.db_location = init_spawn_value(val, value_to_obj)
+                    do_save = True
-                elif key == "home":
+
-                    obj.db_home = init_spawn_value(val, value_to_obj)
+                    if key == "key":
-                elif key == "destination":
+                        obj.db_key = init_spawn_value(val, str)
-                    obj.db_destination = init_spawn_value(val, value_to_obj)
+                    elif key == "typeclass":
-                elif key == "locks":
+                        obj.db_typeclass_path = init_spawn_value(val, str)
-                    if directive == "REPLACE":
+                    elif key == "location":
-                        obj.locks.clear()
+                        obj.db_location = init_spawn_value(val, value_to_obj)
-                    obj.locks.add(init_spawn_value(val, str))
+                    elif key == "home":
-                elif key == "permissions":
+                        obj.db_home = init_spawn_value(val, value_to_obj)
-                    if directive == "REPLACE":
+                    elif key == "destination":
-                        obj.permissions.clear()
+                        obj.db_destination = init_spawn_value(val, value_to_obj)
-                    obj.permissions.batch_add(*(init_spawn_value(perm, str) for perm in val))
+                    elif key == "locks":
-                elif key == "aliases":
+                        if directive == "REPLACE":
-                    if directive == "REPLACE":
+                            obj.locks.clear()
-                        obj.aliases.clear()
+                        obj.locks.add(init_spawn_value(val, str))
-                    obj.aliases.batch_add(*(init_spawn_value(alias, str) for alias in val))
+                    elif key == "permissions":
-                elif key == "tags":
+                        if directive == "REPLACE":
-                    if directive == "REPLACE":
+                            obj.permissions.clear()
-                        obj.tags.clear()
+                        obj.permissions.batch_add(*(init_spawn_value(perm, str) for perm in val))
-                    obj.tags.batch_add(
+                    elif key == "aliases":
-                        *(
+                        if directive == "REPLACE":
-                            (init_spawn_value(ttag, str), tcategory, tdata)
+                            obj.aliases.clear()
-                            for ttag, tcategory, tdata in val
+                        obj.aliases.batch_add(*(init_spawn_value(alias, str) for alias in val))
-                        )
+                    elif key == "tags":
-                    )
+                        if directive == "REPLACE":
-                elif key == "attrs":
+                            obj.tags.clear()
-                    if directive == "REPLACE":
+                        obj.tags.batch_add(
-                        obj.attributes.clear()
+                            *(
-                    obj.attributes.batch_add(
+                                (init_spawn_value(ttag, str), tcategory, tdata)
-                        *(
+                                for ttag, tcategory, tdata in val
-                            (
-                                init_spawn_value(akey, str),
-                                init_spawn_value(aval, value_to_obj),
-                                acategory,
-                                alocks,
                             )
-                            for akey, aval, acategory, alocks in val
                         )
-                    )
+                    elif key == "attrs":
-                elif key == "exec":
+                        if directive == "REPLACE":
-                    # we don't auto-rerun exec statements, it would be huge security risk!
+                            obj.attributes.clear()
-                    pass
+                        obj.attributes.batch_add(
-                else:
+                            *(
-                    obj.attributes.add(key, init_spawn_value(val, value_to_obj))
+                                (
-            elif directive == "REMOVE":
+                                    init_spawn_value(akey, str),
-                do_save = True
+                                    init_spawn_value(aval, value_to_obj),
-                if key == "key":
+                                    acategory,
-                    obj.db_key = ""
+                                    alocks,
-                elif key == "typeclass":
+                                )
-                    # fall back to default
+                                for akey, aval, acategory, alocks in val
-                    obj.db_typeclass_path = settings.BASE_OBJECT_TYPECLASS
+                            )
-                elif key == "location":
+                        )
-                    obj.db_location = None
+                    elif key == "exec":
-                elif key == "home":
+                        # we don't auto-rerun exec statements, it would be huge security risk!
-                    obj.db_home = None
+                        pass
-                elif key == "destination":
+                    else:
-                    obj.db_destination = None
+                        obj.attributes.add(key, init_spawn_value(val, value_to_obj))
-                elif key == "locks":
+                elif directive == "REMOVE":
-                    obj.locks.clear()
+                    do_save = True
-                elif key == "permissions":
+                    if key == "key":
-                    obj.permissions.clear()
+                        obj.db_key = ""
-                elif key == "aliases":
+                    elif key == "typeclass":
-                    obj.aliases.clear()
+                        # fall back to default
-                elif key == "tags":
+                        obj.db_typeclass_path = settings.BASE_OBJECT_TYPECLASS
-                    obj.tags.clear()
+                    elif key == "location":
-                elif key == "attrs":
+                        obj.db_location = None
-                    obj.attributes.clear()
+                    elif key == "home":
-                elif key == "exec":
+                        obj.db_home = None
-                    # we don't auto-rerun exec statements, it would be huge security risk!
+                    elif key == "destination":
-                    pass
+                        obj.db_destination = None
-                else:
+                    elif key == "locks":
-                    obj.attributes.remove(key)
+                        obj.locks.clear()
-
+                    elif key == "permissions":
-        # we must always make sure to re-add the prototype tag
+                        obj.permissions.clear()
-        obj.tags.clear(category=_PROTOTYPE_TAG_CATEGORY)
+                    elif key == "aliases":
-        obj.tags.add(prototype_key, category=_PROTOTYPE_TAG_CATEGORY)
+                        obj.aliases.clear()
+                    elif key == "tags":
+                        obj.tags.clear()
+                    elif key == "attrs":
+                        obj.attributes.clear()
+                    elif key == "exec":
+                        # we don't auto-rerun exec statements, it would be huge security risk!
+                        pass
+                    else:
+                        obj.attributes.remove(key)
+        except Exception:
+            logger.log_trace(f"Failed to apply prototype '{prototype_key}' to {obj}.")
+        finally:
+            # we must always make sure to re-add the prototype tag
+            obj.tags.clear(category=_PROTOTYPE_TAG_CATEGORY)
+            obj.tags.add(prototype_key, category=_PROTOTYPE_TAG_CATEGORY)
 
         if do_save:
             changed += 1