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

Toggle engines #9755

Merged
merged 21 commits into from Nov 2, 2014

Conversation

Projects
None yet
5 participants
@malhawee
Copy link
Contributor

commented Oct 31, 2014

Adds ability to toggle individual engines in vehicle control menu, when more than one engine present. Choice brings up submenu listing all engines. Velocities, noise and fuel usage update accordingly.
If velocity after toggling is higher than the new safe velocity, the cruise control speed is lowered.

TODO: make radiators only work if the engine they are attached to is enabled

@malhawee

This comment has been minimized.

Copy link
Contributor Author

commented Oct 31, 2014

Much less buggy than that hybrid PR I had going before.

@KA101

This comment has been minimized.

Copy link
Contributor

commented Oct 31, 2014

What radiators? Didn't think we had any. Alternators maybe?

@malhawee

This comment has been minimized.

Copy link
Contributor Author

commented Oct 31, 2014

Whoops, you're right, alternators

@malhawee malhawee force-pushed the malhawee:ToggleEngines branch to 523dc97 Oct 31, 2014

@malhawee

This comment has been minimized.

Copy link
Contributor Author

commented Oct 31, 2014

Alternators work now! Calling it feature complete.

@malhawee malhawee changed the title Toggle engines (WIP) Toggle engines Oct 31, 2014

@KA101

This comment has been minimized.

Copy link
Contributor

commented Oct 31, 2014

I'm hesitant to grab this right now given that we're assembling a stable release, but once that's done, I'm confident that it will be VERY popular.

Update: I mentioned it on the IRC. May get in after all.

}

void vehicle::toggle_specific_engine(int p,bool on){
parts[engines[p]].enabled = on;

This comment has been minimized.

Copy link
@KA101

KA101 Oct 31, 2014

Contributor

Eyeball check: I'm not seeing anything to turn an engine off here?

This comment has been minimized.

Copy link
@malhawee

malhawee Oct 31, 2014

Author Contributor

Yep, that function gets the user to select an engine index in the engines vector. Its called from control_engines, which handles disabling the engines. It calls toggle_specific engine to turn the engine on/off.

This comment has been minimized.

Copy link
@KA101

KA101 Oct 31, 2014

Contributor

Well, that's the thing--I'm not seeing an "off" here. Someplace else I should be looking?

@malhawee

This comment has been minimized.

Copy link
Contributor Author

commented Oct 31, 2014

Heh, I figured people would want it in the next release, I certainly do.

@Robik81

This comment has been minimized.

Copy link
Contributor

commented Oct 31, 2014

Oh yes, this feature was sorely missing. True hybrid cars FTW.

if (e_toggle >= 0 && e_toggle < (int)engines.size() &&
(active_count > 1 || !is_engine_on(e_toggle))){
active_count += (!is_engine_on(e_toggle))?1:-1;
toggle_specific_engine(e_toggle, !is_engine_on(e_toggle));

This comment has been minimized.

Copy link
@malhawee

malhawee Oct 31, 2014

Author Contributor

This line (441) toggles engines on or off. It first fetches the state of the engine, and inverts it.

This comment has been minimized.

Copy link
@KA101

KA101 Oct 31, 2014

Contributor

OK, thanks.

parts[engines[p]].enabled = on;
}

bool vehicle::is_engine_on(int p){

This comment has been minimized.

Copy link
@kevingranade

kevingranade Nov 1, 2014

Member

How about pulling the damage and fuel checks into this too? That would clean up a lot of code I think.

return parts[engines[p]].enabled;
}

bool vehicle::is_part_on(int p){

This comment has been minimized.

Copy link
@kevingranade

kevingranade Nov 1, 2014

Member

Especially if you do the above, you probably want one method that expects a part index that does the real work, and another method that expects the engine index and just passes the part index to the other one.

}

bool vehicle::is_active_engine_at(int x,int y){
for( size_t p = 0; p < engines.size(); ++p ) {

This comment has been minimized.

Copy link
@kevingranade

kevingranade Nov 1, 2014

Member

Probably redefine this one in terms of calling the is_part_on() method too, nice to have it only check things in one place.

@@ -474,6 +549,7 @@ void vehicle::use_controls()
has_reactor = true;
}
else if (part_flag(p, "ENGINE")) {
has_mult_engine = has_engine;

This comment has been minimized.

Copy link
@kevingranade
switch(options_choice[select]) {
//brackets prevent initialisation errors

This comment has been minimized.

Copy link
@kevingranade

kevingranade Nov 1, 2014

Member

I'm guessing this comment is obsolete.

@@ -10,6 +10,7 @@
#include <map>
#include <string>
#include <iosfwd>
#include "ui.h"

This comment has been minimized.

Copy link
@kevingranade

kevingranade Nov 1, 2014

Member

Why add this here? If it's only referenced in vehicle.cpp, #include it there.

@malhawee

This comment has been minimized.

Copy link
Contributor Author

commented Nov 2, 2014

Looks like I've made all the code cleaner than it was before. My testing seems to show that its still working as expected.

active_count += (!is_engine_on(e_toggle))?1:-1;
toggle_specific_engine(e_toggle, !is_engine_on(e_toggle));

add_msg("Switched %s %s",part_info(engines[e_toggle]).name.c_str(),

This comment has been minimized.

Copy link
@BevapDin

BevapDin Nov 2, 2014

Contributor

This needs _() to be translated. The "on" / "off" needs this, too.

// control an engine
if (has_mult_engine) {
options_choice.push_back(cont_engines);
options_message.push_back(uimenu_entry("Control individual engines", 'y'));

This comment has been minimized.

Copy link
@BevapDin

BevapDin Nov 2, 2014

Contributor

Translation missing.

@kevingranade kevingranade merged commit 07a12f0 into CleverRaven:master Nov 2, 2014

1 check passed

default This has been rescheduled for testing as the 'master' branch has been updated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.