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

Comprehensive grammar sweep #46181

Merged
merged 53 commits into from
Jan 5, 2021
Merged

Conversation

ToxiClay
Copy link
Contributor

Summary

SUMMARY: None

Purpose of change

The purpose of this change is to correct, standardize, and prettify the texts.json file that controls what is displayed in the game's Help menu.

Describe the solution

I sat down and went over the file.

Describe alternatives you've considered

None.

Testing

Attempting to lint the file resulted in a syntax error alert. Impact unknown.

Additional context

data/help/texts.json Outdated Show resolved Hide resolved
Copy link
Contributor

@actual-nh actual-nh left a comment

Choose a reason for hiding this comment

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

Definite improvements!

data/help/texts.json Outdated Show resolved Hide resolved
data/help/texts.json Outdated Show resolved Hide resolved
data/help/texts.json Outdated Show resolved Hide resolved
data/help/texts.json Outdated Show resolved Hide resolved
data/help/texts.json Outdated Show resolved Hide resolved
data/help/texts.json Outdated Show resolved Hide resolved
data/help/texts.json Outdated Show resolved Hide resolved
data/help/texts.json Outdated Show resolved Hide resolved
data/help/texts.json Outdated Show resolved Hide resolved
data/help/texts.json Outdated Show resolved Hide resolved
@anothersimulacrum anothersimulacrum added the [JSON] Changes (can be) made in JSON label Dec 20, 2020
ToxiClay and others added 26 commits December 20, 2020 13:53
Co-authored-by: Jianxiang Wang (王健翔) <[email protected]>
Wouldn't want someone trying to use a military ID card to open a science lab

Co-authored-by: actual-nh <[email protected]>
Co-authored-by: actual-nh <[email protected]>
Style guides don't seem to agree on whether to follow i.e. with a comma.

Co-authored-by: actual-nh <[email protected]>
Co-authored-by: actual-nh <[email protected]>
@ToxiClay
Copy link
Contributor Author

All proposed changes reviewed and incorporated, to one extent or another.

Lingering issue: in most cases, the Help screen uses a dynamic replacement for buttons, like <press_fire> and <press_examine>, but there are still places where static string literals are used instead.

To fire, press <press_fire>, move the cursor to the desired tile, press '.' to improve your aim, and then 'f' to fire. Additionally, you may want to fire at predefined aim levels using 'a', 'c' or 'p'.

I couldn't find where the codebase sets out what the <press_X> equivalents are for these, and so I couldn't update them. Assistance here would be appreciated.

@actual-nh
Copy link
Contributor

See data/raw/keybindings.json#L1040. Looks like <press_aim>, <press_aimed_shot>, <press_careful_shot>, and <press_precise_shot> should work, although this will require testing.

data/help/texts.json Outdated Show resolved Hide resolved
Co-authored-by: actual-nh <[email protected]>
@ToxiClay
Copy link
Contributor Author

See data/raw/keybindings.json#L1040. Looks like <press_aim>, <press_aimed_shot>, <press_careful_shot>, and <press_precise_shot> should work, although this will require testing.

It seems not...fortunately, I can test JSON edits without needing to build CDDA from source.

ReplacementFailure

<press_pause> is valid in place of <press_aim>, and both . and 5 work in game. I can't determine what to put in for the others.

@actual-nh
Copy link
Contributor

actual-nh commented Dec 21, 2020

Hrm. Ah. I think I see where I went wrong. It looks like only ones in action.cpp's action_ident or look_up_action work... which doesn't include any of these at the moment. Change in a future PR, perhaps.

data/help/texts.json Outdated Show resolved Hide resolved
data/help/texts.json Outdated Show resolved Hide resolved
ToxiClay and others added 2 commits December 20, 2020 20:39
Co-authored-by: actual-nh <[email protected]>
data/help/texts.json Outdated Show resolved Hide resolved
Co-authored-by: Serhiy Zahoriya <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[JSON] Changes (can be) made in JSON
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants