Muttley / foundryvtt-ac2d20

Achtung! Cthulhu 2d20 System for Foundry VTT
MIT License
4 stars 3 forks source link

Hardcoded skills in autocalc MaxStress #66

Closed lozanoje closed 6 months ago

lozanoje commented 6 months ago

The function searchs for the skill Resilience:

_calculateMaxStress() { ... i => i.type === "skill" && i.name === "Resilience" );

I use custom skills in spanish, so Resilience is

"img": "systems/ac2d20/assets/skills.svg", "name": "Fortaleza", "type": "skill",

And produces this error:

2024-05-28 00_05_34-Foundry Virtual Tabletop - Personal_ Microsoft​ Edge

Muttley commented 6 months ago

You don't need to use custom skills for translation purposes any more as the default ones are automatically translated if necessary. Custom skills are now purely for if you want to add your own additional skills and if so you use a copy of the base skills to start with.

lozanoje commented 6 months ago

I dont like the idea of using system english skills for two reasons: +Skills compendium is not translated itself (appears in english, so for spanish user it keeps appearing Resilience instead of Fortaleza in the compendium listing, and if someone want to search compendia for Fortaleza it wont find anything) - i know this is a little minor detail +I lose the spanish description of each skill and focus in the skill item, there is no way of adding spanish description and case-use of each skill if I use the i18n

Muttley commented 6 months ago

I dont like the idea of using system english skills for two reasons:

Unfortunately we need some way of locating the correct skill, and we can't rely on the usual uuid of the document because by making a copy that will change, and I want to avoid flooding the system with too many configuration options that people need to set before they can even begin using the system.

+Skills compendium is not translated itself (appears in english, so for spanish user it keeps appearing Resilience instead of Fortaleza in the compendium listing, and if someone want to search compendia for Fortaleza it wont find anything) - i know this is a little minor detail

This is not really an issue as these items are automatically added to new characters, so you never actually need to look in the compendiums for them. On the actual character sheet they always appear translated.

Also, they can be edited directly on the sheet now without them even having to be opened (Ctrl+Click to toggle a focus, and amending the skill value)

+I lose the spanish description of each skill and focus in the skill item, there is no way of adding spanish description and case-use of each skill if I use the i18n

This will not be an issue shortly, as we will be able to include the skill/focus descriptions as part of the translation data, and so can provide fully translated tooltips for each skill/focus directly on the character sheet.

lozanoje commented 6 months ago

Ok, I understand it is not a good decision to fill the system with settings.

But currently you can choose the skills compendium in configuration settings that will be copied over all new characters, this setting will break the autocalc function if the Resilience skill is not in the set of skills chosen in the dropdown.

I'd definitively go for one direction or the other:

Or allowing custom skills: skill compendium to be selected and config the Resilience skill in settings. Or going definitively for system skills, removing the skill-compendium setting, you can always draganddrop custom skills whenever needed

Muttley commented 6 months ago

Or going definitively for system skills, removing the skill-compendium setting, you can always draganddrop custom skills whenever needed

This is probably the way forward now everything is translated properly.

Muttley commented 6 months ago

@lozanoje I'm probably going ahead with retiring the custom skills for the next release (which will probably only be a few days).

As there's no easy way for me to migrate character skills data, I'm thinking of the following migration steps to run automatically:

Does that sound OK to you? That should at least reduce the amount of work required by the GM after updating the system.

lozanoje commented 6 months ago

@lozanoje I'm probably going ahead with retiring the custom skills for the next release (which will probably only be a few days).

As there's no easy way for me to migrate character skills data, I'm thinking of the following migration steps to run automatically:

  • Go through each Character and NPC and find any non-standard skills (ie. names don't match exactly), and if found:

    • Capture the skill name, value and list of tagged Focuses for later
    • Delete the skill from the character
  • Add any "missing" skills from the core set
  • Output a message to chat with the character name and a listing of the skills that were removed by their original name, showing each one's value and a list of tagged focuses (one message per character).

Does that sound OK to you? That should at least reduce the amount of work required by the GM after updating the system.

I made it automatically outside Foundry, used foundry-cli to export all actors and then I programmed the replacement of each skill in spanish back to english in every file and then I packed it again.

Probably best option is: take skill names from world's language file (es.json), find the counterparts in english (en.json) and replace, then list the missing skills

I dont know how many users has their custom skills, probably only spanish users (due to the skill compendium i provided with my translation module)

Fully updated spanish file is at: https://github.com/lozanoje/fvtt-module-ac2d20-es/blob/main/lang/es.json including new tooltip feature, I will try to send you a PR with this updated file

Muttley commented 6 months ago

Resolved by the data migration in the next release