Cataclysm-DDA icon indicating copy to clipboard operation
Cataclysm-DDA copied to clipboard

Allow appending and prepending copy-from descriptions

Open Procyonae opened this issue 1 year ago • 12 comments

Summary

None

Purpose of change

Aid for #71112 and #70352 Less repetition -> less translation needed + less typo opportunities + less bloat -> yay

Describe the solution

Adds optional "description_prepend" and "description_append" to items and item variants that add text to the start or end of the description respectively. Combined with copy-from this allows adding to descriptions without repetition. Adds a disgustingly named (please suggest alternatives) "inherit_extended_description" bool that defaults to true but can be specified to false in which case items that inherit from that item will only use the "description" not the extended description after prepending/appending/both, useful for stuff like steel grades where the base item can append its grade without it being inherited to save adding abstract entries for all steel graded stuff. The old "append" behaviour for item variants is still supported to avoid making this PR harder to review, will remove in a follow-up PR.

Describe alternatives you've considered

Adding better context to translator comments but that would require searching through the json to find information about copy-from members which is probably expensive even without doing it recursively and only checking the same file I've no idea if extended_description() is defined in a sensible place nor whether everything should be public Idk if there's much performance benefit if any to having the extend_description bool

Testing

Tested with custom test stuff like below and the changes in the PR

  {
    "id": "magic_1_ball",
    "type": "GENERIC",
    "category": "other",
    "name": { "str": "Magic 1-Ball" },
    "description": "Once I were a 1-Ball.",
    ...
  },
  {
    "id": "magic_2_ball",
    "type": "GENERIC",
    "copy-from": "magic_1_ball",
    "name": { "str": "Magic 2-Ball" },
    "description_prepend": "Twice a 2-Ball.",
    "description_append": "Twice a 2-Ball."
  },
  {
    "id": "magic_3_ball",
    "type": "GENERIC",
    "copy-from": "magic_2_ball",
    "name": { "str": "Magic 3-Ball" },
    "description_append": "I even became a 3-Ball."
  },
  {
    "id": "magic_4_ball",
    "type": "GENERIC",
    "copy-from": "magic_3_ball",
    "name": { "str": "Magic 4-Ball" },
    "description_prepend": "I came to be a 4-Ball.",
    "description_append": "But decided a 4-Ball was my true form."
  },

image

Additional context

Procyonae avatar Jan 20 '24 19:01 Procyonae

btw, name and description is still mandatory for items, and not carried by inheritance

GuardianDll avatar Jan 20 '24 23:01 GuardianDll

btw, name and description is still mandatory for items, and not carried by inheritance

Description isn't mandatory, the test example above shows it isn't mandatory and is inherited. (I think there's a pre-existing check to make sure the description isn't empty somewhere though)

Procyonae avatar Jan 20 '24 23:01 Procyonae

Huh, i was sure description is also mandatory

GuardianDll avatar Jan 21 '24 00:01 GuardianDll

@BrettDong I could use a review from someone who understands how the translation stuff works as to whether I'm creating unnecessary translation strings or missing necessary ones anywhere.

Procyonae avatar Jan 26 '24 22:01 Procyonae

Kevin is debating whether this should go through or whether the existing append feature from #56302 should be scrapped so this'll be drafted until they make a decision, see this message chain on the devcord

Procyonae avatar Jan 27 '24 00:01 Procyonae

whether I'm creating unnecessary translation strings or missing necessary ones anywhere.

You can check out the output in "List text changes" step in "Text Changes Analyser" CI job to see the impact on translatable strings from the pull request:

https://github.com/CleverRaven/Cataclysm-DDA/actions/runs/7678085991/job/20927556843?pr=71121#step:9:13

It lists 1556 strings deleted and only 7 added, so looks like a lot of stuff is missing.

BrettDong avatar Jan 27 '24 14:01 BrettDong

If you need help on Python code formatting: I use autopep8 to format Python code:

cd ./lang/string_extractor
autopep8

BrettDong avatar Jan 27 '24 14:01 BrettDong

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Please do not bump or comment on this issue unless you are actively working on it. Stale issues, and stale issues that are closed are still considered.

github-actions[bot] avatar Feb 27 '24 02:02 github-actions[bot]

Reopened on request of the author.

RenechCDDA avatar Apr 19 '24 13:04 RenechCDDA

Having a bit of a mare with this one. After further discussion with kevin this is fine to add as long as the full extended string is exposed to translation as opposed to the individual sentences which felt like it should be easy enough but bc of the base description potentially being copy-frommed multiple times the python code we use to add comments to it currently has no simple way to piece it together and I have no idea how to do dynamic C++ translation comments if we even can.

Procyonae avatar Apr 23 '24 12:04 Procyonae

From my experience, I think it would be enough to write in the comment that this line is added to the description of the specified thing. If anyone is unsure of the context, they can look it up.

Yes, it will not be easy to write down in the comments the entire description of what it will be like ingame. An item can be copied from an item in another file.

We can try, as far as I've seen, it's rare that a definition is copied more than one level of inheritance. Perhaps it is possible in most cases to write down a complete description, otherwise fallback to specifying the copied entity.

Uwuewsky avatar Apr 24 '24 04:04 Uwuewsky

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Please do not bump or comment on this issue unless you are actively working on it. Stale issues, and stale issues that are closed are still considered.

github-actions[bot] avatar May 24 '24 05:05 github-actions[bot]

should this be reopen, or put into "abandoned but desired PRs" list, or else? @Procyonae

GuardianDll avatar Jun 23 '24 10:06 GuardianDll

I'm not working on it rn (and I can reopen stuff myself now) but it's a bit of a mess so I wouldn't bother adding it to "abandoned but desired PRs", idm if it is tho

Procyonae avatar Jun 27 '24 11:06 Procyonae