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

Gun Pluralization Template Standardization #74537

Closed
wants to merge 8 commits into from

Conversation

sadenar
Copy link
Contributor

@sadenar sadenar commented Jun 14, 2024

Summary

Content "Standardizing most guns to follow the same pluralization template"

Purpose of change

Some previous incomplete passes among the guns within the project changed the way we pluralize multiple copies of a given gun from:

  • [gun name]s

To:

  • [gun name] + [kind of gun (pistol/rifle/shotgun...)]s

This PR just applies this template to every gun I could find that wasn't already pluralized like this within vanilla CDDA as well as Aftershock and Magiclysm.

Describe the solution

Pluralization template applied everywhere it made sense with what I thought was the closest and most fitting "type" of firearm to the given gun, although I'm by no means a gun person so perhaps @DoctorBoomstick may have comments on my choices.

Did not add obviously redundant descriptors behind abbreviations such as GL or HMG, this is subject to discussion, it just didn't feel necessary.

Removed almost every str_sp gun name string as this pluralization template makes them obsolete.

May impact or conflict with #74530 ? @anothersimulacrum

Describe alternatives you've considered

  • Add gun types behind obvious abbreviations
  • Keep this merged with a misc string fixes PR I was working on, decided it was too big to keep it together
  • Get a bit more specific on some weapons and their types and less on others

Testing

Simple string changes.

Additional context

@github-actions github-actions bot added [JSON] Changes (can be) made in JSON Mods Issues related to mods or modding Items: Ammo / Guns Ammunition for all kinds of weapons and these weapons themselves Mods: Magiclysm Anything to do with the Magiclysm mod Mods: Aftershock Anything to do with the Aftershock mod Mods: Xedra Evolved Anything to do with Xedra Evolved labels Jun 14, 2024
Copy link
Contributor

Spell checker encountered unrecognized words in the in-game text added in this pull request. See below for details.

Click to expand
  • FN Five-seveN suppressor pistol
  • FN Five-seveN suppressor pistols
  • GSG Firefly pistol
  • GSG Firefly pistols
  • Milkor MGLs
  • Pistolet modèle Gardes de la Marine flintlock pistols
  • Pistolet modèle Gardes de la Marine flintlock pistolss
  • SIG SG 556 rifle
  • SIG SG 556 rifles
  • The GSG Firefly is a .22 semi-automatic handgun. While the GSG Firefly started off as the SIG Mosquito, after the Mosquito ceased production, German Sport Guns picked it up and renamed it to the Firefly.
  • av-22 laser pistol
  • av-22 laser pistols

This alert is automatically generated. You can simply disregard if this is inaccurate, or (optionally) you can also add the new words to tools/spell_checker/dictionary.txt so they will not trigger an alert next time.

Hints for adding a new word to the dictionary
  • If the word is normally in all lowercase, such as the noun word or the verb does, add it in its lower-case form; if the word is a proper noun, such as the surname George, add it in its initial-caps form; if the word is an acronym or has special letter case, such as the acronym CDDA or the unit mW, add it by preserving the case of all the letters. A word in the dictionary will also match its initial-caps form (if the word is in all lowercase) and all-uppercase form, so a word should be added to the dictionary in its normal letter case even if used in a different letter case in a sentence.
  • For a word to be added to the dictionary, it should either be a real, properly-spelled modern American English word, a foreign loan word (including romanized foreign names), or a foreign or made-up word that is used consistently and commonly enough in the game. Intentional misspelling (including eye dialect) of a word should not be added unless it has become a common terminology in the game, because while someone may have a legitimate use for it, another person may spell it that way accidentally.

@github-actions github-actions bot added json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Jun 14, 2024
@John-Candlebury
Copy link
Member

I rather dont like this for aftershock. Most the guns are non standard enough that the clarifier is actually a source of confusion.

Example: The Gibson S86. is either an auto shotgun, a grenade launcher or an M2 HMG equivalent depending on the ammo you load.

I think it would be best to exclude the mod from this.

@anothersimulacrum anothersimulacrum self-requested a review June 15, 2024 06:25
Copy link
Contributor

@DoctorBoomstick DoctorBoomstick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the whole, vanilla looks great. Most of the comments I left are with regards to elements of the names present before your changes, but that might be wise to address while you are fiddling with names and adding extra words.

data/json/items/gun/22.json Show resolved Hide resolved
data/json/items/gun/300BLK.json Outdated Show resolved Hide resolved
data/json/items/gun/300BLK.json Outdated Show resolved Hide resolved
data/json/items/gun/38super.json Outdated Show resolved Hide resolved
data/json/items/gun/40.json Outdated Show resolved Hide resolved
data/json/items/gun/50.json Outdated Show resolved Hide resolved
data/json/items/gun/50.json Outdated Show resolved Hide resolved
data/json/items/gun/50.json Outdated Show resolved Hide resolved
data/json/items/gun/50.json Outdated Show resolved Hide resolved
data/json/items/gun/shot.json Outdated Show resolved Hide resolved
@sadenar
Copy link
Contributor Author

sadenar commented Jun 15, 2024

Applied all changes suggested by Boomstick and should have reverted all the fiddling I did with Aftershock as per Candlebury's request.

@anothersimulacrum
Copy link
Member

Can you remove the ids of all the guns you've touched in this PR from this list?

@github-actions github-actions bot added [Python] Code made in Python Code: Tooling Tooling that is not part of the main game but is part of the repo. labels Jun 15, 2024
@anothersimulacrum
Copy link
Member

You have merge conflicts that need to be resolved.

@@ -439,7 +439,7 @@
"variants": [
{
"id": "ruger_charger",
"name": { "str": "Ruger 22 Charger" },
"name": { "str": "Ruger 22 machine pistol" },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Machine pistol is a specific term for automatic pistols. The Charger is semi-automatic

@Holli-Git
Copy link
Contributor

I honestly do not like this, as I think the name of a gun should be the name of the gun and nothing more. Adding the extra "rifle", "pistol", etc feels weird for brand name guns, as people who already know about the firearm, either from previous knowledge or just remembering know it as "Oh, AK? That's a rifle" or "P226? That's a pistol", and this new system makes it redundant. For people who don't know, but don't mind the bit of learning can inspect the gun either from range or from close. The generic named guns should be the naming system to cover for people who aren't familiar with guns or want on-sight recognition, or both.

These two naming systems should remain more separate, and it can also bring up some issues of naming. "Should I call this a revolver, a handgun, a machine pistol?" "Should this be a carbine, or a rifle?", "Is this a battle rifle or a hunting rifle?", "Is this a PDW, an SMG?". I think the best way to do this would be to have it based off of the skill of the gun, and nothing else. So just "handgun", "shotgun", etc, but that also has issues for different platforms in different calibers.

I understand where this is coming from, but IMO I think it should be worked the opposite way instead. Though, this is just my own personal opinion, I may be in the minority here.

Copy link
Contributor

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 github-actions bot added the stale Closed for lack of activity, but still valid. label Jul 23, 2024
@Night-Pryanik
Copy link
Contributor

Closing as stale. If you wish to continue working on this, ping me to reopen.

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 Code: Tooling Tooling that is not part of the main game but is part of the repo. Items: Ammo / Guns Ammunition for all kinds of weapons and these weapons themselves [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions Mods: Aftershock Anything to do with the Aftershock mod Mods: Magiclysm Anything to do with the Magiclysm mod Mods: Xedra Evolved Anything to do with Xedra Evolved Mods Issues related to mods or modding [Python] Code made in Python stale Closed for lack of activity, but still valid.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants