From 403f2663c2ee122a77d0dd6b3205b31327c327c1 Mon Sep 17 00:00:00 2001
From: Stefan Zermatten <stefanzermatten@gmail.com>
Date: Tue, 12 Jan 2021 12:54:02 +0200
Subject: [PATCH] Fixed bugs with item display, equipment will now
 automatically move to the first property with the 'equipment' tag, carried
 items will move to the first property with the 'carried' tag

---
 .../api/creature/CreatureProperties.js        |  12 +-
 .../assertPropertyEditPermission.js           |   7 +
 .../getClosestPropertyAncestorCreature.js     |   7 +
 .../getClosestPropertyAncestorCreatureId.js   |  13 ++
 .../creatureProperties/manageEquipment.js     |  64 +++++++++
 .../recomputeInactiveProperties.js            |  33 +++--
 app/imports/constants/INVENTORY_TAGS.js       |   7 +
 .../characterSheetTabs/InventoryTab.vue       |  54 +++++---
 .../CreaturePropertyDialog.vue                | 129 ++++++++++--------
 .../components/inventory/ContainerCard.vue    |  15 +-
 .../components/inventory/ItemList.vue         |   2 +-
 11 files changed, 244 insertions(+), 99 deletions(-)
 create mode 100644 app/imports/api/creature/creatureProperties/assertPropertyEditPermission.js
 create mode 100644 app/imports/api/creature/creatureProperties/getClosestPropertyAncestorCreature.js
 create mode 100644 app/imports/api/creature/creatureProperties/getClosestPropertyAncestorCreatureId.js
 create mode 100644 app/imports/api/creature/creatureProperties/manageEquipment.js
 create mode 100644 app/imports/constants/INVENTORY_TAGS.js

diff --git a/app/imports/api/creature/CreatureProperties.js b/app/imports/api/creature/CreatureProperties.js
index 2581ab6c..85af5e32 100644
--- a/app/imports/api/creature/CreatureProperties.js
+++ b/app/imports/api/creature/CreatureProperties.js
@@ -22,6 +22,7 @@ import { storedIconsSchema } from '/imports/api/icons/Icons.js';
 import { reorderDocs } from '/imports/api/parenting/order.js';
 
 import '/imports/api/creature/actions/doAction.js';
+import '/imports/api/creature/creatureProperties/manageEquipment.js';
 
 let CreatureProperties = new Mongo.Collection('creatureProperties');
 
@@ -50,6 +51,15 @@ let CreaturePropertySchema = new SimpleSchema({
   inactive: {
     type: Boolean,
     optional: true,
+    index: 1,
+  },
+  // Denormalised flag if this property was made inactive by an inactive
+  // ancestor. True if this property has an inactive ancestor even if this
+  // property is itself inactive
+  deactivatedByAncestor: {
+    type: Boolean,
+    optional: true,
+    index: 1,
   },
 });
 
@@ -205,7 +215,7 @@ const insertPropertyFromLibraryNode = new ValidatedMethod({
 const updateProperty = new ValidatedMethod({
   name: 'creatureProperties.update',
   validate({_id, path}){
-		if (!_id) return false;
+		if (!_id) throw new Meteor.Error('No _id', '_id is required');
 		// We cannot change these fields with a simple update
 		switch (path[0]){
 			case 'type':
diff --git a/app/imports/api/creature/creatureProperties/assertPropertyEditPermission.js b/app/imports/api/creature/creatureProperties/assertPropertyEditPermission.js
new file mode 100644
index 00000000..5f91cf21
--- /dev/null
+++ b/app/imports/api/creature/creatureProperties/assertPropertyEditPermission.js
@@ -0,0 +1,7 @@
+import { assertEditPermission } from '/imports/api/sharing/sharingPermissions.js';
+import getClosestPropertyAncestorCreature from '/imports/api/creature/creatureProperties/getClosestPropertyAncestorCreature.js';
+
+export default function assertPropertyEditPermission(prop, userId){
+  let creature = getClosestPropertyAncestorCreature(prop);
+  assertEditPermission(creature, userId);
+}
diff --git a/app/imports/api/creature/creatureProperties/getClosestPropertyAncestorCreature.js b/app/imports/api/creature/creatureProperties/getClosestPropertyAncestorCreature.js
new file mode 100644
index 00000000..9e8e3145
--- /dev/null
+++ b/app/imports/api/creature/creatureProperties/getClosestPropertyAncestorCreature.js
@@ -0,0 +1,7 @@
+import Creatures from '/imports/api/creature/Creatures.js';
+import getClosestPropertyAncestorCreatureId from '/imports/api/creature/creatureProperties/getClosestPropertyAncestorCreatureId.js';
+
+export default function getClosestPropertyAncestorCreature(prop){
+  let creatureId = getClosestPropertyAncestorCreatureId(prop);
+  return Creatures.findOne(creatureId);
+}
diff --git a/app/imports/api/creature/creatureProperties/getClosestPropertyAncestorCreatureId.js b/app/imports/api/creature/creatureProperties/getClosestPropertyAncestorCreatureId.js
new file mode 100644
index 00000000..1a6b8836
--- /dev/null
+++ b/app/imports/api/creature/creatureProperties/getClosestPropertyAncestorCreatureId.js
@@ -0,0 +1,13 @@
+export default function getClosestPropertyAncestorCreatureId(prop){
+  if (!prop.ancestors) throw 'Property has no ancestors';
+  let creatureId;
+  // Find the last ancestor in the creature collection
+  for (let i = prop.ancestors.length - 1; i >= 0; i--){
+    if (prop.ancestors[i].collection === 'creatures'){
+      creatureId = prop.ancestors[i].id;
+      break;
+    }
+  }
+  if (!creatureId) throw 'This property has no creature ancestors';
+  return creatureId;
+}
diff --git a/app/imports/api/creature/creatureProperties/manageEquipment.js b/app/imports/api/creature/creatureProperties/manageEquipment.js
new file mode 100644
index 00000000..a5f88759
--- /dev/null
+++ b/app/imports/api/creature/creatureProperties/manageEquipment.js
@@ -0,0 +1,64 @@
+import { ValidatedMethod } from 'meteor/mdg:validated-method';
+import CreatureProperties from '/imports/api/creature/CreatureProperties.js';
+import { RateLimiterMixin } from 'ddp-rate-limiter-mixin';
+import { assertEditPermission } from '/imports/api/sharing/sharingPermissions.js';
+import { organizeDoc } from '/imports/api/parenting/organizeMethods.js';
+import getClosestPropertyAncestorCreature from '/imports/api/creature/creatureProperties/getClosestPropertyAncestorCreature.js';
+import INVENTORY_TAGS from '/imports/constants/INVENTORY_TAGS.js';
+
+function getParentRefByTag(creatureId, tag){
+  let prop = CreatureProperties.findOne({
+    'ancestors.id': creatureId,
+    removed: {$ne: true},
+    inactive: {$ne: true},
+    tags: tag,
+  }, {
+    sort: {order: 1},
+  });
+  if (prop){
+    return {id: prop._id, collection: 'creatureProperties'};
+  } else {
+    return {id: creatureId, collection: 'creatures'};
+  }
+}
+
+// Equipping or unequipping an item will also change its parent
+const equipItem = new ValidatedMethod({
+  name: 'creatureProperties.equip',
+  validate({_id, equipped}){
+		if (!_id) throw new Meteor.Error('No _id', '_id is required');
+    if (equipped !== true && equipped !== false) {
+      throw new Meteor.Error('No equipped', 'equipped is required to be true or false');
+    }
+  },
+  mixins: [RateLimiterMixin],
+  rateLimit: {
+    numRequests: 5,
+    timeInterval: 5000,
+  },
+  run({_id, equipped}) {
+    let item = CreatureProperties.findOne(_id);
+    if (item.type !== 'item') throw new Meteor.Error('wrong type',
+    'Equip and unequip can only be performed on items');
+    let creature = getClosestPropertyAncestorCreature(item);
+    assertEditPermission(creature, this.userId);
+    CreatureProperties.update(_id, {
+      $set: {equipped},
+    }, {
+			selector: {type: 'item'},
+		});
+    let tag = equipped ? INVENTORY_TAGS.equipment : INVENTORY_TAGS.carried;
+    let parentRef = getParentRefByTag(creature._id, tag);
+    // organizeDoc handles recompuation
+    organizeDoc.call({
+      docRef: {
+        id: _id,
+        collection: 'creatureProperties',
+      },
+      parentRef,
+      order: Number.MAX_SAFE_INTEGER,
+    });
+  },
+});
+
+export { equipItem, getParentRefByTag }
diff --git a/app/imports/api/creature/denormalise/recomputeInactiveProperties.js b/app/imports/api/creature/denormalise/recomputeInactiveProperties.js
index eb988f8b..dbdb976c 100644
--- a/app/imports/api/creature/denormalise/recomputeInactiveProperties.js
+++ b/app/imports/api/creature/denormalise/recomputeInactiveProperties.js
@@ -15,28 +15,43 @@ export default function recomputeInactiveProperties(ancestorId){
     fields: {_id: 1},
   }).map(prop => prop._id);
 
-  // Set all the properties inactive that aren't already inactive but should be
+  // Deactivate relevant properties
+  // Inactive properties
   CreatureProperties.update({
     'ancestors.id': ancestorId,
-    $or: [{
-      '_id': {$in: disabledIds}
-    }, {
-      'ancestors.id': {$in: disabledIds}
-    }],
-    inactive: {$ne: true},
+    '_id': {$in: disabledIds},
+    $or: [{inactive: {$ne: true}}, {deactivatedByAncestor: true}],
   }, {
     $set: {inactive: true},
+    $unset: {deactivatedByAncestor: 1},
   }, {
     multi: true,
     selector: {type: 'any'},
   });
+  // Decendants of inactive properties
+  CreatureProperties.update({
+    'ancestors.id': {$eq: ancestorId, $in: disabledIds},
+    $or: [{inactive: {$ne: true}}, {deactivatedByAncestor: {$ne: true}}],
+  }, {
+    $set: {
+      inactive: true,
+      deactivatedByAncestor: true,
+    },
+  }, {
+    multi: true,
+    selector: {type: 'any'},
+  });
+
   // Remove inactive from all the properties that are inactive but shouldn't be
   CreatureProperties.update({
     'ancestors.id': {$eq: ancestorId, $nin: disabledIds},
     '_id': {$nin: disabledIds},
-    inactive: true,
+    $or: [{inactive: true}, {deactivatedByAncestor: true}],
   }, {
-    $unset: {inactive: 1},
+    $unset: {
+      inactive: 1,
+      deactivatedByAncestor: 1,
+    },
   }, {
     multi: true,
     selector: {type: 'any'},
diff --git a/app/imports/constants/INVENTORY_TAGS.js b/app/imports/constants/INVENTORY_TAGS.js
new file mode 100644
index 00000000..25a00cc9
--- /dev/null
+++ b/app/imports/constants/INVENTORY_TAGS.js
@@ -0,0 +1,7 @@
+const INVENTORY_TAGS = Object.freeze({
+  inventory: 'inventory',
+  equipment: 'equipment',
+  carried: 'carried',
+});
+
+export default INVENTORY_TAGS;
diff --git a/app/imports/ui/creature/character/characterSheetTabs/InventoryTab.vue b/app/imports/ui/creature/character/characterSheetTabs/InventoryTab.vue
index 3f29124c..70a1cb70 100644
--- a/app/imports/ui/creature/character/characterSheetTabs/InventoryTab.vue
+++ b/app/imports/ui/creature/character/characterSheetTabs/InventoryTab.vue
@@ -12,7 +12,7 @@
             <item-list
               equipment
               :items="equippedItems"
-              :parent-ref="{id: creatureId, collection: 'creatures'}"
+              :parent-ref="equipmentParentRef"
             />
           </v-card-text>
         </toolbar-card>
@@ -27,7 +27,7 @@
           <v-card-text class="px-0">
             <item-list
               :items="carriedItems"
-              :parent-ref="{id: creatureId, collection: 'creatures'}"
+              :parent-ref="carriedParentRef"
             />
           </v-card-text>
         </toolbar-card>
@@ -51,7 +51,8 @@ import ContainerCard from '/imports/ui/properties/components/inventory/Container
 import ToolbarCard from '/imports/ui/components/ToolbarCard.vue';
 import ItemList from '/imports/ui/properties/components/inventory/ItemList.vue';
 import { updateProperty } from '/imports/api/creature/CreatureProperties.js';
-import getActiveProperties from '/imports/api/creature/getActiveProperties.js';
+import { getParentRefByTag } from '/imports/api/creature/creatureProperties/manageEquipment.js';
+import INVENTORY_TAGS from '/imports/constants/INVENTORY_TAGS.js';
 
 export default {
 	components: {
@@ -78,6 +79,7 @@ export default {
 				'ancestors.id': this.creatureId,
 				type: 'container',
 				removed: {$ne: true},
+        inactive: {$ne: true},
 			}, {
 				sort: {order: 1},
 			});
@@ -90,29 +92,43 @@ export default {
 				},
 				type: 'container',
 				removed: {$ne: true},
+        inactive: {$ne: true},
 			}, {
 				sort: {order: 1},
 			});
 		},
     carriedItems(){
-      return getActiveProperties({
-        ancestorId: this.creatureId,
-        includeUnequipped: true,
-        filter: {
-          type: 'item',
-          equipped: {$ne: true},
-          'parent.id': this.creatureId
-        },
-      });
+      return CreatureProperties.find({
+        'ancestors.id': {
+					$eq: this.creatureId,
+					$nin: this.containerIds
+				},
+				type: 'item',
+        equipped: {$ne: true},
+				removed: {$ne: true},
+        deactivatedByAncestor: {$ne: true},
+			}, {
+				sort: {order: 1},
+			});
     },
     equippedItems(){
-      return getActiveProperties({
-        ancestorId: this.creatureId,
-        filter: {
-          type: 'item',
-          equipped: true,
-        },
-      });
+      return CreatureProperties.find({
+        'ancestors.id': {
+					$eq: this.creatureId,
+				},
+				type: 'item',
+        equipped: true,
+				removed: {$ne: true},
+        inactive: {$ne: true},
+			}, {
+				sort: {order: 1},
+			});
+    },
+    equipmentParentRef(){
+      return getParentRefByTag(this.creatureId, INVENTORY_TAGS.equipment);
+    },
+    carriedParentRef(){
+      return getParentRefByTag(this.creatureId, INVENTORY_TAGS.carried);
     },
 	},
 	computed: {
diff --git a/app/imports/ui/creature/creatureProperties/CreaturePropertyDialog.vue b/app/imports/ui/creature/creatureProperties/CreaturePropertyDialog.vue
index df8f118b..c3868b73 100644
--- a/app/imports/ui/creature/creatureProperties/CreaturePropertyDialog.vue
+++ b/app/imports/ui/creature/creatureProperties/CreaturePropertyDialog.vue
@@ -60,12 +60,12 @@
 
 <script>
 import CreatureProperties, {
-	updateProperty,
-	damageProperty,
+  updateProperty,
+  damageProperty,
   duplicateProperty,
-	pushToProperty,
-	pullFromProperty,
-	softRemoveProperty,
+  pushToProperty,
+  pullFromProperty,
+  softRemoveProperty,
   restoreProperty,
 } from '/imports/api/creature/CreatureProperties.js';
 import Creatures from '/imports/api/creature/Creatures.js';
@@ -79,38 +79,39 @@ import CreaturePropertiesTree from '/imports/ui/creature/creatureProperties/Crea
 import getPropertyTitle from '/imports/ui/properties/shared/getPropertyTitle.js';
 import { assertEditPermission } from '/imports/api/creature/creaturePermissions.js';
 import { get, findLast } from 'lodash';
+import { equipItem } from '/imports/api/creature/creatureProperties/manageEquipment.js';
 
 let formIndex = {};
 for (let key in propertyFormIndex){
-	formIndex[key + 'Form'] = propertyFormIndex[key];
+  formIndex[key + 'Form'] = propertyFormIndex[key];
 }
 
 let viewerIndex = {};
 for (let key in propertyViewerIndex){
-	formIndex[key + 'Viewer'] = propertyViewerIndex[key];
+  formIndex[key + 'Viewer'] = propertyViewerIndex[key];
 }
 
 export default {
-	components: {
-		...formIndex,
-		...viewerIndex,
-		PropertyIcon,
-		DialogBase,
+  components: {
+    ...formIndex,
+    ...viewerIndex,
+    PropertyIcon,
+    DialogBase,
     PropertyToolbar,
-		CreaturePropertiesTree,
-	},
-	props: {
-		_id: String,
+    CreaturePropertiesTree,
+  },
+  props: {
+    _id: String,
     embedded: Boolean, // This dialog is embedded in a page
-		startInEditTab: Boolean,
-	},
-	data(){ return {
-		editing: !!this.startInEditTab,
-	}},
-	meteor: {
-		model(){
-			return CreatureProperties.findOne(this._id);
-		},
+    startInEditTab: Boolean,
+  },
+  data(){ return {
+    editing: !!this.startInEditTab,
+  }},
+  meteor: {
+    model(){
+      return CreatureProperties.findOne(this._id);
+    },
     editPermission(){
       try {
         assertEditPermission(this.creature, Meteor.userId());
@@ -119,7 +120,7 @@ export default {
         return false;
       }
     },
-	},
+  },
   computed: {
     creature(){
       if (!this.model) return;
@@ -135,8 +136,8 @@ export default {
     name: 'context',
     include: ['creature', 'editPermission'],
   },
-	methods: {
-		getPropertyName,
+  methods: {
+    getPropertyName,
     duplicate(){
       duplicateProperty.call({_id: this._id}, (error) => {
         if (error) {
@@ -149,35 +150,43 @@ export default {
         }
       });
     },
-		change({path, value, ack}){
-			updateProperty.call({_id: this._id, path, value}, (error) =>{
+    change({path, value, ack}){
+      console.log({path, value})
+      if (path && path[0] === 'equipped'){
+        equipItem.call({_id: this._id, equipped: value}, (error) =>{
+          if (error) console.warn(error);
+          ack && ack(error && error.reason || error);
+        });
+        return;
+      }
+      updateProperty.call({_id: this._id, path, value}, (error) =>{
         if (error) console.warn(error);
-				ack && ack(error && error.reason || error);
-			});
-		},
+        ack && ack(error && error.reason || error);
+      });
+    },
     damage({operation, value, ack}){
-			damageProperty.call({_id: this._id, operation, value}, (error) =>{
+      damageProperty.call({_id: this._id, operation, value}, (error) =>{
         if (error) console.warn(error);
-				ack && ack(error && error.reason || error);
-			});
-		},
-		push({path, value, ack}){
-			pushToProperty.call({_id: this._id, path, value}, (error) =>{
+        ack && ack(error && error.reason || error);
+      });
+    },
+    push({path, value, ack}){
+      pushToProperty.call({_id: this._id, path, value}, (error) =>{
         if (error) console.warn(error);
-				ack && ack(error && error.reason || error);
-			});
-		},
-		pull({path, ack}){
-			let itemId = get(this.model, path)._id;
-			path.pop();
-			pullFromProperty.call({_id: this._id, path, itemId}, (error) =>{
+        ack && ack(error && error.reason || error);
+      });
+    },
+    pull({path, ack}){
+      let itemId = get(this.model, path)._id;
+      path.pop();
+      pullFromProperty.call({_id: this._id, path, itemId}, (error) =>{
         if (error) console.warn(error);
-				ack && ack(error && error.reason || error);
-			});
-		},
-		remove(){
+        ack && ack(error && error.reason || error);
+      });
+    },
+    remove(){
       const _id = this._id;
-			softRemoveProperty.call({_id});
+      softRemoveProperty.call({_id});
       if (this.embedded){
         this.$emit('removed');
       } else {
@@ -190,15 +199,15 @@ export default {
           restoreProperty.call({_id});
         },
       });
-		},
-		selectSubProperty(_id){
-			this.$store.commit('pushDialogStack', {
-				component: 'creature-property-dialog',
-				elementId: `tree-node-${_id}`,
-				data: {_id},
-			});
-		}
-	}
+    },
+    selectSubProperty(_id){
+      this.$store.commit('pushDialogStack', {
+        component: 'creature-property-dialog',
+        elementId: `tree-node-${_id}`,
+        data: {_id},
+      });
+    }
+  }
 };
 </script>
 
diff --git a/app/imports/ui/properties/components/inventory/ContainerCard.vue b/app/imports/ui/properties/components/inventory/ContainerCard.vue
index 731abc09..3ee931a4 100644
--- a/app/imports/ui/properties/components/inventory/ContainerCard.vue
+++ b/app/imports/ui/properties/components/inventory/ContainerCard.vue
@@ -22,7 +22,7 @@
 <script>
 import ToolbarCard from '/imports/ui/components/ToolbarCard.vue';
 import ItemList from '/imports/ui/properties/components/inventory/ItemList.vue';
-import getActiveProperties from '/imports/api/creature/getActiveProperties.js';
+import CreatureProperties from '/imports/api/creature/CreatureProperties.js';
 
 export default {
 	components: {
@@ -53,14 +53,11 @@ export default {
 	},
   meteor: {
     items(){
-      return getActiveProperties({
-        ancestorId: this.model._id,
-        includeUnequipped: true,
-        filter: {
-          type: {$in: ['item', 'container']},
-          'parent.id': this.model._id,
-          equipped: {$ne: true},
-        },
+      return CreatureProperties.find({
+        'parent.id': this.model._id,
+        type: {$in: ['item', 'container']},
+        equipped: {$ne: true},
+        deactivatedByAncestor: {$ne: true},
       });
     },
   }
diff --git a/app/imports/ui/properties/components/inventory/ItemList.vue b/app/imports/ui/properties/components/inventory/ItemList.vue
index 5eff12a3..3ba08126 100644
--- a/app/imports/ui/properties/components/inventory/ItemList.vue
+++ b/app/imports/ui/properties/components/inventory/ItemList.vue
@@ -90,7 +90,7 @@ export default {
           order = -0.5;
         }
         let doc = event.element;
-          organizeDoc.call({
+        organizeDoc.call({
           docRef: {
             id: doc._id,
             collection: 'creatureProperties',