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

Remove STI 2011, P-3AT, PF-9, M714, Taurus Spectrum, ZPAP 85, .40 PPQ, .40 90-two, P230, ARX-160, CX4, Saiga 410 #77206

Merged
merged 4 commits into from
Oct 21, 2024

Conversation

Holli-Git
Copy link
Contributor

@Holli-Git Holli-Git commented Oct 20, 2024

Summary

Bugfixes "Removes a few handguns, rifles, and a shotgun that didn't meet inclusion criteria"

Purpose of change

After doing #76714, I compiled a list of guns that do not meet our criteria for inclusion. There was a lot and it was over a pretty wide variety of guns.

Gunbroker stats STI 2011

image
For the life of me I couldn't find any other double stack 19/2011s, the closest was the EAA Hunter which didn't make the cut either.

P-3AT/PF-9
image
image
IIRC Tonk added these and both had a description of them being common so I'm a bit hesitant to remove em.

M714
image
It seems to be a rare item so I didn't apply the gun filter to be nice and yeah, it's not cutting it.

Taurus Spectrum
image

ZPAP 85
image
I was going to axe the M90 too, but realized it passed. Still had some redundant groups though.

.40 90-two
image

.40 PPQ
image

P230
image
Majority of P230s seem to be 9mm or 380.

ARX-160
image
These are all .22 LR. While there seems to be some military use, the list of users is pretty short and America isn't really mentioned past some trials.

CX4
image

Saiga 410
image

Describe the solution

Removes em.

Describe alternatives you've considered

Not removing em.

Testing

Before:
image

After:
image
Note that the M90 was in the original as I intended to replace it, but I have undone it since.

Additional context

Mucked up and realized M90 passes and quite well, got mixed up with the M85
@github-actions github-actions bot added [JSON] Changes (can be) made in JSON Items: Magazines Ammo holding items and objects. Items: Gunmod / Toolmod Weapon and tool attachments, and add-ons Spawn Creatures, items, vehicles, locations appearing on map Code: Tests Measurement, self-control, statistics, balancing. Items: Ammo / Guns Ammunition for all kinds of weapons and these weapons themselves Mods: Generic Guns Anything to do with Generic Guns Mods: Dark Days of the Dead Anything related to the DDotD mod (classic style zombies) Mods: Defense Mode Anything to do with the Defense Mode mod <Bugfix> This is a fix for a bug (or closes open issue) labels Oct 20, 2024
Forgot GG will complain about obsoleted guns
@AudBobb
Copy link
Contributor

AudBobb commented Oct 20, 2024

Oh boy I can see the reddit posts already

@github-actions github-actions bot added astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions labels Oct 20, 2024
@NetSysFire
Copy link
Member

NetSysFire commented Oct 20, 2024

Please be a bit more specific with your PR description. "Gun removals" is not very helpful in a changelog.

@Holli-Git
Copy link
Contributor Author

Please be a bit more specific with your PR description. "Gun removals" is not very helpful in a changelog.

How would you like me to be more specific? The guns removed, why guns are removed? I'm always a bit unsure how much detail I need to put into them so I go on the short side

@Nebnis
Copy link
Contributor

Nebnis commented Oct 21, 2024

"Removed x and x and x because there is not enough hits in gunbrokers"

@NetSysFire
Copy link
Member

I recommend imagining your PR description in the changelog if you are having trouble gauging whats right. Take this one for example: https://github.com/CleverRaven/Cataclysm-DDA/releases/tag/cdda-experimental-2024-10-20-1406

"Gun removals" would stick out because it is very vague with no details. Look through more changelogs if you'd like and you can get a feeling for what level of detail is appropriate.

@Holli-Git
Copy link
Contributor Author

"Gun removals" would stick out because it is very vague with no details. Look through more changelogs if you'd like and you can get a feeling for what level of detail is appropriate.

Is this better?

@NetSysFire
Copy link
Member

Yes. Personally I'd likely use something like "remove mistakenly included guns" or similar to make it clear that these should probably never have existed in the first place. But this is all subjective now.

@Holli-Git
Copy link
Contributor Author

Yes. Personally I'd likely use something like "remove mistakenly included guns" or similar to make it clear that these should probably never have existed in the first place. But this is all subjective now.

I didn't do that as at one point these weren't mistakenly included as the criteria didn't exist, even if the specific guns were very odd choices. Not trying to say your idea was wrong, just wanting to explain my thinking, if it is of any use.

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Oct 21, 2024
@x-qq
Copy link

x-qq commented Oct 21, 2024

Out of the entire list, Cx4 is the only useful thing.

It is sad to see it go.

@Holli-Git
Copy link
Contributor Author

Out of the entire list, Cx4 is the only useful thing.

Eh we have the Sub 2000, which is pretty similar and takes glock mags

@kevingranade
Copy link
Member

Regarding the summary, this one would be a "None".
If anything these removals (in aggregate) would boil down to something like "Applied objective criteria for gun appearance rates to prevent oddball guns dominating spawns."

I took a few minutes to review the gunbroker stats, mostly because you expressed some concern yourself.

STI 2011 - This one is a pain, STI calls 2011 their "platform" and they seem very willing to brand what would normally be considered variants of the same gun as different gun lines based on relatively small changes like slightly different barrel lengths. This means no one "model" meets our inclusion criteria, but in aggregate they would do so easily. What we have in the game though is the "DS" which is not only discontinued but apparently not that popular in the first place, so no issue with the removal.

P3AT I was able to widen it a bit by searching for KelTek pistols chambered in .380, which all seem to be P3ATs, but it still only had ~50 hits.

PF9 again shows up under KelTek/9mm, but in far less than the requisite numbers. It looks like if you smushed together the P11, PF9, and P15 into one gun it would barely scrape by? There's some amount of that we could look at doing so that if you have many similar subcompact pistols that individually miss the threshold you can still have some representation for that "genre" of gun, but that requires something more than just the threshold based approach we have now so it's not pertinent for this issue.

Ideally we'd have a big table of guns and their characteristics and gunbroker stats, and could use that to highlight types of guns that are common enough in aggregate to share a spot on the spawn tables.

As for the rest the Siaga 410 surprised me but my search agreed with yours, .410 is just really not that popular.

@kevingranade kevingranade merged commit b246d05 into CleverRaven:master Oct 21, 2024
27 checks passed
@NetSysFire
Copy link
Member

Obsoleting (and thus migrating) a good amount of items is definitely not a "None" in terms of the PR description for the changelog.

@AudBobb
Copy link
Contributor

AudBobb commented Oct 22, 2024

should probably have an obseletion tag no?

@NetSysFire
Copy link
Member

This PR properly obsoletes many items. I do not know what you mean with "obseletion tag". It is normally not specifically marked in the changelog or the PR title.

@AudBobb
Copy link
Contributor

AudBobb commented Oct 22, 2024

I mean a category in the changelog for obseletions. So instead of [Bugfix "removes item name"] we could do [Obsoleted "item name"], because having something like this as a "None" doesn't make sense, but it's also not really a bugfix. It's an obseletion.

@NetSysFire
Copy link
Member

Probably Content as a category as it affects content.

@Holli-Git
Copy link
Contributor Author

I've been putting it as bugfixes due to my definition of a bug being an unwanted/unintended feature, and and obsoletions remove unwanted features such as really rare guns

@Jesper-Johansson

This comment was marked as off-topic.

@Holli-Git
Copy link
Contributor Author

Holli-Git commented Oct 22, 2024

I get wanting to combat bloat, but ideally there should be a way for very uncommon guns to be in the game. These guns exist IRL, so the most immersive implementation would be balancing their rarity so maybe you come across one once in a blue moon, in the basement of some foreign war veteran or something.

It adds flavor and variety, which is one of CDDA:s core strengths.

A) My intention isn't combatting bloat. I'm not removing guns for the sake of it, I'm removing it because they don't fit.

B) I never understood why there is so much uproar about the uncommon guns. We don't have much common guns. There are so many guns out in the world and it seems a portion of CDDA players only care about the very uncommon ones. We quite literally don't have enough common guns to make the uncommon ones uncommon. I'm not talking about ARs or Glocks for common guns either, I'm talking R92s, HiPoint Carbines, Ruger PCCs. That's another reason I'm removing all of these guns, it makes it a whole lot easier to clean up item groups and implement guns that actually make reasonable sense to find in the USA. And also, just so happens removing guns is easier than adding them, so I did those first.

C) There is nothing stopping you from working in the uncommon guns that would still fit the games inclusion criteria. It's not my job to bend to your will.

@AudBobb

This comment was marked as off-topic.

@RenechCDDA
Copy link
Member

RenechCDDA commented Oct 22, 2024

A reminder to those coming from off-site that our gun policy (the one which allows us to have 300+ separate firearms and hundreds more variants) is the result of years of discussion. The idea of adding or keeping impossible to find things to the game is not a new one, and it is an idea that has been soundly rejected.

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 <Bugfix> This is a fix for a bug (or closes open issue) Code: Tests Measurement, self-control, statistics, balancing. Items: Ammo / Guns Ammunition for all kinds of weapons and these weapons themselves Items: Gunmod / Toolmod Weapon and tool attachments, and add-ons Items: Magazines Ammo holding items and objects. [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions Mods: Dark Days of the Dead Anything related to the DDotD mod (classic style zombies) Mods: Defense Mode Anything to do with the Defense Mode mod Mods: Generic Guns Anything to do with Generic Guns Spawn Creatures, items, vehicles, locations appearing on map
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants