Attach/detach mod

[Reaper42]

I can't add json entity to keybindings.json.
Mod add possibility to attach/detach parts with "ATTACH" flag with wrench and wheels with wrench and jack. After removing vehicle part become slot for part.

[narc0tiq]

Shouldn't this be enabled[7] = !cant_do('t')?

[Reaper42]

No, because there is not checks part in it.

[narc0tiq]

@Reaper42 said:

No, because there is not checks part in it.

Huh? I have no idea what you mean.

Look, I know there's a language barrier here, but I really can't tell what you're trying to say. My first guess was that you meant cant_do('t') doesn't have any checks in it, but of course it does. Your diff shows:

@@ -383,6 +386,11 @@ task_reason veh_interact::cant_do (char mode)
...
+    case 't': // attach/detach
+        enough_morale = g->u.morale_level() >= MIN_MORALE_CRAFT;
+        valid_target = cpart >= 0;
+        has_tools = has_wrench || can_remove_wheel;
+        break;

That's something you wrote, so of course you're aware of it. It can't be what you were trying to tell me.

There was a second way I could parse it -- as a claim that setting enabled[7] doesn't make any difference. But it does:

        for (size_t i = 0; i < actions.size(); i++) {
            shortcut_print(w_mode, 0, pos[i] + spacing * i + shift,
                           enabled[i]? c_ltgray : c_dkgray, enabled[i]? c_ltgreen : c_green,
                           actions[i]);
        }

It's technically feasible you'd missed that enabled[i] is used to determine whether it shows as "active" (light gray with light green accelerator key) or "inactive" (dark gray with dark green accelerator key). On the other hand, you were looking at that code fairly recently, and you must've noticed it, so in all honesty I can't imagine that's what you were trying to tell me, either.

So, since I've clearly failed to understand what you meant, would you be willing to try rephrasing?

[kevingranade]

We don't need a second copy of do_install() that works slightly differently, what we need is a set of requirements for installing/removing parts, similar to the crafting and construction systems.

[Reaper42]

We don't need a second copy of do_install() that works slightly differently

Why not? You have change wheel code, it work like do_install() and do_remove() but "slightly differently". Why you can change wheel but can't change battery?

we need is a set of requirements for installing/removing parts, similar to the crafting and construction systems.

First: explain this.
Second: you want me to rewrite all this code? You don't merge more easy things. You merge my code if i do that? I don't think.

[Zireael07]

@kevingranade : "what we need is a set of requirements for installing/removing parts, similar to the crafting and construction systems"

I thought @BevapDin did something like that?

[kevingranade]

@Zireael07 he extracted the requirements thing from crafting.cpp so it can be used more easily, but he didn't go through the vehicle parts and add requirements to them, or update the vehicle construction code to use them, that's what still needs to happen. Essentially, we need a new "construct/remove vehicle part recipe", and the vehicle construction code should use that to handle tool requirements and such.

[BevapDin]

Vehicle modification uses a complete different set of requirements, not compatible with what recipes/constructions use.

For example installing needs:
has_wrench && ((has_welder && has_goggles) || has_duct_tape)
which is actually:

quality(WRENCH) &&
(
    (
        (has_welder && has_welder_charges) ||
        (has_crude_welder && has_crude_welder_charges) ||
        (has_toolset && has_toolset_charges) ||
    ) &&
    has_goggles
) ||
(
    (has_toolbox && has_toolbox_charges) ||
    has_duct_tape_charges
)

Recipes use a structure like this:

(a OR b OR c) AND
(d OR e OR f)

The fact that you need welding goggles, only when using the welder (and not when using the toolbox/duct tape) can not be modeled by this.

@Reaper42 Setting the part-hp to 0 is problematic when the part gets damaged further, it will break from the vehicle and give back items (see vehicle::damage_direct), even through there is actually no part there.

[kevingranade]

For the welding goggle thing in particular, that could be a specific override enforced in the vehicle assembly code. If we use the welder quality, we also need eye protection.
Alternately we could possibly model it as two recipes, one of which uses welder and goggles, the other of which uses duct tape. That would potentially allow setting the HP lower if duct tape is used.
If it's a total blocker for making vehicle construction more sensible, I'd be fine with removing the welding goggle requirement entirely, to be added back if someone finds a clean way to do it. It's a pretty nit-picky thing to stand in the way of an important refactor like this.

[Reaper42]

@BevapDin
It is not problem. Make exception in do_remove/something else if part has flag "slot". But i think it not needed, because slot too can be broken, and slot contains parts too. Usually it broken to metal scarp, i can add exception for slot, but because this pr will be never merged i don't want do it here.