try to make it drier

module/applications/sheets/items/armor.mjs

@@ -1,4 +1,5 @@
 import DHBaseItemSheet from '../api/base-item.mjs';
+import { copyAttachmentEffectsToActor, removeAttachmentEffectsFromActor } from '../../../helpers/attachmentHelper.mjs';
 
 export default class ArmorSheet extends DHBaseItemSheet {
     /**@inheritdoc */
@@ -130,28 +131,12 @@ export default class ArmorSheet extends DHBaseItemSheet {
         // Both attachment-only and regular effects should be copied when attached
         const actor = this.document.parent;
         if (actor && item.effects.size > 0 && this.document.system.equipped) {
-            const effectsToCreate = [];
+            await copyAttachmentEffectsToActor({
-            for (const effect of item.effects) {
+                parentItem: this.document,
-                // Copy ALL effects when item is attached - attachment-only flag only matters for non-attached items
+                attachedItem: item,
-                const effectData = effect.toObject();
+                attachedUuid: newUUID,
-                effectData.origin = `${this.document.uuid}:${newUUID}`; // Track which armor and which item this came from
+                parentType: 'armor'
-                effectData.flags = {
+            });
-                    ...effectData.flags,
-                    daggerheart: {
-                        ...effectData.flags?.daggerheart,
-                        attachmentSource: {
-                            armorUuid: this.document.uuid,
-                            itemUuid: newUUID,
-                            originalEffectId: effect.id
-                        }
-                    }
-                };
-                effectsToCreate.push(effectData);
-            }
-            
-            if (effectsToCreate.length > 0) {
-                await actor.createEmbeddedDocuments('ActiveEffect', effectsToCreate);
-            }
         }
     }
 
@@ -170,18 +155,10 @@ export default class ArmorSheet extends DHBaseItemSheet {
         });
         
         // Remove any effects on the actor that came from this attached item
-        const actor = this.document.parent;
+        await removeAttachmentEffectsFromActor({
-        if (actor) {
+            parentItem: this.document,
-            const effectsToRemove = actor.effects.filter(effect => {
+            attachedUuid: uuid,
-                const attachmentSource = effect.flags?.daggerheart?.attachmentSource;
+            parentType: 'armor'
-                return attachmentSource && 
+        });
-                       attachmentSource.armorUuid === this.document.uuid && 
-                       attachmentSource.itemUuid === uuid;
-            });
-            
-            if (effectsToRemove.length > 0) {
-                await actor.deleteEmbeddedDocuments('ActiveEffect', effectsToRemove.map(e => e.id));
-            }
-        }
     }
 }

module/applications/sheets/items/weapon.mjs

@@ -1,4 +1,5 @@
 import DHBaseItemSheet from '../api/base-item.mjs';
+import { copyAttachmentEffectsToActor, removeAttachmentEffectsFromActor } from '../../../helpers/attachmentHelper.mjs';
 
 export default class WeaponSheet extends DHBaseItemSheet {
     /**@inheritdoc */
@@ -129,28 +130,12 @@ export default class WeaponSheet extends DHBaseItemSheet {
         // Both attachment-only and regular effects should be copied when attached
         const actor = this.document.parent;
         if (actor && item.effects.size > 0 && this.document.system.equipped) {
-            const effectsToCreate = [];
+            await copyAttachmentEffectsToActor({
-            for (const effect of item.effects) {
+                parentItem: this.document,
-                // Copy ALL effects when item is attached - attachment-only flag only matters for non-attached items
+                attachedItem: item,
-                const effectData = effect.toObject();
+                attachedUuid: newUUID,
-                effectData.origin = `${this.document.uuid}:${newUUID}`; // Track which weapon and which item this came from
+                parentType: 'weapon'
-                effectData.flags = {
+            });
-                    ...effectData.flags,
-                    daggerheart: {
-                        ...effectData.flags?.daggerheart,
-                        attachmentSource: {
-                            weaponUuid: this.document.uuid,
-                            itemUuid: newUUID,
-                            originalEffectId: effect.id
-                        }
-                    }
-                };
-                effectsToCreate.push(effectData);
-            }
-            
-            if (effectsToCreate.length > 0) {
-                await actor.createEmbeddedDocuments('ActiveEffect', effectsToCreate);
-            }
         }
     }
 
@@ -169,18 +154,10 @@ export default class WeaponSheet extends DHBaseItemSheet {
         });
         
         // Remove any effects on the actor that came from this attached item
-        const actor = this.document.parent;
+        await removeAttachmentEffectsFromActor({
-        if (actor) {
+            parentItem: this.document,
-            const effectsToRemove = actor.effects.filter(effect => {
+            attachedUuid: uuid,
-                const attachmentSource = effect.flags?.daggerheart?.attachmentSource;
+            parentType: 'weapon'
-                return attachmentSource && 
+        });
-                       attachmentSource.weaponUuid === this.document.uuid && 
-                       attachmentSource.itemUuid === uuid;
-            });
-            
-            if (effectsToRemove.length > 0) {
-                await actor.deleteEmbeddedDocuments('ActiveEffect', effectsToRemove.map(e => e.id));
-            }
-        }
     }
 }

module/data/item/armor.mjs

@@ -2,6 +2,7 @@ import BaseDataItem from './base.mjs';
 import ActionField from '../fields/actionField.mjs';
 import { armorFeatures } from '../../config/itemConfig.mjs';
 import { actionsTypes } from '../action/_module.mjs';
+import { handleAttachmentEffectsOnEquipChange } from '../../helpers/attachmentHelper.mjs';
 
 export default class DHArmor extends BaseDataItem {
     /** @inheritDoc */
@@ -56,7 +57,11 @@ export default class DHArmor extends BaseDataItem {
 
         // Handle equipped status changes for attachment effects
         if (changes.system?.equipped !== undefined && changes.system.equipped !== this.equipped) {
-            await this._handleAttachmentEffectsOnEquipChange(changes.system.equipped);
+            await handleAttachmentEffectsOnEquipChange({
+                parentItem: this.parent,
+                newEquippedStatus: changes.system.equipped,
+                parentType: 'armor'
+            });
         }
 
         if (changes.system.features) {
@@ -97,54 +102,4 @@ export default class DHArmor extends BaseDataItem {
             }
         }
     }
-
-    /**
-     * Handle adding/removing attachment effects when armor is equipped/unequipped
-     * @param {boolean} newEquippedStatus - The new equipped status
-     */
-    async _handleAttachmentEffectsOnEquipChange(newEquippedStatus) {
-        const actor = this.parent.parent;
-        if (!actor || !this.attached?.length) return;
-
-        if (newEquippedStatus) {
-            // Armor is being equipped - add attachment effects
-            const effectsToCreate = [];
-            for (const attachedUuid of this.attached) {
-                const attachedItem = await fromUuid(attachedUuid);
-                if (attachedItem && attachedItem.effects.size > 0) {
-                    for (const effect of attachedItem.effects) {
-                        // Copy ALL effects when item is attached - attachment-only flag only matters for non-attached items
-                        const effectData = effect.toObject();
-                        effectData.origin = `${this.parent.uuid}:${attachedUuid}`;
-                        effectData.flags = {
-                            ...effectData.flags,
-                            daggerheart: {
-                                ...effectData.flags?.daggerheart,
-                                attachmentSource: {
-                                    armorUuid: this.parent.uuid,
-                                    itemUuid: attachedUuid,
-                                    originalEffectId: effect.id
-                                }
-                            }
-                        };
-                        effectsToCreate.push(effectData);
-                    }
-                }
-            }
-            
-            if (effectsToCreate.length > 0) {
-                await actor.createEmbeddedDocuments('ActiveEffect', effectsToCreate);
-            }
-        } else {
-            // Armor is being unequipped - remove attachment effects
-            const effectsToRemove = actor.effects.filter(effect => {
-                const attachmentSource = effect.flags?.daggerheart?.attachmentSource;
-                return attachmentSource && attachmentSource.armorUuid === this.parent.uuid;
-            });
-            
-            if (effectsToRemove.length > 0) {
-                await actor.deleteEmbeddedDocuments('ActiveEffect', effectsToRemove.map(e => e.id));
-            }
-        }
-    }
 }

module/data/item/weapon.mjs

@@ -1,6 +1,7 @@
 import BaseDataItem from './base.mjs';
 import { actionsTypes } from '../action/_module.mjs';
 import ActionField from '../fields/actionField.mjs';
+import { handleAttachmentEffectsOnEquipChange } from '../../helpers/attachmentHelper.mjs';
 
 export default class DHWeapon extends BaseDataItem {
     /** @inheritDoc */
@@ -81,7 +82,11 @@ export default class DHWeapon extends BaseDataItem {
 
         // Handle equipped status changes for attachment effects
         if (changes.system?.equipped !== undefined && changes.system.equipped !== this.equipped) {
-            await this._handleAttachmentEffectsOnEquipChange(changes.system.equipped);
+            await handleAttachmentEffectsOnEquipChange({
+                parentItem: this.parent,
+                newEquippedStatus: changes.system.equipped,
+                parentType: 'weapon'
+            });
         }
 
         if (changes.system?.features) {
@@ -122,54 +127,4 @@ export default class DHWeapon extends BaseDataItem {
             }
         }
     }
-
-    /**
-     * Handle adding/removing attachment effects when weapon is equipped/unequipped
-     * @param {boolean} newEquippedStatus - The new equipped status
-     */
-    async _handleAttachmentEffectsOnEquipChange(newEquippedStatus) {
-        const actor = this.parent.parent;
-        if (!actor || !this.attached?.length) return;
-
-        if (newEquippedStatus) {
-            // Weapon is being equipped - add attachment effects
-            const effectsToCreate = [];
-            for (const attachedUuid of this.attached) {
-                const attachedItem = await fromUuid(attachedUuid);
-                if (attachedItem && attachedItem.effects.size > 0) {
-                    for (const effect of attachedItem.effects) {
-                        // Copy ALL effects when item is attached - attachment-only flag only matters for non-attached items
-                        const effectData = effect.toObject();
-                        effectData.origin = `${this.parent.uuid}:${attachedUuid}`;
-                        effectData.flags = {
-                            ...effectData.flags,
-                            daggerheart: {
-                                ...effectData.flags?.daggerheart,
-                                attachmentSource: {
-                                    weaponUuid: this.parent.uuid,
-                                    itemUuid: attachedUuid,
-                                    originalEffectId: effect.id
-                                }
-                            }
-                        };
-                        effectsToCreate.push(effectData);
-                    }
-                }
-            }
-            
-            if (effectsToCreate.length > 0) {
-                await actor.createEmbeddedDocuments('ActiveEffect', effectsToCreate);
-            }
-        } else {
-            // Weapon is being unequipped - remove attachment effects
-            const effectsToRemove = actor.effects.filter(effect => {
-                const attachmentSource = effect.flags?.daggerheart?.attachmentSource;
-                return attachmentSource && attachmentSource.weaponUuid === this.parent.uuid;
-            });
-            
-            if (effectsToRemove.length > 0) {
-                await actor.deleteEmbeddedDocuments('ActiveEffect', effectsToRemove.map(e => e.id));
-            }
-        }
-    }
 }

module/helpers/attachmentHelper.mjs

@@ -0,0 +1,114 @@
+/**
+ * Utility functions for handling item attachments and their effects
+ */
+
+/**
+ * Copy all effects from an attached item to the actor when the parent item is equipped
+ * @param {Object} options - Configuration options
+ * @param {Item} options.parentItem - The item (armor/weapon) that the item is being attached to
+ * @param {Item} options.attachedItem - The item being attached
+ * @param {string} options.attachedUuid - UUID of the attached item
+ * @param {string} options.parentType - Type of parent item ("armor" or "weapon")
+ * @returns {Promise<ActiveEffect[]>} Created effects
+ */
+export async function copyAttachmentEffectsToActor({ parentItem, attachedItem, attachedUuid, parentType }) {
+    const actor = parentItem.parent;
+    if (!actor || !attachedItem.effects.size > 0 || !parentItem.system.equipped) {
+        return [];
+    }
+
+    const effectsToCreate = [];
+    for (const effect of attachedItem.effects) {
+        // Copy ALL effects when item is attached - attachment-only flag only matters for non-attached items
+        const effectData = effect.toObject();
+        effectData.origin = `${parentItem.uuid}:${attachedUuid}`;
+        
+        // Set up attachment source metadata with the appropriate property name
+        const attachmentSource = {
+            itemUuid: attachedUuid,
+            originalEffectId: effect.id
+        };
+        attachmentSource[`${parentType}Uuid`] = parentItem.uuid;
+        
+        effectData.flags = {
+            ...effectData.flags,
+            daggerheart: {
+                ...effectData.flags?.daggerheart,
+                attachmentSource
+            }
+        };
+        effectsToCreate.push(effectData);
+    }
+
+    if (effectsToCreate.length > 0) {
+        return await actor.createEmbeddedDocuments('ActiveEffect', effectsToCreate);
+    }
+    
+    return [];
+}
+
+/**
+ * Remove effects from the actor that came from a specific attached item
+ * @param {Object} options - Configuration options
+ * @param {Item} options.parentItem - The item (armor/weapon) that the item was attached to
+ * @param {string} options.attachedUuid - UUID of the attached item being removed
+ * @param {string} options.parentType - Type of parent item ("armor" or "weapon")
+ * @returns {Promise<void>}
+ */
+export async function removeAttachmentEffectsFromActor({ parentItem, attachedUuid, parentType }) {
+    const actor = parentItem.parent;
+    if (!actor) return;
+
+    const parentUuidProperty = `${parentType}Uuid`;
+    const effectsToRemove = actor.effects.filter(effect => {
+        const attachmentSource = effect.flags?.daggerheart?.attachmentSource;
+        return attachmentSource && 
+               attachmentSource[parentUuidProperty] === parentItem.uuid && 
+               attachmentSource.itemUuid === attachedUuid;
+    });
+
+    if (effectsToRemove.length > 0) {
+        await actor.deleteEmbeddedDocuments('ActiveEffect', effectsToRemove.map(e => e.id));
+    }
+}
+
+/**
+ * Handle adding/removing attachment effects when a parent item is equipped/unequipped
+ * @param {Object} options - Configuration options
+ * @param {Item} options.parentItem - The item (armor/weapon) being equipped/unequipped
+ * @param {boolean} options.newEquippedStatus - The new equipped status
+ * @param {string} options.parentType - Type of parent item ("armor" or "weapon")
+ * @returns {Promise<void>}
+ */
+export async function handleAttachmentEffectsOnEquipChange({ parentItem, newEquippedStatus, parentType }) {
+    const actor = parentItem.parent?.parent;
+    if (!actor || !parentItem.system.attached?.length) return;
+
+    if (newEquippedStatus) {
+        // Item is being equipped - add attachment effects
+        const effectsToCreate = [];
+        for (const attachedUuid of parentItem.system.attached) {
+            const attachedItem = await fromUuid(attachedUuid);
+            if (attachedItem && attachedItem.effects.size > 0) {
+                const newEffects = await copyAttachmentEffectsToActor({
+                    parentItem,
+                    attachedItem,
+                    attachedUuid,
+                    parentType
+                });
+                effectsToCreate.push(...newEffects);
+            }
+        }
+    } else {
+        // Item is being unequipped - remove attachment effects
+        const parentUuidProperty = `${parentType}Uuid`;
+        const effectsToRemove = actor.effects.filter(effect => {
+            const attachmentSource = effect.flags?.daggerheart?.attachmentSource;
+            return attachmentSource && attachmentSource[parentUuidProperty] === parentItem.uuid;
+        });
+
+        if (effectsToRemove.length > 0) {
+            await actor.deleteEmbeddedDocuments('ActiveEffect', effectsToRemove.map(e => e.id));
+        }
+    }
+}

templates/sheets/activeEffect/details.hbs

@@ -9,7 +9,7 @@
     {{#if isItemEffect}}
     {{formGroup fields.transfer value=source.transfer rootId=rootId label=legacyTransfer.label hint=legacyTransfer.hint}}
     
-    {{!-- Attachment-only flag for item effects --}}
+    {{!-- Attachment-only flag for item effects TODO figure out how to do this with formGroups--}}
     <div class="form-group">
         <label for="{{rootId}}-attachmentOnly">{{localize "DAGGERHEART.Effect.AttachmentOnly.Label"}}</label>
         <input type="checkbox" id="{{rootId}}-attachmentOnly" name="flags.daggerheart.attachmentOnly" {{#if source.flags.daggerheart.attachmentOnly}}checked{{/if}}>