MetaMorphic-Digital / draw-steel

The fan-made Draw Steel implementation for FoundryVTT
Other
13 stars 4 forks source link

Add descriptions to CONFIG entries #11

Open JPMeehan opened 2 months ago

JPMeehan commented 2 months ago

The backer packet license allows the full text, so we can include things like the descriptions of keywords

Taroc0 commented 1 month ago

As my knowledge on how to build a game system in Foundry is very limited, I could work on this issue.

What keywords should be described?

I could start with conditions like dazed, frightend and so on. All skills could be next.

What would be the best way to do it?

Would it make sense to add the description to en.json first like this:

      "Conditions": {
        "Dazed": {
          "name": "Dazed",
          "description": "While you are dazed, you can do only one thing on your turn: use a maneuver, use an action, or take a move action. You also can’t use triggered actions, free triggered actions,or free maneuvers."
        },

And add it to config.mjs like this:

/**
 * @type {Record<string, {img: string, name: string}>}
 */
DRAW_STEEL.conditions = {
  // bleeding: {},
  dazed: {
    name: "DRAW_STEEL.Effect.Conditions.Dazed.name",
    img: "icons/svg/daze.svg",
    desc: "DRAW_STEEL.Effect.Conditions.Dazed.description"
  },
JPMeehan commented 1 month ago

Yes, although two adjustments

Also, can you switch var toRemove to const toRemove in draw-steel.mjs? I missed that in my earlier review

Taroc0 commented 1 month ago

Sure. That var instead of const is a stupid mistake of mine. I'll push with the next PR.

Does that separate localization pass through relate to Babele or is it something else? Is it ok to begin adding the description values now or is it better to wait and figure out that localization pass through first?

JPMeehan commented 1 month ago

You can start adding values. To briefly explain, there is a preLocalize helper that I copied from dnd5e that allows us to define this CONFIG in init and then make a pass updating the values in i18nInit. However, the status effects are an exception because we're adding them somewhere else in a way that just iterating over CONFIG.DRAW_STEEL won't help — need to loop through that CONFIG.statusEffects and localize the descriptions.

krbz999 commented 1 month ago

I think it would be better to include a journal in the system and have the status conditions point to those pages. Then we can use the @Embed enricher. This would mean the text is not copied for every effect but is drawn dynamically from a single journal.

We can override Actor#toggleStatusEffect (or some adjacent internal function) to set the description of the effect to be @Embed[${effect.rule}].

JPMeehan commented 1 month ago

So in this case rather than an i18n string we'd have the description in the status effects be set to the @Embed. I can get the journals set up in a PR.

Also, the spot to override would be ActiveEffect._fromStatusEffect which was added in v12.

krbz999 commented 1 month ago

So in this case rather than an i18n string we'd have the description in the status effects be set to the @Embed. I can get the journals set up in a PR.

Also, the spot to override would be ActiveEffect._fromStatusEffect which was added in v12.

While storing the journal entry page's uuid in a different property from description - then we can additionally combine the two if description exists.

JPMeehan commented 1 month ago

So, in ActiveEffect._fromStatusEffect we'll append to the description an @Embed[uuid]. The UUID is pulled from some CONFIG object, possibly a non-schema property in CONFIG.statusEffects?

krbz999 commented 1 month ago

Yes. I suggested .rule above.

Taroc0 commented 1 month ago

So, we add a journal entry to a compendium like this: image

And config.mjs would have a .rule property with that journal entries UUID?

/**
 * @type {Record<string, {img: string, name: string}>}
 */
DRAW_STEEL.conditions = {
  bleeding: {
    name: "DRAW_STEEL.Effect.Conditions.Bleeding.name",
    img: "icons/svg/blood.svg",
    rule: "Compendium.world.drawsteel-rules.JournalEntry.yKhPQSIaIEfNLJaA.JournalEntryPage.WbAnV19RjUwXEYx8"
  },
JPMeehan commented 1 month ago

It'll be like that, but the compendiums will need to be system compendiums rather than world compendiums. I'll set that up this weekend so we can turn the backer packet into a journal compendium

JPMeehan commented 1 month ago

@Taroc0 the pack/unpack stuff is now available, I've even started two journals you can start to fill in.

Taroc0 commented 1 month ago

Thanks for the update. I studied your code yesterday and added a test page already. What would be the best way to test my work before I send a PR? The system compediums aren't visible in Foundry yet (although they are visible in the dnd5e system), so I don't know if I did everything right. I assume it's the best way to write the pages in Foundry, export them and clean up the pages part of the resulting json before I add them to our journal json. This way the it's easy to style the text and have IDs generated by Foundry.

JPMeehan commented 1 month ago

You'd run npm run pullYMLtoLDB while foundry is shut down — that will build them from the json

Taroc0 commented 1 month ago

My fault. I did run npm run build:packs before and the .ldb had been created. I couldn't see the new system journals in Foundry because I had manually created a world compendium last week. After deleting the world compendium the system journal became visible. I'll try to add the status effects till friday.