DFreds / dfreds-convenient-effects

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

Underwater Combat effect definition #284

Closed thatlonelybugbear closed 3 months ago

thatlonelybugbear commented 1 year ago

Needs MidiQOL 11.0.11 or higher and a bit more testing.

thatlonelybugbear commented 1 year ago

It seems that this needs to be available only if MidiQOL is active 🤔

thatlonelybugbear commented 1 year ago

Will reopen when this works... This will error out and I am on the way now 🤦

thatlonelybugbear commented 1 year ago

This should do it 🤔 🤞

thatlonelybugbear commented 1 year ago

Added a new foundry-helper to check for a module being active and its version against provided one

/**
   * Checks if the module provided is active and if yes and a version is provided, compares against its currect version
   *
   * @param {string} key - the module id
   * @param {string} version - the module version
   * @returns {Boolean} true if module.active+version > version OR true if module.active+!version
   */
 moduleActiveVersionCheck(key, version) {
    const module = game.modules.get(key);
    if (module && !version) return module.active;
    else if (module?.active && version)
      return foundry.utils.isNewerVersion(module.version, version);
    else return false;    
  }

I hope this makes sense.

thatlonelybugbear commented 1 year ago

Maybe that would need to offer a bit more granularity to the MidiQOL specific effects it adds depending on the version of MidiQOL the user currently is. So something like (pseudo)

if (midi.active) {/*initial check to add the MidiQOL folder*/}
if (midi.version >= target.version) {/*add a specific list of MidiQOL specific CEs*/}
if (midi.version >= target.version.2) {/*add another specific list of MidiQOL specific CEs etc*/}
if (!midiFolder.length) {/*do not display folder*/}

Might be better for the future, but only relevant if more and more such effects keep popping up. So for now it should be fine 🤔

DFreds commented 11 months ago

I'm a bit confused. What happens when you don't do all these midi checks and midi is inactive? Does it actually error or just doesn't work as intended? Because if it just doesn't work, that seems reasonable enough. Maybe a new icon could and boolean flag could be added for midi specific rather than doing all this extra checking stuff.

thatlonelybugbear commented 11 months ago

I thought of just adding the MidiQOL ones (ones not having also a WIRE counterpart). only when the module is actually active, so as not to bloat the in game CE list.

If MidiQOL is inactive and someone adds the "available" Underwater Combat CE, it will just do nothing, but won't error AFAIK.

However, the use of evaluation conditions in AEs values for Midi flags is somewhat new, so checking MidiQOL version for compatibility reasons might be needed. If MidiQOL is active and underwater combat is available no matter what Midi version one is currently on, the results can be unpredictable, from erroring out to actually always evaluating to true, which in not ideal.