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

Removing legacy json name / name_plural convention. #36611

Closed
wants to merge 2 commits into from

Conversation

jayhide
Copy link

@jayhide jayhide commented Jan 1, 2020

#35102 Summary

SUMMARY: Infrastructure "Replacing legacy json for name strings with more up-to-date conventions."

Purpose of change

As described in #36110 this is for replacing the legacy json throughout the code with more current conventions, keeping it consistent for ease of tooling and other development. This PR switches all older instances of name string formatting to the current convention.

Describe the solution

Changes were automated via python script.

Progress:

  • Removed instances of "name_plural": "fishes" key / value in json items and replaced with the "name": {"str": "fish", "str_pl": "fishes"} format.

Testing

Ran all unit tests and wish-spawned some of the affected items in plural and singular numbers just to check.

Additional context

This was done via python script rather than by hand, and as I am brand new to the codebase, I would appreciate any eyes or commentary.

…o preferred "name": { "str": [X], "str_pl": [Y] } format.
@ymber
Copy link
Member

ymber commented Jan 1, 2020

Don't try to do all of #36110 in a single PR.

@jayhide jayhide changed the title [WIP] Removing legacy json to make json consistent [WIP] Removing legacy json name / name_plural convention. Jan 1, 2020
@jayhide
Copy link
Author

jayhide commented Jan 1, 2020

Don't try to do all of #36110 in a single PR.

Yeah good idea, I'll break it up.

Doing this update for data/json/body_parts.json alone breaks tests, because (I think) of hardcoding in bodypart.cpp lines 185-186 where we expect name and name_plural. Should I leave body_parts.json alone or change the code?

@Qrox
Copy link
Contributor

Qrox commented Jan 2, 2020

The plural_name of bodyparts are actually not plural names -- they are names for a pair of body parts, such as arms and legs. It's done this way because some languages do not have plural forms so these names should be separately translated.

@jayhide
Copy link
Author

jayhide commented Jan 2, 2020

Ah, gotcha. I'll just leave that one alone then.

@jayhide jayhide changed the title [WIP] Removing legacy json name / name_plural convention. Removing legacy json name / name_plural convention. Jan 2, 2020
@I-am-Erk
Copy link
Member

I-am-Erk commented Jan 5, 2020

This is incredibly long. I've reviewed a large number and seen no problems, but have not read every single change. If there's a problem, I'll have your badge and gun jayhide!

You need to resolve conflicts though

@ymber
Copy link
Member

ymber commented Jan 5, 2020

Did you do a new script for this or adapt adjust_values.py? If you did something new put it in tools/ so other people can use it. Checking the script is valid is also a lot easier than going through thousands of automated changes.

@kevingranade
Copy link
Member

This touches too many strings to sneak it in right now, it's a perfect "right after a release" PR though.

@ZhilkinSerg
Copy link
Contributor

I will close this PR as it has too many conflicts while @snipercup is doing the same thing in small steps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Items / Item Actions / Item Qualities Items and how they work and interact [JSON] Changes (can be) made in JSON
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants