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

Prevent dialog asking to enter dangerous terrain #35312

Merged
merged 108 commits into from
Dec 10, 2019

Conversation

Ramza13
Copy link
Contributor

@Ramza13 Ramza13 commented Nov 4, 2019

Summary

SUMMARY: Interface "Prevent dialog asking to enter dangerous terrain"

Purpose of change

Fixes #28215
When attempting to walk into dangerous terrain it will be treated as a wall and print a message telling you to run if you want to enter it. If you run into it you will go onto it with no warning or you will be warned depending on a new option which defaults to warning you.

Describe the solution

Describe alternatives you've considered

Testing

Additional context

Ramza13 and others added 30 commits October 10, 2019 07:35
Co-Authored-By: anothersimulacrum <[email protected]>
This reverts commit 88637aa.
… of item on arm break if it can't be wielded anymore
This reverts commit 98e88b3.
…d decided to still allow martial arts/normal attack damage while on the ground
Fix some more typos and delete some extra spaces
@Ramza13
Copy link
Contributor Author

Ramza13 commented Nov 19, 2019

I added an option which defaults to false to prevent being warned while running. If you have this option set to true you will not be warned while running. If you have it set to false then when you run into a dangerous tile you will be warned.

src/options.cpp Outdated Show resolved Hide resolved
src/game.cpp Outdated Show resolved Hide resolved
@esotericist
Copy link
Contributor

I'm terribly sorry for how long it took for me to give feedback on this; I somehow completely spaced that you had put work into making the changes I was requesting

@Ramza13
Copy link
Contributor Author

Ramza13 commented Dec 9, 2019

I'm terribly sorry for how long it took for me to give feedback on this; I somehow completely spaced that you had put work into making the changes I was requesting

Ha no worries its the holiday season after all. Thanks for helping me out!

@esotericist
Copy link
Contributor

I started to complain that you hadn't addressed my concern about not checking for the config setting when moving into the tile, but I only now realized that turning on running still gets you the "are you sure?" prompt if dangerous running is off, so the instructions are correct.

I feel dumb for missing that, and I should have done actual testing.

Let me do that now.

@esotericist esotericist merged commit e8bfb5e into CleverRaven:master Dec 10, 2019
@esotericist
Copy link
Contributor

esotericist commented Dec 10, 2019

@Ramza13 The commit history for this is unnecessarily long considering how small a change it is, so I do need to urge you once again to learn more about how git works, and in particular how to set up branches off of upstream master (so you aren't carrying around all of that history) and how to rebase things when needed (to move changes to a newer version of upstream master).

Please try to work on this for other contributions? I squashed it here for merging, but it's easier on reviewers if they don't have to wade past the other stuff.

fengjixuchui added a commit to fengjixuchui/Cataclysm-DDA that referenced this pull request Dec 10, 2019
Prevent dialog asking to enter dangerous terrain (CleverRaven#35312)
@Ramza13 Ramza13 deleted the prevent_dangerous branch December 10, 2019 12:37
@RDru
Copy link
Contributor

RDru commented Dec 10, 2019

I'm sorry but it's kinda illogical to have to run to be able to pass dangerous terrain like wreckage. In reality you would prefer to walk carefully over them.

Plus this is worse than before. Even with the option set to true, Now I have to press cycle move 3 times to pass dangerous terrain when before I had to press once ('y').

@I-am-Erk
Copy link
Member

I-am-Erk commented Dec 10, 2019

Wreckage is a bit of a niche scenario there, it's the only dangerous terrain I can think of where you might actually want to walk on it. We might need a flag for terrain like that to change the behaviour. Possibly making it climbable would work, then you'd be back to one press.

@esotericist
Copy link
Contributor

esotericist commented Dec 10, 2019

Wreckage is a bit of a niche scenario there, it's the only dangerous terrain I can think of where you might actually want to walk on it. We might need a flag for terrain like that to change the behaviour. Possibly making it climbable would work, then you'd be back to one press.

My hope is to actually get us a "safety" concept component to movement mode, alongside speed. That would cover this problem space (among others) much better than this PR did, but this does the job for now.

@esotericist
Copy link
Contributor

Plus this is worse than before. Even with the option set to true, Now I have to press cycle move 3 times to pass dangerous terrain when before I had to press once ('y').

Fwiw, you can always bind the "toggle run" keybinding or the movement mode menu, instead of using cycle movement mode

@Ramza13
Copy link
Contributor Author

Ramza13 commented Dec 10, 2019

Is it worth adding an option to disable this and just use the previous behavior?

@esotericist
Copy link
Contributor

Is it worth adding an option to disable this and just use the previous behavior?

Could change the setting from a boolean to a three-way, though it'd need different framing

@Ramza13
Copy link
Contributor Author

Ramza13 commented Dec 10, 2019

Is it worth adding an option to disable this and just use the previous behavior?

Could change the setting from a boolean to a three-way, though it'd need different framing

I'll probably make it four and allow you to set crouching as the movement style also.

@Unrepentant-Atheist
Copy link

I don't like it like it is now. Please make it an option, because I'm getting kinda annoyed by this:
image

I could do that while walking not running before, now I have to switch to walk, then switch off again. C'mon....no, just no. Good idea, bad implementation.

@Ramza13
Copy link
Contributor Author

Ramza13 commented Dec 16, 2019

I have a pr awaiting review right now to make it optional #36037

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` Fields / Furniture / Terrain / Traps Objects that are part of the map or its features. Info / User Interface Game - player communication, menus, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature/fix: Moving over this repeated message is slow.