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

Added golf ball to dodge practice #60642

Merged
merged 1 commit into from
Sep 2, 2022

Conversation

rj8216
Copy link
Contributor

@rj8216 rj8216 commented Sep 1, 2022

Golf balls for dodging practice

Summary

Balance "Added Golf Ball to required item for Dodge (Beginner) practice recipe"

Purpose of change

The description of the dodge practice "Practice your dodging by throwing a ball at a wall and dodging it when it bounces off." would imply that one only needs a ball capable of bouncing off a hard surface when thrown. In reality a golf ball certainly merits being added to the various balls that are currently available. They have hard rubber cores, and reliably bounce when thrown with any force. They are usually brightly colored to help a person to aid with eye / body coordination while dodging.

Describe the solution

I added golf ball to the dodge practice recipe.

Describe alternatives you've considered

In the dev discord, someone suggested adding "rubber golf ball," but that seems needless since golf balls are already 80% rubber. This seemed the simplest fix.

Testing

I did edit the recipe on my local game on my PC, and successfully was able to practice with a golf ball, and gain dodge proficiency. Nothing bad seemed to happen.

None

Additional context

I can give anyone a hilarious gifycat link to an emu throwing a golf ball on a side walk, to watch it fall and practice dodging it. I just figured on here wasn't the right place. I also have links to articles about the makeup and composition of a standard golf ball. Just saying I did my research!

Golf balls are really bouncy too!
@github-actions github-actions bot added [JSON] Changes (can be) made in JSON Crafting / Construction / Recipes Includes: Uncrafting / Disassembling Melee Melee weapons, tactics, techniques, reach attack labels Sep 1, 2022
@rj8216
Copy link
Contributor Author

rj8216 commented Sep 1, 2022

It felt weird that golf balls weren't eligible for this dodge training. They have rubber cores, and the description says the PC throws a ball against a hard surface, so golf balls should count for sure. This is my first time doing this, LMK if everything is alright.

@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 Sep 1, 2022
@PatrikLundell
Copy link
Contributor

PatrikLundell commented Sep 1, 2022

The PR validator fails because the summary doesn't match the format the comment describes. That format is parsed by a script, and so is rigid. The summary is used to automatically add revision history entries (I believe manual processing then removes most of them, but that's after the script has been satisfied). I typically just use None here, as I don't consider my PRs to be revision history entry worthy.

Purpose of change ought to have a line. That can basically be a repeat of the title. Similarly the solution section should have something. I would assume it could be "Added golf balls as valid balls for dodge practice recipe." (without the quotes).

Testing DEFINITELY should have something. Untested things are suspect in general, and doubly so when they're from new people. This doesn't have to be anything extreme, but I'd expect you to say that you've loaded the game and verified that you've been able to use a golf ball for dodge training (and that you've actually done that, not just lied about it, of course). There are various reasons for why "trivial" changes may not work as intended, such as e.g. modifying a similar recipe instead of the one intended (which might result in a check whether both should be modified when verification fails).

Your comment after the PR would have been better as part of the PR body. The "..." icon contains an entry that allows you to edit the PR body.

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Sep 1, 2022
@rj8216
Copy link
Contributor Author

rj8216 commented Sep 1, 2022

Thanks for the help. Sorry had a long day at work, and this is my first time doing this, so it took a hot minute for me to get to this. I appreciate the patience, love this game and just want to help make it better. :-D hopefully I followed directions a little better this time.

@Rivet-the-Zombie Rivet-the-Zombie merged commit 75bf50e into CleverRaven:master Sep 2, 2022
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 Crafting / Construction / Recipes Includes: Uncrafting / Disassembling [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions Melee Melee weapons, tactics, techniques, reach attack
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants