Vehicle installation requirements (part 2) [RDY] #17209

mugling · 2016-06-16T10:13:05Z

Replaces #16941 and follows #17208

Display LIFTING or JACKING requirements (fixes Need info on what level of lifting equipment needed for vehicle modification #16407)
Display installation time
Allow specifying installation and removal requirements in JSON

Coolthulhu · 2016-06-16T10:18:05Z

src/veh_type.cpp

+                part.removal_reqs.qualities = { { { { quality_id( "HAMMER" ), 1, 1 } } } };
+                part.install_reqs.components.push_back( { { { "nail", 20 } } } );
+            } else {
+                part.install_reqs.components.push_back( { { { "duct_tape", 20 } } } );


Will it allow welding?

Yes, but this is specified via the vehicle parts requirement data

See 13ddd1c. Heavy duty frames now require much more welding than standard frames

mugling · 2016-06-16T12:12:13Z

There's a lot more refactoring possible within veh_interact.cpp however that can wait for a further PR.

See updated preamble for screenshot

Coolthulhu · 2016-06-16T12:14:20Z

Could it also fix the regression with wheels not showing their sizes in install menu?
They did show everything before the var size tampering.

mugling · 2016-06-16T12:14:48Z

Preferably #17195 and #17061 first as we can then have much more interesting installation requirements. For example headlights should take a small amount of electronics skill as well as a supply of electrical wire.

Coolthulhu · 2016-06-16T12:25:02Z

But that's a rather significant regression brought by recent changes to the vehicle system.
In general, fixing regressions gets a priority over new features.

mugling · 2016-06-16T12:26:15Z

@Coolthulhu We were both typing at the same time - that isn't the reply to your earlier comment

mugling · 2016-06-16T12:41:26Z

See 360d00a. Can handle set of 4" casters as well as 17" wheel

Cyrano7 · 2016-06-16T20:50:39Z

data/json/vehicleparts/frames.json

+        "skills": [ [ "mechanics", 3 ] ],
+        "time": 240000,
+        "qualities": [ { "id": "GLARE", "level": 2 } ],
+        "tools": [ [ [ "welder", 200 ], [ "welder_crude", 300 ], [ "toolset", 150 ], [ "oxy_torch", 60 ] ] ],


Shouldn't "toolset" be 300?

Coolthulhu · 2016-06-17T13:40:44Z

Conflict with #17208

codemime · 2016-06-17T21:06:55Z

src/game.cpp

@@ -7467,16 +7464,17 @@ void game::exam_vehicle(vehicle &veh, const tripoint &p, int cx, int cy)
        // Stored in activity.index and used in the complete_vehicle() callback to finish task.
        switch (vehint.sel_cmd) {
        case 'i':
-            time = setuptime + std::max(mintime, 5000 * diff - skill * 2500);
+            time = vehint.sel_vpart_info->install_time( g->u );


g-> can (should) be omitted. It's in the game class (x4).

Coolthulhu · 2016-06-22T01:50:59Z

Lack of relevant tools isn't properly checked until the work is finished.

mugling · 2016-06-22T01:53:06Z

Lack of relevant tools isn't properly checked until the work is finished.

Can you expand upon this and also confirm the above test case?

Coolthulhu · 2016-06-22T01:54:00Z

The extra steering axles requirement isn't properly checked when you don't meet the base requirements. For example, you can install extra casters with 0 skill if the vehicle has a steering axle.

Coolthulhu · 2016-06-22T01:57:09Z

Can you expand upon this and also confirm the above test case?

You can start working on grayed out parts.

Not sure what you did differently with your test case, but in general it works like this:

Try to install a part you don't meet some requirements for
Some parts won't be installable
Some parts will be installable despite unmet requirements

mugling · 2016-06-22T01:58:22Z

What is the exact sequence as ~~I still can't reproduce that~~

EDIT: Can reproduce. It only occurs for multiple axles/engines

mugling · 2016-06-22T02:01:46Z

The ok -= conditional idiom looks to be unsound

Coolthulhu · 2016-06-22T02:04:04Z

Minimal example would be:

Start new game
Drop everything
Set skill levels to zero
Spawn a car
Try to install a roof over the engine
Long action starts despite unmet dependencies

The only requirements that seem to stop it are the extra requirements like strength.

Coolthulhu · 2016-06-22T02:07:10Z

The ok -= conditional idiom looks to be unsound

In a language that has implicit casts to and from bool, anything that treats any non-bool as bool or vice versa is generally a bad idea.
There are good reasons for why Java and C# and many other languages didn't inherit the implicit bool cast from C and C++.

mugling · 2016-06-22T02:10:11Z

Agreed, fixed in 88c5409

Coolthulhu · 2016-06-22T02:14:50Z

src/veh_type.cpp

+                                                  { { quality_id( "GLARE" ), 1, 2 } } } };
+                part.install_reqs.tools.push_back( { { { "welder", 50 }, { "welder_crude", 75 }, { "oxy_torch", 10 } } } );
+                part.removal_reqs.qualities = { { { { quality_id( "WRENCH" ), 1, 2 } },
+                                                  { { quality_id( "SAW_M" ), 1, 2 } } } };


I was pretty sure wrench 2 existed, but looks like it doesn't.

Ok, it should though and I'm going to handle that now

Coolthulhu · 2016-06-22T02:32:13Z

Add WRENCH 2 quality

Also the survivor utility belt.

Removal difficulty still needs changing (or arguments that it is an improvement).
In master it is:

2 by default
1 for stuff that isn't welded
Equal to install difficulty for security (which has the difficult removal flag)

mugling · 2016-06-22T02:44:17Z

Fixed removal difficulty

Coolthulhu · 2016-06-22T02:55:00Z

Screw driving 2 doesn't exist. It's fine screw driving.

mugling · 2016-06-22T02:57:19Z

Screw driving 2 doesn't exist. It's fine screw driving.

Done

Coolthulhu · 2016-06-22T02:58:45Z

It's FINE, not F

mugling · 2016-06-22T03:02:46Z

Ok, sorted

BevapDin · 2016-06-23T09:56:19Z

src/veh_interact.cpp

-    bool has_skill = g->u.get_skill_level( skill_mechanics ) >= vpart.difficulty;
-    bool is_wheel = vpart.has_flag("WHEEL");
-    return (has_comps && (has_skill || is_wheel));
+    return g->u.has_trait( "DEBUG_HS" ) || vpart.install_reqs.can_make_with_inventory( crafting_inv );


@mugling The check for the debug trait should probably be in can_make_with_inventory or even deeper in the inventory. At least you have to add this special case to the check in complete_vehicle. Currently I can start installing things with the help of that trait, but when the character finishes it gets the sarcastic "You lack the requirements to install the %s." and the vehicle part is not installed.

Noted. I've patched it to fix the instance @Coolthulhu noticed testing #17302 but I agree we should move the check higher to prevent future regression.

mugling force-pushed the veh16 branch 2 times, most recently from 038a94a to 7509dbb Compare June 16, 2016 10:13

mugling mentioned this pull request Jun 16, 2016

Vehicle installation requirements [WIP] #16941

Closed

Coolthulhu reviewed Jun 16, 2016
View reviewed changes

mugling force-pushed the veh16 branch 2 times, most recently from 03452a6 to 13ddd1c Compare June 16, 2016 11:59

mugling changed the title ~~Vehicle installation requirements (part 2) [WIP]~~ Vehicle installation requirements (part 2) [CR] Jun 16, 2016

Cyrano7 reviewed Jun 16, 2016
View reviewed changes

Coolthulhu self-assigned this Jun 17, 2016

Coolthulhu removed their assignment Jun 17, 2016

codemime reviewed Jun 17, 2016
View reviewed changes

mugling force-pushed the veh16 branch from 360d00a to 51d459a Compare June 19, 2016 08:38

Coolthulhu reviewed Jun 22, 2016
View reviewed changes

Update legacy requirements

c450359

mugling added 2 commits June 22, 2016 03:42

Refactor part list suggestion order

7e6df20

Correctly break for missing requirements

2cf5a94

Merge branch 'master' into veh16

4e8028f

Coolthulhu merged commit e025ca4 into CleverRaven:master Jun 22, 2016

Replace unsafe idiom with conditional

88c5409

Night-Pryanik mentioned this pull request Jun 22, 2016

Possibility to select what vehicle part in particular we want to mount #11706

Closed

mugling added 5 commits June 22, 2016 04:23

Add WRENCH 2 quality

a216c26

Update missed WRENCH 2

d441efc

Update removal difficulty

6ec8ef5

Use fine screwdriving

6c7a189

Try again

cbe52fd

BevapDin reviewed Jun 23, 2016
View reviewed changes

keyspace mentioned this pull request Feb 2, 2017

Vehicle parts requiring 0 or 1 mechanics still require 2 to uninstall. #19863

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Vehicle installation requirements (part 2) [RDY] #17209

Vehicle installation requirements (part 2) [RDY] #17209

mugling commented Jun 16, 2016 •

edited

Loading

Coolthulhu Jun 16, 2016

mugling Jun 16, 2016

mugling Jun 16, 2016

mugling commented Jun 16, 2016

Coolthulhu commented Jun 16, 2016

mugling commented Jun 16, 2016

Coolthulhu commented Jun 16, 2016

mugling commented Jun 16, 2016

mugling commented Jun 16, 2016

Cyrano7 Jun 16, 2016

Coolthulhu commented Jun 17, 2016

codemime Jun 17, 2016

Coolthulhu commented Jun 22, 2016

mugling commented Jun 22, 2016

Coolthulhu commented Jun 22, 2016

Coolthulhu commented Jun 22, 2016

mugling commented Jun 22, 2016 •

edited

Loading

mugling commented Jun 22, 2016

Coolthulhu commented Jun 22, 2016

Coolthulhu commented Jun 22, 2016

mugling commented Jun 22, 2016

Coolthulhu Jun 22, 2016

mugling Jun 22, 2016

mugling Jun 22, 2016

Coolthulhu commented Jun 22, 2016

mugling commented Jun 22, 2016

Coolthulhu commented Jun 22, 2016

mugling commented Jun 22, 2016

Coolthulhu commented Jun 22, 2016

mugling commented Jun 22, 2016

BevapDin Jun 23, 2016

mugling Jun 23, 2016 •

edited

Loading

Vehicle installation requirements (part 2) [RDY] #17209

Vehicle installation requirements (part 2) [RDY] #17209

Conversation

mugling commented Jun 16, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mugling commented Jun 16, 2016

Coolthulhu commented Jun 16, 2016

mugling commented Jun 16, 2016

Coolthulhu commented Jun 16, 2016

mugling commented Jun 16, 2016

mugling commented Jun 16, 2016

Choose a reason for hiding this comment

Coolthulhu commented Jun 17, 2016

Choose a reason for hiding this comment

Coolthulhu commented Jun 22, 2016

mugling commented Jun 22, 2016

Coolthulhu commented Jun 22, 2016

Coolthulhu commented Jun 22, 2016

mugling commented Jun 22, 2016 • edited Loading

mugling commented Jun 22, 2016

Coolthulhu commented Jun 22, 2016

Coolthulhu commented Jun 22, 2016

mugling commented Jun 22, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Coolthulhu commented Jun 22, 2016

mugling commented Jun 22, 2016

Coolthulhu commented Jun 22, 2016

mugling commented Jun 22, 2016

Coolthulhu commented Jun 22, 2016

mugling commented Jun 22, 2016

Choose a reason for hiding this comment

mugling Jun 23, 2016 • edited Loading

Choose a reason for hiding this comment

mugling commented Jun 16, 2016 •

edited

Loading

mugling commented Jun 22, 2016 •

edited

Loading

mugling Jun 23, 2016 •

edited

Loading