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

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

Closed
Asmageddon opened this issue Dec 27, 2016 · 13 comments
Closed
Labels
<Bug> This needs to be fixed Crafting / Construction / Recipes Includes: Uncrafting / Disassembling Vehicles Vehicles, parts, mechanics & interactions

Comments

@Asmageddon
Copy link
Contributor

Asmageddon commented Dec 27, 2016

The only exception are frames, which appear to require 1, not sure about the other frame types.

I think that this is the primary cause of why training mechanics up to 2 is so agonizing.

@keyspace
Copy link
Contributor

keyspace commented Dec 28, 2016

vpart_info::check() sets the minimum removal skill to 2 for "legacy" parts. These are all parts by default, except for those that set requirements in JSON. (EDIT: Was made so in #17209.)

The best solution IMO is updating vehicle part definitions (see #19407, which is a specific case of this issue, for more info), preferably by making use of copy-from, extends, etc. for less repetition.

@Asmageddon
Copy link
Contributor Author

Asmageddon commented Dec 28, 2016 via email

@keyspace
Copy link
Contributor

keyspace commented Jan 4, 2017

@Asmageddon you have mentioned in #19942 that

I've been in the middle of migrating vehicle parts to the requirements system for the past week. I did the nailable and screwdriver parts

That is good news!

Do consider pushing it to a branch and opening a provisional PR, like

[WIP] Add "requirements" JSON section to vehicle parts that lack it

(Check CONTRIBUTING.md if you haven't already.)


EDIT: style

@keyspace
Copy link
Contributor

keyspace commented Jan 4, 2017

BTW, have you verified that it works so far?

@Asmageddon
Copy link
Contributor Author

Yeah, all I've got works. I'll finish the rest, I promise. I guess may as well try to do it right now even if anxiety murders me.

@Asmageddon
Copy link
Contributor Author

Oh by the way, I'm changing the removal level requirement. Namely:

  • Screwable, nailable, and no-tool parts to min(difficulty, 2), instead of constant 2

  • Bolt and weld to max(difficulty-2, 1) instead of max(difficulty-2, 2) since I'm assuming they require basic knowledge of mechanics for a reason.

I have not done bolt/weld, so if anyone has an opinion to go ahead with this please tell me. I was so down when my big PR got rejected some ~2 years ago.

@keyspace
Copy link
Contributor

keyspace commented Jan 4, 2017

I was so down when my big PR got rejected some ~2 years ago.

(Looked up to get context.)

Exactly why I suggested opening a [WIP] PR.

Note the "IMO" in my comment above:

The best solution IMO is updating vehicle part definitions

There are other possible approaches!

There are over 300 parts currently in the core pack. Many of them don't need direct requirements. Some would benefit more if given a copy-from a-la {small_,medium_,}storage_battery, and just one requirements section for the root part. Perhaps even introduction of an "abstract" root part - IMO wheels could use this...

These are just two that would benefit from being separate PRs if no overwhelming approval is reached beforehand.

@Asmageddon
Copy link
Contributor Author

Yeah but I already did some of this n.n

I'll just ask @kevingranade for permission before proceeding further and otherwise undo the changes I already made.

@Leland Leland added <Bug> This needs to be fixed Crafting / Construction / Recipes Includes: Uncrafting / Disassembling Vehicles Vehicles, parts, mechanics & interactions labels May 2, 2017
@Night-Pryanik
Copy link
Contributor

@Asmageddon what's the status of your work?

@Asmageddon
Copy link
Contributor Author

@Night-Pryanik I'm sorry, I ended up giving up ages ago. It wasn't hard work, just fairly tedious, and anyone could probably do the same thing - it was just figuring out patterns to replace, doing search&replace or manually fixing stuff(I recall numbers, time things take I guess), and doublechecking it. I just struggle with a lot of mental illness and anxiety and motivation issues and never ended up doing it

If you really want, I can try digging up the old files, but they'd probably have merge conflicts, by now.

@Night-Pryanik
Copy link
Contributor

Well, I'd might take a look if you manage to find your files.

@Asmageddon
Copy link
Contributor Author

I'm sorry, I didn't. I think I deleted them when cleaning up data from my old damaged HDD. I have partial notes, but I've deleted ones about the changes I already applied, so... whoever does this would likely have to do it from scratch anyway.

From what I can gather from my notes and a quick look at the game files, I've:

  • Added new templates to data/json/requirements/vehicle.json: vehicle_bolt, sewing_standard, vehicle_nail_install, vehicle_nail_removal, vehicle_screw - some of the above already exist.

  • Created templates such as this for the different component types:

## Bolt template
    "requirements": {
      "install": {
        "skills": [ [ "mechanics", 1 ] ],
        "time": 60000,
        "using": [ [ "vehicle_bolt", 1 ] ]
      },
      "removal": {
        "skills": [ [ "mechanics", 1 ] ],
        "time": 30000,
        "using": [ [ "vehicle_bolt", 1 ] ]
      },
      "repair": {
        "skills": [ [ "mechanics", 1 ] ],
        "time": 60000,
        "using": [ [ "welding_standard", 5 ] ]
      }
    }
## Welder template
    "requirements": {
      "install": {
        "skills": [ [ "mechanics", 1 ] ],
        "time": 120000,
        "using": [ [ "welding_standard", 10 ] ]
      },
      "removal": {
        "skills": [ [ "mechanics", 1 ] ],
        "time": 60000,
        "using": "vehicle_weld_removal"
      },
      "repair": {
        "skills": [ [ "mechanics", 1 ] ],
        "time": 60000,
        "using": [ [ "welding_standard", 5 ] ]
      }
  • Made screwable, nailable, and no-tool parts have removal difficulty of min(difficulty, 2), instead of constant 2, and bolt and weld max(difficulty-2, 1) instead of max(difficulty-2, 2) although I do not recall the rationale behind the latter change.

  • IIRC set the removal time to half the install time, which I think matches most existing values - I think as is, removal always takes 15m which translates to... not sure what number. 30k or 60k.

I'm sorry that I don't have more, it's been so long since I've last done anything that it didn't even occur to me that the CDDA repo I've had was anything but a sad reminder of the times back when I still did some things...

@Night-Pryanik
Copy link
Contributor

Since all vehicle parts are now refactored to use proper requirements instead of difficulty, I'm closing the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bug> This needs to be fixed Crafting / Construction / Recipes Includes: Uncrafting / Disassembling Vehicles Vehicles, parts, mechanics & interactions
Projects
None yet
Development

No branches or pull requests

4 participants