Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed descriptions for reloaded and black powder rounds #75874

Merged
merged 3 commits into from
Sep 24, 2024

Conversation

Uwuewsky
Copy link
Contributor

Summary

Interface "Removed descriptions for reloaded and black powder rounds"

Purpose of change

Initially, I moved the repeating text into snippets, because 99.99% of such descriptions are simply a description of the factory round + a small explanation. It was all good, but not being able to add some text to the parent item description without copying it makes it less useful, and the text becomes more cluttered. And it's a bit hard to maintain.

Describe the solution

So I removed those descriptions in favor of effect (RECYCLED, BLACKPOWDER, MATCHHEAD) descriptions. There are about 5 unchanged exceptions where this description differed from a simple concatenation with a "snippet".

Also, I changed the description of the effect in item.cpp similar to what was in the descriptions of the items (see third commit).

The second commit contains unrelated minor fixes that I noticed. I didn't put them in a separate PR because of possible merge conflicts.

Describe alternatives you've considered

Testing

Almost every ammo has a description of the parent.
ammo_new1

Additional context

@github-actions github-actions bot added [JSON] Changes (can be) made in JSON [C++] Changes (can be) made in C++. Previously named `Code` Info / User Interface Game - player communication, menus, etc. astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Aug 22, 2024
@esotericist
Copy link
Contributor

you seem to have put the answer to 'alternatives you considered' into the 'purpose of change' section, and omitted the rationale for this entirely. what is the objective? what makes that objective necessary?

@Uwuewsky
Copy link
Contributor Author

in general, the rationale here is the same as in #71138

@kevingranade
Copy link
Member

In short, your goal is reducing redundancy in descriptive strings in order to reduce the number of distinct strings for translation and translator workload.
In the future please just copy something like the above into related prs to give people context.

@kevingranade kevingranade merged commit b3a2fbd into CleverRaven:master Sep 24, 2024
27 checks passed
@Uwuewsky Uwuewsky deleted the ammo-desc branch September 25, 2024 01:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` Info / User Interface Game - player communication, menus, etc. [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants