foundryvtt / foundryvtt

Public issue tracking and documentation for Foundry Virtual Tabletop - software connecting RPG gamers in a shared multiplayer environment with an intuitive interface and powerful API.
https://foundryvtt.com/
198 stars 10 forks source link

Active Effects With Changes And Conditions do not properly apply conditions #10864

Open JaysonMendoza opened 3 weeks ago

JaysonMendoza commented 3 weeks ago

What happened?

When an ActiveEffect that applies conditions and statuses is triggered, it will apply both but the statuses application is incomplete and doesn't trigger the unique statuses active effect.

My research shows the root cause of this problem is twofold.

  1. The Active Effect Creation Workflow only concerns itself with the active effect and not subordinate effects.
  2. Standardized subordinate effects may have multiple sources keeping them active and the system has no way to coordinate them.

Active Effect Workflow & Statuses

Part of the problem here is that the way statuses are handled. In the current design, the active effect has a list of status keys in the statuses area for the ActiveEffect. However, the design also has a unique active effect for each status defined by the system. Since these statuses are shared by many sources they would also need a different way to be handled since the status effect to active effect relationship is many-to-many.

Thus the expected behavior is that when an Active Effect triggers a status effect, it should add or activate it on the target. It should also remove it from the target when it becomes inactive only if no other sources of the status condition are active unless the GM removes it.

Proposed Solution

I would suggest the behaviour for automatic deactivation of AE based on potentially having multiple sources become a standard behaviour between status effects and normal Active Effects. My reasoning is that systems and modules should be able to coordinate multiple sources for the same effect and use the basic behaviour for all of them. To do this we need two things.

  1. A key that uniquely identifies an effect based on some outside real-world information defined by the system or a module.
  2. The ability to track multiple origins

To facilitate this I would suggest the following.

  1. Add a key called effectGroup which is a string or array of strings. This would allow us to identify if an effect already exists in the application or activation workflow and allow us to reuse that effect instead of creating a new one. This key should be nullable and if absent it will not use that behavior.
  2. Change the duration key to an array of duration documents that now have new keys including origin with a document UUID and a isCumalitive boolean key. This would allow an effectGroup like a status effect to add itself by adding a duration that includes a reference to the origin of an existing effect that is part of the same effect group.
  3. Toggling off an ActiveEffect by an origin would then need to check for a duration belonging to that origin and remove it. Then reevaluate the duration to determine if its activity status has changed.
  4. Duration would need to change so that it decreases all durations as normal unless they are cumulative. If a duration is cumulative then it will only decrement the timer if it is the first cumulative timer found and no non-cumulative timers were found (Single iteration through array but save ref to first non-reference duration and use it only if the array is traversed without finding any non-cumulative duration). If a timer is complete, it should be removed from the array of durations. It should then do the normal check to see if it's active and deactivate it if there are no more valid timers.
  5. If the GM manually deactivates an effect then it should delete all durations. Perhaps a pop-up message warning about it that lists the current origins of the effect and its effectGroup.

Compatibility Considerations to avoid breaking changes

The above change for duration would be a breaking change since systems and modules expecting a single object will get an array if they access that field directly. A way to potentially handle this might be to implement the new behaviour for duration under a new key durations and convert the old duration to be deprecated. Then have the system warn when the key is accessed and potentially grab the top duration from the durations array and return it. Since the change to the Durations data model only adds keys, legacy code would not be affected.

What ways of accessing Foundry can you encounter this issue in?

Reproduction Steps

1) Get an Actor with no active effect or condition.

image

2) Apply an effect that has both a statusCondition and changes then apply it to the actor

ActiveEffect.create({
    "_id": "8rP3gwmXVTgZqYZE",
    "flags": {
        "drazevs-regional-effects.isModuleSourced": true,
        "drazevs-regional-effects.ModuleMajorVersion": "1",
    },
    "changes": [
        {
            "key": "system.bonuses.abilities.save",
            "mode": 2,
            "value": "1d4",
            "priority": null
        },
        {
            "key": "system.bonuses.mwak.attack",
            "mode": 2,
            "value": "1d4",
            "priority": null
        },
        {
            "key": "system.bonuses.msak.attack",
            "mode": 2,
            "value": "1d4",
            "priority": null
        },
        {
            "key": "system.bonuses.rsak.attack",
            "mode": 2,
            "value": "1d4",
            "priority": null
        },
        {
            "key": "system.bonuses.rwak.attack",
            "mode": 2,
            "value": "1d4",
            "priority": null
        }
    ],
    "disabled": false,
    "duration": {
        "startTime": null,
        "seconds": 60,
        "rounds": 10,
        "combat": null,
        "turns": null,
        "startRound": null,
        "startTurn": null
    },
    "origin": "Item.kZZAZ6kp9YzgPQEe",
    "tint": "#ff0000",
    "transfer": false,
    "name": "Test Bless",
    "description": "This is a test of bless from a template for DRE",
    "statuses": [
        "flying"
    ],
    "_stats": {
        "compendiumSource": null,
        "duplicateSource": null,
        "coreVersion": "12.321",
        "systemId": "dnd5e",
        "systemVersion": "3.1.2",
        "createdTime": null,
        "modifiedTime": null,
        "lastModifiedBy": null
    },
    "img": "icons/magic/control/buff-flight-wings-blue.webp",
    "type": "base",
    "system": {}
}, {
    parent: fromUuidSync('Actor.Vcld2aYB5E9uPSdi')
  });

image

3) Inspect object and notice that statuses has the effect but there is no related active effect on the actor or UI indicators.

{
    "name": "Ocdnst Noteworthy",
    "type": "character",
    "sort": 500000,
    "img": "PlayerImages/Punch.jpg",
    "_id": "Vcld2aYB5E9uPSdi",
    "effects": [
        {
            "name": "Burning Acid",
            "img": "icons/svg/acid.svg",
            "_id": "97XtCfDOoq0pBBcq",
            "type": "base",
            "system": {},
            "changes": [
                {
                    "key": "system.attributes.movement.walk",
                    "mode": 5,
                    "value": "10",
                    "priority": null
                }
            ],
            "disabled": true,
            "duration": {
                "startTime": 79861255047,
                "seconds": 30,
                "combat": null,
                "rounds": null,
                "turns": null,
                "startRound": null,
                "startTurn": null
            },
            "description": "<p>test</p>",
            "origin": null,
            "tint": "#ffffff",
            "transfer": false,
            "statuses": [
                "flying",
                "ethereal"
            ],
            "flags": {},
            "_stats": {
                "compendiumSource": null,
                "duplicateSource": null,
                "coreVersion": "12.321",
                "systemId": "dnd5e",
                "systemVersion": "3.1.2",
                "createdTime": 1715105570659,
                "modifiedTime": 1715132989784,
                "lastModifiedBy": "RCjTTRIcOUdAPQyv"
            }
        },
        {
            "_id": "M7w6uXs0rEb3kvZx",
            "flags": {
                "drazevs-regional-effects": {
                    "isModuleSourced": true,
                    "ModuleMajorVersion": "1"
                }
            },
            "changes": [
                {
                    "key": "system.bonuses.abilities.save",
                    "mode": 2,
                    "value": "1d4",
                    "priority": null
                },
                {
                    "key": "system.bonuses.mwak.attack",
                    "mode": 2,
                    "value": "1d4",
                    "priority": null
                },
                {
                    "key": "system.bonuses.msak.attack",
                    "mode": 2,
                    "value": "1d4",
                    "priority": null
                },
                {
                    "key": "system.bonuses.rsak.attack",
                    "mode": 2,
                    "value": "1d4",
                    "priority": null
                },
                {
                    "key": "system.bonuses.rwak.attack",
                    "mode": 2,
                    "value": "1d4",
                    "priority": null
                }
            ],
            "disabled": false,
            "duration": {
                "startTime": 79861255047,
                "seconds": 60,
                "rounds": 10,
                "combat": null,
                "turns": null,
                "startRound": null,
                "startTurn": null
            },
            "origin": "Item.kZZAZ6kp9YzgPQEe",
            "tint": "#ff0000",
            "transfer": false,
            "name": "Test Bless",
            "description": "This is a test of bless from a template for DRE",
            "statuses": [
                "flying"
            ],
            "_stats": {
                "compendiumSource": null,
                "duplicateSource": null,
                "coreVersion": "12.321",
                "systemId": "dnd5e",
                "systemVersion": "3.1.2",
                "createdTime": 1715186162726,
                "modifiedTime": 1715186162726,
                "lastModifiedBy": "RCjTTRIcOUdAPQyv"
            },
            "img": "icons/magic/control/buff-flight-wings-blue.webp",
            "type": "base",
            "system": {}
        }
    ],
    **"statuses": Set([
        "flying"
    ]),**
    "_stats": {
        "systemId": "dnd5e",
        "systemVersion": "3.1.2",
        "coreVersion": "12.321",
        "createdTime": 1700097680288,
        "modifiedTime": 1714614000173,
        "lastModifiedBy": "RCjTTRIcOUdAPQyv",
        "compendiumSource": null,
        "duplicateSource": null
    }
}

image

This is what I would have expected... image

What core version are you reporting this for?

Version 12 Testing 2 Build 321

Relevant log output

No log output is necessary since no errors are thrown

Bug Checklist

JaysonMendoza commented 3 weeks ago

Note, This will occur v11 as well since the code for these related areas shares the same problem.