DFreds / dfreds-convenient-effects

A FoundryVTT module that adds easy to use toggleable active effects for any system.
MIT License
45 stars 39 forks source link

Add inaudible effect to DFreds #253

Closed Dyrnwyrm closed 1 year ago

Dyrnwyrm commented 1 year ago

Is your feature request related to a problem? Please describe. On v11 if you install the Module Vision/Detection Modes for 5e you cant see the new "condition" inaudible if you use the "replace" option of the module, if only visible if you change the option to "add" wich makes a huge mess in the conditions UI (non dfreds and dfreds conditions sharing the list)

Describe the solution you'd like Have dfreds add the condition so the list of conditions is not a huge mess

Additional context Link the the module: https://github.com/dev7355608/vision-5e

Example of the new condition:

imagen_2023-06-10_155549473

txm3278 commented 1 year ago

Vision/Detection Modes also uses the flying condition which this removes

dev7355608 commented 1 year ago

Vision 5e defines the following special status effects (https://github.com/dev7355608/vision-5e/blob/ff9b3ea3d48af1396ad4fdf1194f846cbd78dbd0/scripts/config.mjs#L94-L107):

Convenient Effects removing them from the HUD is only one part of the problem. The additionally defined special status effects are also erased from CONFIG.specialStatusEffects. In general special status effects should not be erased, because this would likely break the module that has added them. So if a module defines CONFIG.specialStatusEffects.DEAF = "deaf", then Convenient Effects shouldn't erase it from CONFIG.specialStatusEffects and instead remap CONFIG.specialStatusEffects.DEAF to Convenient Effect: Deafened. Special status effect that aren't on the list of 5e conditions shouldn't be erased from CONFIG.specialStatusEffects but may be removed from CONFIG.statusEffects if desired.

DFreds commented 1 year ago

Vision 5e defines the following special status effects (https://github.com/dev7355608/vision-5e/blob/ff9b3ea3d48af1396ad4fdf1194f846cbd78dbd0/scripts/config.mjs#L94-L107):

* deaf

* disease

* fly

* inaudible

* poison

Convenient Effects removing them from the HUD is only one part of the problem. The additionally defined special status effects are also erased from CONFIG.specialStatusEffects. In general special status effects should not be erased, because this would likely break the module that has added them. So if a module defines CONFIG.specialStatusEffects.DEAF = "deaf", then Convenient Effects shouldn't erase it from CONFIG.specialStatusEffects and instead remap CONFIG.specialStatusEffects.DEAF to Convenient Effect: Deafened. Special status effect that aren't on the list of 5e conditions shouldn't be erased from CONFIG.specialStatusEffects but may be removed from CONFIG.statusEffects if desired.

Appreciate your input.

By default, Foundry sets these

{DEFEATED: 'dead', INVISIBLE: 'invisible', BLIND: 'blind'}

If you opt to add or replace effects, CE sets them to this: {DEFEATED: 'Convenient Effect: Dead', INVISIBLE: 'Convenient Effect: Invisible', BLIND: 'Convenient Effect: Blinded'}

I changed the code so I'm no longer removing any other special status effects now. That was bad practice. With that change, your five special status effects should now remain. The change is here. It's not released yet, but will probably be a quick hotfix (v5.0.2).

I do need to remove and replace all the regular statusEffects if the user opts to replace them in the settings.

However, I think there is a work around to add yours back in at the right time. I have a hook called dfreds-convenient-effects.ready that you could listen to and use to determine if you need to add INAUDIBLE back into the status effects. The hook only runs once my status effects have been initialized, and occurs after Foundry's ready hook is called. Code is here. I think you could do something like this:

Hooks.once('dfreds-convenient-effects.ready', () => {
    // TODO maybe some check to see if it's already there... not sure
    registerStatusEffect(
        {
            id: CONFIG.specialStatusEffects.INAUDIBLE,
            label: "VISION5E.Inaudible",
            icon: "icons/svg/sound-off.svg"
        },
        CONFIG.statusEffects.findIndex(s => s.id === CONFIG.specialStatusEffects.INVISIBLE) + 1
    );
}

Otherwise, I'm happy to work with you @dev7355608 if you have another suggestion.

DFreds commented 1 year ago

v5.0.2 fixes me overwriting the special status effects. I'll keep this open for a bit and close if no further feedback is provided

dev7355608 commented 1 year ago

Vision 5e 1.3.0 is out. The status effects are reregistered and remap as you suggested: https://github.com/dev7355608/vision-5e/blob/23e0517632dbbc29ff9b520488f0ca18b7243465/scripts/compatibility.mjs#L67-L124. Should you ever add the Inaudible, Flying, or Diseased conditions to Convenient Effects, it shouldn't break the compatibility.

It doesn't feel great that the status IDs of Convenient Effects are different from core's. If Convenient Effects is disabled, the Blinded effect doesn't blind the token anymore. That's not great. Ideally both core's Blinded status and Convenient Effects' Blinded status apply the the same blind status. But I think it might need some core changes to make this possible.

lacktheknack commented 1 year ago

Unfortunately, with the newest of each module, now neither effect works with Vision 5e.

DFreds commented 1 year ago

@dev7355608 So I'm actually updating this so instead of overwriting the CONFIG.specialStatusEffects, I'm instead adding the blind status to the Blinded condition. So CONFIG.specialStatusEffects.BLIND = 'blind', but Convenient Effect: Blinded will have two status IDs: ['Convenient Effect: Blinded', 'blind']. This should work in all cases, and also be a lot more flexible with integrating with other default status IDs (like flying). See #261