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

Ballistic armor syntax audit #56763

Merged
merged 7 commits into from
Apr 19, 2022
Merged

Ballistic armor syntax audit #56763

merged 7 commits into from
Apr 19, 2022

Conversation

Ilysen
Copy link
Contributor

@Ilysen Ilysen commented Apr 12, 2022

Summary

Content "Touched up the descriptions of various ballistic armors"

Purpose of change

The new ballistic armors suffered from the same problems that I'm addressing for some other items in #56641 - essentially, wording that was often repetitive or had grammatical issues.

Describe the solution

  • Adjusted the descriptions of the majority of ballistic armors and their variants, such as the US ballistic vest, the stab vest, and pretty much everything else.
  • The prices of items in ballistic_armor.json now use the USD syntax instead of cents (i.e. 1900 USD instead of 190000).
  • Pockets for body and side plates now specifically refer to Pocket for a large armor plate. and so on, instead of attempting to differentiate front/back and left/right in their descriptions. The game automatically consolidated these pockets when displaying them due to having the exact same storage parameters, resulting in one of the descriptions not being displayed in the first place.

Describe alternatives you've considered

I was originally going to adjust the dragon skin vest and lamellar Kevlar vest as well, but these items' real-world counterparts have some stuff that might be out-of-scope to mention here (controversies with Pinnacle Armor, mainly), so I held off.

Testing

I spawned in stuff with debug.

Additional context

I'm uncertain if these are all good changes. The large swathe of ballistic armors introduced that are pretty much the same is kind of bloaty and makes my head spin, so it's pretty likely that I missed something or got a description in the wrong spot, or in general didn't grasp what kind of vibe the armor was going for. Please give me a holler if that's the case.

@github-actions github-actions bot added [JSON] Changes (can be) made in JSON astyled astyled PR, label is assigned by github actions labels Apr 12, 2022
@github-actions github-actions bot added the json-styled JSON lint passed, label assigned by github actions label Apr 12, 2022
@github-actions
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
  • A low-cost tactical vest styled in black camouflage, used in cosplay and airsoft. It'll let you shrug off airsoft pellets like a badass, but its protection is medicore against anything that's actually dangerous. It sports several small pouches, and includes touch fasteners to attach identifying patches.

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.

Copy link
Contributor

@bombasticSlacks bombasticSlacks left a comment

Choose a reason for hiding this comment

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

Thanks as always for the cleanup. Some of the older really clunky descriptions that predate me getting a touch up was well overdue as well.

Sorry if it was a lot to sort through. Other than the one confusion with the 3a vs 3 vests you seem to have a handle on everything 😄

data/json/items/armor/ballistic_armor.json Show resolved Hide resolved
data/json/items/armor/ballistic_armor.json Outdated Show resolved Hide resolved
data/json/items/armor/ballistic_armor.json Outdated Show resolved Hide resolved
data/json/items/armor/ballistic_armor.json Outdated Show resolved Hide resolved
@bombasticSlacks
Copy link
Contributor

As an aside as I understand it the disk armor is basically deprecated, last I checked it didn't really spawn anywhere.

@Ilysen
Copy link
Contributor Author

Ilysen commented Apr 12, 2022

I'd like to follow up on the disk armor later, yeah. Specifically, I think it's cool that it exists, but I want to update its description and spawn locations to reflect its real-world counterpart, which promised to be the next generation of body armor but ended up flopping for several reasons. I think it'd fit perfectly as a rare spawn in collectors' places; I think it'd be a shame to remove it entirely, since it taught me something cool and I think it's neat to keep in that way.

The funny thing is that I think it might have been added before the controversies and such, which is why it's talked about so favorably! It's been around as long as I can remember. It's an in-game example of content changing with time passing.

@bombasticSlacks
Copy link
Contributor

Well though I'm sure you'd be quite capable yourself if you'd like me to give it a balance pass when you give it some love I would be happy to do it. I also think it's a cool item and giving it some appropriate homes in game would be great.

@Ilysen
Copy link
Contributor Author

Ilysen commented Apr 12, 2022

Not sure yet. It might be fine as-is, but I've never used one as actual armor.

Copy link
Contributor

@Faalagorn Faalagorn left a comment

Choose a reason for hiding this comment

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

I left some comments based on the changes I made in #56697 and you have one missed space noted by GitHub action. You also seem to incorporated the change I made in #56695 so if yours gets merged instead, I guess mine could be closed. :)

data/json/items/armor/ballistic_armor.json Outdated Show resolved Hide resolved
data/json/items/armor/ballistic_armor.json Outdated Show resolved Hide resolved
data/json/items/armor/ballistic_armor.json Outdated Show resolved Hide resolved
data/json/items/armor/ballistic_armor.json Outdated Show resolved Hide resolved
data/json/items/armor/ballistic_armor.json Outdated Show resolved Hide resolved
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Apr 16, 2022
@kevingranade kevingranade merged commit 46ffb0f into CleverRaven:master Apr 19, 2022
@kevingranade
Copy link
Member

Side note for the dragonskin removal. It was pulled not because of the controversy but because if the outcome of that controversy. I.e. because now that it's not on the market any more it's prohibitively rare.

@Ilysen
Copy link
Contributor Author

Ilysen commented Apr 19, 2022

Makes sense. I was fine having the fictionalized story taking some liberties, but worst-case scenario, we can tweak the description a bit more in a different PR.

@Ilysen Ilysen deleted the vest-grammarification branch April 19, 2022 05:30
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 [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.

4 participants