moo-man / WFRP4e-FoundryVTT

The premiere system for running grim and perilous games of Warhammer Fantasy Role-play in the Foundry VTT environment.
Apache License 2.0
92 stars 53 forks source link

Add Effect Script helper functions #2000

Open StevenEvans1966 opened 6 months ago

StevenEvans1966 commented 6 months ago

In the Effect scrips I see this a lot, some magic GUID that gets found converted to an object set a value on it then add it to an actor

`let ward = await fromUuid("Compendium.wfrp4e-core.items.Bvd2aZ0gQUXHfCTh") wardData = ward.toObject() wardData.system.specification.value = "8"

this.actor.createEmbeddedDocuments("Item", [wardData], {fromEffect : this.effect.id}) ` Could we have a helper function like;

let addedWard = game.whfr4e.AddToActor(this.actor, "Trait", "Ward") addedWard .system.specification.value = "8" //would be better as a option input in the function

Forien commented 6 months ago
  1. This is unsafe as you have no control over what specific item you want effect to add. What if there are 2 compendiums with that Trait? What if there is trait of the same name in the world? And they are all different? It's bad to assume it will just work.
  2. moving function to utility and not passing the effect... how do you want to do the fromEffect part? You would need additional parameter
  3. Your code would not update the Trait, so it would never set value of 8 to its specification. You would need to call update() on it

at that point there would be no much difference between these codes, except the first one makes less updates to the database:

//what you can do now, typical way (single database update)
const wardData = (await fromUuid("Compendium.wfrp4e-core.items.Bvd2aZ0gQUXHfCTh")).toObject();
wardData.system.specification.value = 8;
await this.actor.createEmbeddedDocuments("Item", [wardData], {fromEffect: this.effect.id});
// what you can do now differently (two database updates)
const wardData = (await fromUuid("Compendium.wfrp4e-core.items.Bvd2aZ0gQUXHfCTh")).toObject();
// this is trait from Core compendium and will always be the same item, so we know what it is
const ward = await this.actor.createEmbeddedDocuments("Item", [wardData], {fromEffect: this.effect.id});
ward?.update({"system.specification.value": 8});
// your idea (two database updates)
const ward = await game.wfrp4e.utility.addItemToActor(this.actor, "trait", "Ward", {fromEffect: this.effect.id});
// this is unknown item called "ward" and we don't know where it came from. 
// Hell, could be undefined if function above didn't find it
ward?.update({"system.specification.value": 8}); 

So a helper function that doesn't really help, results in almost the same amount of code and that it's not 100% identical to the original code (because it needs to find by name, now UUID), and can result in unpredictable outcome, is not really a helper.


Alternative

That said, there could be different helper, this time on the EffectWfrp4e class.

// this is code in effect's script
this.addItem("Compendium.wfrp4e-core.items.Bvd2aZ0gQUXHfCTh", {"system.specification.value": 8});

and method on EffectWfrp4e class would look like this:


async addItem(uuid, updates = {}, options = {}) {
  const compendiumItem = await fromUuid("Compendium.wfrp4e-core.items.Bvd2aZ0gQUXHfCTh");
  try {
    let itemData = compendiumItem.toObject();
    itemData = foundry.utils.mergeObject(itemData, updates);
    await this.actor.createEmbeddedDocuments("Item", [itemData ], {fromEffect: this.effect.id});
  } catch (e) {
  }
}
StevenEvans1966 commented 6 months ago

Apologies as a professional developer I tend to think in code, unfortunately my knowledge of the wfrp4e code base and javascript is at the "enough to be dangerous" level. My post was to improve the transparency of the code and create a single point (function) of maintenance.

this is the main problem: let item= await fromUuid("Compendium.wfrp4e-core.items.Bvd2aZ0gQUXHfCTh")

all I know when reading the script is that I is getting an Object from the core module. Compare with this

let item = game.wfrp4e.FindItem("Core", "Trait", "Ward")

I can now see I am looking in the Core module for the Trait Ward. All we have to create a lookup to the UUid from the three keys. I am assuming that items (Traits, spell etc.) have unique Names within a module.

Forien commented 6 months ago

Right, from the top of my head I know of a one instance where multiple items of the same type share a name image

It probably is not the case for most of other things. I know some traits and talents share a name, but they are different type, so it wouldn't matter.