Join GitHub today
GitHub is home to over 36 million developers working together to host and review code, manage projects, and build software together.
Sign upDriving & safemode fixes (PWR) #4771
Conversation
KA101
added some commits
Nov 30, 2013
narc0tiq
reviewed
Nov 30, 2013
| @@ -2067,14 +2067,15 @@ bool game::handle_action() | |||
| switch (act) { | |||
|
|
|||
| case ACTION_PAUSE: | |||
| if (run_mode == 2 && (u.controlling_vehicle && safemodeveh) ) { // Monsters around and we don't wanna pause | |||
| if (run_mode == 2 && ((u.controlling_vehicle && safemodeveh) || !(u.controlling_vehicle))) { // Monsters around and we don't wanna pause | |||
This comment has been minimized.
This comment has been minimized.
narc0tiq
Nov 30, 2013
Contributor
This looks really strange, and caught my eye. It's semantically identical to run_mode == 2 && (safemodeveh || !u.controlling_vehicle), and I'm not sure if that's the actual intent of the test.
This comment has been minimized.
This comment has been minimized.
KA101
Nov 30, 2013
Author
Contributor
OK, that streamlines things. Was intended to have safemode both stop the action and throw the warning; it wasn't kicking in when on foot, likely thanks to the Web Weaver action. As this stands, it stops you just fine but doesn't throw the warning when you're driving.
This comment has been minimized.
This comment has been minimized.
narc0tiq
Nov 30, 2013
Contributor
In that case, yep, that'll do it. In plain English: "if we're in safe mode and a monster is detected, and either we're not driving, or else we're driving but the player doesn't want to trigger safe-mode while driving".
narc0tiq
reviewed
Nov 30, 2013
| use the vehicle controls, %s to bring up the \"Vehicle Controls\" menu."), | ||
| use the vehicle controls, %s to bring up the \"Vehicle Controls\" menu.\n\ | ||
| \n\ | ||
| Recently, we improved the engine code. You must now manually turn\n\ |
This comment has been minimized.
This comment has been minimized.
narc0tiq
Nov 30, 2013
Contributor
This is a severe contrast in writing style from the earlier "instruction manual" style to "the developers are speaking directly to you" style. And it gives some bad news to the player. And it doesn't actually tell them how to manually turn on the engine, nor how to turn it off when finished (i.e. what option in the vehicle controls does what).
This comment has been minimized.
This comment has been minimized.
axujen
Nov 30, 2013
Contributor
I agree, it looks awkward in here, the player should simply be told how turn on/off the engine.
This comment has been minimized.
This comment has been minimized.
KA101
Nov 30, 2013
Author
Contributor
We can probably just scrap this part of the PR altogether, given @yobbobanana's PR. Agreed that I could tweak the style.
...but folks? If you want to toggle the engine and can't figure out how to do so in a menu that says "turn (on/off) engine", there's no hope for you.
This comment has been minimized.
This comment has been minimized.
narc0tiq
Nov 30, 2013
Contributor
If you want to toggle the engine and can't figure out how to do so in a menu that says "turn (on/off) engine", there's no hope for you.
That's not the point. Anything that might get in someone's way will get in someone's way eventually. If the reader missed the "in the vehicle controls menu" part of that sentence, they'll be looking everywhere, complaining, and likely eventually feeling stupid when they do see it -- and you do not want to make people feel stupid if you can avoid it.
KA101
added some commits
Nov 30, 2013
This comment has been minimized.
This comment has been minimized.
|
Check that: "Cleanup a bit" jams vehicle-safemode in the Off position, not the On. You can jam it On by changing the "else" to "if (run_mode <=1) and bracing the u.pause. |
KA101
closed this
Nov 30, 2013
KA101
reopened this
Nov 30, 2013
This comment has been minimized.
This comment has been minimized.
|
@narc0tiq was quite helpful on the trace there. Thanks much. |
KA101 commentedNov 30, 2013
In response to http://smf.cataclysmdda.com/index.php?topic=4541.0 . Looks like vehicle-safemode may have been the culprit, but Web Weaver certainly contributed. Noe Web Weaver's in player.cpp where it belongs and the option calls go direct, thus providing the sort of realtime-changing that players'll want.
Tested and works. Feel free to pull when ready.
(I thought the engine-upgrade merited a Help update, too. #4770 is a good solution too.)