fix: address a few problems with active effect application

lang/de.json

@@ -371,5 +371,5 @@
     "DS4.ActiveEffectApplyToItems": "Auf Items Andwenden",
     "DS4.ActiveEffectItemName": "Itemname",
     "DS4.ActiveEffectItemCondition": "Bedingung",
-    "DS4.TooltipDisabledDueToEffects": "inaktiv, weil von Aktiven Effekten beeinflusst"
+    "DS4.TooltipNotEditableDueToEffects": "Feld nicht bearbeitbar, weil von Aktiven Effekten beeinflusst"
 }

lang/en.json

@@ -371,5 +371,5 @@
     "DS4.ActiveEffectApplyToItems": "Apply to Items",
     "DS4.ActiveEffectItemName": "Item Name",
     "DS4.ActiveEffectItemCondition": "Condition",
-    "DS4.TooltipDisabledDueToEffects": "disabled, because affected by Active Effects"
+    "DS4.TooltipNotEditableDueToEffects": "field not editable, because affected by Active Effects"
 }

scss/components/actor/_actor_sheet.scss

@@ -5,6 +5,6 @@
  */
 
 .ds4-actor-sheet {
-    min-height: 625px;
+    min-height: 635px;
     min-width: 650px;
 }

src/active-effect/active-effect.ts

@@ -76,7 +76,6 @@ export class DS4ActiveEffect extends ActiveEffect {
         try {
             change.value = DS4ActiveEffect.safeEval(change.value).toString();
         } catch (e) {
-            logger.warn(e);
             // this is a valid case, e.g., if the effect change simply is a string
         }
         // eslint-disable-next-line @typescript-eslint/ban-ts-comment

src/actor/actor-sheet.ts

@@ -25,7 +25,7 @@ export class DS4ActorSheet extends ActorSheet<ActorSheet.Options, DS4ActorSheetD
     static override get defaultOptions(): ActorSheet.Options {
         return foundry.utils.mergeObject(super.defaultOptions, {
             classes: ["sheet", "ds4-actor-sheet"],
-            height: 625,
+            height: 635,
             scrollY: [".ds4-sheet-body"],
             tabs: [{ navSelector: ".ds4-sheet-tab-nav", contentSelector: ".ds4-sheet-body", initial: "values" }],
             dragDrop: [
@@ -111,7 +111,14 @@ export class DS4ActorSheet extends ActorSheet<ActorSheet.Options, DS4ActorSheetD
 
         html.find(".sort-items").on("click", this.onSortItems.bind(this));
 
-        disableOverriddenFields.call(this);
+        disableOverriddenFields(this.form, this.actor.overrides, (key) => `[name="${key}"]`);
+        for (const item of this.actor.items) {
+            disableOverriddenFields(
+                this.form,
+                item.overrides,
+                (key) => `[data-item-id="${item.id}"] .change-item[data-property="${key}"]`,
+            );
+        }
     }
 
     /**
@@ -359,6 +366,7 @@ export class DS4ActorSheet extends ActorSheet<ActorSheet.Options, DS4ActorSheetD
         enforce(type !== undefined, `Could not find property 'type' in the dataset of the parent of ${target}`);
         const dataPath = target.dataset["dataPath"];
         enforce(dataPath !== undefined, `Could not find property 'dataPath' in the dataset of ${target}`);
+        const dataPath2 = target.dataset["dataPath2"];
         const items = this.actor.items.filter((item) => item.type === type);
         items.sort((a, b) => a.data.sort - b.data.sort);
 
@@ -367,13 +375,20 @@ export class DS4ActorSheet extends ActorSheet<ActorSheet.Options, DS4ActorSheetD
             (a: DS4Item, b: DS4Item): number => {
                 const propertyA = getProperty(a.data, dataPath);
                 const propertyB = getProperty(b.data, dataPath);
-                if (typeof propertyA === "string" || typeof propertyB === "string") {
-                    return invert
-                        ? (propertyB ?? "").localeCompare(propertyA ?? "")
-                        : (propertyA ?? "").localeCompare(propertyB ?? "");
-                } else {
-                    return invert ? propertyB - propertyA : propertyA - propertyB;
+                const comparison =
+                    typeof propertyA === "string" || typeof propertyB === "string"
+                        ? compareAsStrings(propertyA, propertyB, invert)
+                        : compareAsNumbers(propertyA, propertyB, invert);
+
+                if (comparison === 0 && dataPath2 !== undefined) {
+                    const propertyA = getProperty(a.data, dataPath);
+                    const propertyB = getProperty(b.data, dataPath);
+                    return typeof propertyA === "string" || typeof propertyB === "string"
+                        ? compareAsStrings(propertyA, propertyB, invert)
+                        : compareAsNumbers(propertyA, propertyB, invert);
                 }
+
+                return comparison;
             };
 
         const sortedItems = [...items].sort(sortFunction(false));
@@ -434,3 +449,11 @@ const embeddedDocumentListEntryProperties = Object.freeze({
         idDataAttribute: "itemId",
     },
 });
+
+const compareAsStrings = (a: { toString(): string }, b: { toString(): string }, invert: boolean): number => {
+    return invert ? b.toString().localeCompare(a.toString()) : a.toString().localeCompare(b.toString());
+};
+
+const compareAsNumbers = (a: number, b: number, invert: boolean): number => {
+    return invert ? b - a : a - b;
+};

src/actor/actor.ts

@@ -79,12 +79,12 @@ export class DS4Actor extends Actor {
 
             if (condition !== undefined && condition !== "") {
                 try {
-                    const replacedCondition = Roll.replaceFormulaData(condition, {
+                    const replacedCondition = DS4Actor.replaceFormulaData(condition, {
                         item: item.data,
                         actor: this.data,
                         effect: effect.data,
                     });
-                    return Boolean(mathEvaluator.evaluate(replacedCondition));
+                    return replacedCondition !== undefined ? Boolean(mathEvaluator.evaluate(replacedCondition)) : false;
                 } catch (error) {
                     logger.warn(error);
                     return false;
@@ -95,6 +95,21 @@ export class DS4Actor extends Actor {
         });
     }
 
+    private static replaceFormulaData(formula: string, data: object): string | undefined {
+        const dataRgx = new RegExp(/@([a-z.0-9_\-]+)/gi);
+        try {
+            return formula.replace(dataRgx, (_, term) => {
+                const value = foundry.utils.getProperty(data, term);
+                if (value == null) {
+                    throw new Error();
+                }
+                return String(value).trim();
+            });
+        } catch {
+            return undefined;
+        }
+    }
+
     /**
      * We override this with an empty implementation because we have our own custom way of applying
      * {@link ActiveEffect}s and {@link Actor#prepareEmbeddedDocuments} calls this.

src/apps/sheet-helpers.ts

@@ -4,15 +4,17 @@
 
 import { getGame } from "../helpers";
 
-export function disableOverriddenFields<
-    Options extends FormApplicationOptions,
-    Data extends object,
-    ConcreteObject extends { overrides: Record<string, unknown> },
->(this: FormApplication<Options, Data, ConcreteObject>): void {
+export function disableOverriddenFields(
+    form: HTMLElement | null,
+    overrides: Record<string, unknown>,
+    selector: (key: string) => string,
+): void {
     const inputs = ["INPUT", "SELECT", "TEXTAREA", "BUTTON"];
-    const titleAddition = `(${getGame().i18n.localize("DS4.TooltipDisabledDueToEffects")})`;
-    for (const key of Object.keys(foundry.utils.flattenObject(this.object.overrides))) {
-        const elements = this.form?.querySelectorAll(`[name="${key}"]`);
+    const titleAddition = `(${getGame().i18n.localize("DS4.TooltipNotEditableDueToEffects")})`;
+
+    for (const key of Object.keys(foundry.utils.flattenObject(overrides))) {
+        const sel = selector(key);
+        const elements = form?.querySelectorAll(sel);
         elements?.forEach((element) => {
             if (inputs.includes(element.tagName)) {
                 element.setAttribute("disabled", "");

src/item/item-sheet.ts

@@ -72,7 +72,7 @@ export class DS4ItemSheet extends ItemSheet<ItemSheet.Options, DS4ItemSheetData>
 
         html.find(".control-effect").on("click", this.onControlEffect.bind(this));
 
-        disableOverriddenFields.call(this);
+        disableOverriddenFields(this.form, this.item.overrides, (key) => `[name="${key}"]`);
     }
 
     /**