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

Wheel hub assemblies and wheel adjustments #33520

Merged
merged 37 commits into from
Aug 30, 2019

Conversation

Hirmuolio
Copy link
Contributor

@Hirmuolio Hirmuolio commented Aug 24, 2019

Summary

SUMMARY: Balance "Vehicle wheels require vehicle mounts"

Purpose of change

The skill requirements for replacing a wheel is too high. (note: Actual remove and install, not the less known "change tire")

The requirements to install a wheel on new spot is too low.

Repairing wheel with welder makes no sense.

Describe the solution

TL;DR:

  • Wheels are attached to hub assemblies. Hub assemblies require skills and welder.
  • Removing/installing wheel requires no skill. Just the wrench.
  • Wheels are repaired with glue and plastic.
  • Steerable wheels are (mostly) no more. Instead the wheel mount is steerable and a normal wheel is attached.

Now the wheel mount is separate part. the hub assembly.
Doing anything to the hub assembly requires few skill levels. Installing requires welder while removing requires hacksaw. Repairing requires welder.

Wheel mounts come in three variants:

  • Light wheel mount for light wheels like bike/motorcycle wheels. The mount is easily crafted with welder, power drill and two scrap metal pieces.
  • Hub assembly for normal car wheels (17" and 24"). This also has an associated item that you get when you remove it from the car. Uncraftable but can be disassembled for some metal and bearings.
  • Heavy hub assembly for large car wheels (32" armored). Higher skill requirements than the normal one.

The wheel is bolted on the hub assembly. In practice this just means that the wheel part can be installed only if there is already a hub assembly of appropriate size in the spot.

Additionally each wheel mount has steerable variant. It is identical except for the steerable flag on it. Wheels attached on these mounts will act as steerable. (old steerable wheels still work)

You can remove and install wheels on hub assemblies without any skills.

Casters and tricycle wheels don't need mounts.

Wheel repairing has also been adjusted. Wheels are repaired with glue and plastic.
Repairing skill requirements have also been adjusted a bit. Mostly they are similar to before.

  • Light (motorcycle/bike) 2
  • Car wheels (17") 3
  • Wide car wheels (24") 4
  • Armored wheel (32") 5

Wheel install/remove/repair times are now (used to be 1h install, 30m remove 25m repair), :

  • For all wheels except armored Install/remove/repair 15 min
  • Armored wheel has install/repair/remove 20 min

Additional infrastructure changes:

  • Vehicle part repair times can now be given as a string like "20 m".
  • Flag "NEEDS_JACKING" on the part makes it require jacking (used to be hardcoded for wheels).
  • The "change wheel" option has been removed from the vehicle window as it became unnecessary.
  • Saves with wheels without wheel hubs will get wheel hubs added on them automatically.

Describe alternatives you've considered

Additional context

The install/remove/repair values were not defined in the json before this. The code has some legacy values for them that were used.
I was going to put the time/repair adjustments separately but since they were not defined in json I had to add some definition for them in json so I put them here.

You can still use welder to repair wheels in inventory. There is no way to define how items are repaired.

Drum rollers and train wheels are left unchanged.

The save conversion will leave steerable wheels on vehicles from old saves. This will not cause any problems and can be left as is.

Open for value change suggestions.

@kevingranade
Copy link
Member

This pull request has been mentioned on Cataclysm: Dark Days Ahead. There might be relevant details there:

https://discourse.cataclysmdda.org/t/i-need-mechanincs-4-to-swap-a-tire-aka-wheel-rework/21138/8

@ZhilkinSerg ZhilkinSerg added [C++] Changes (can be) made in C++. Previously named `Code` [JSON] Changes (can be) made in JSON Game: Balance Balancing of (existing) in-game features. Vehicles Vehicles, parts, mechanics & interactions labels Aug 24, 2019
src/vehicle.cpp Outdated Show resolved Hide resolved
@Hirmuolio
Copy link
Contributor Author

Getting close to being done. I managed to move steering from steerable wheels to steerable wheel mounts. In the end it was just a small one line edit.

Vehicle efficiency test is a problem since the new parts change the vehicle weights slightly.
The numbers in this test are a mystery to me. They are at the end of vehicle_test.cpp.
The first value is the starting weight of the vehicle. I can fix that.
Rest of numbers are not the same numbers that get used in the test. The numbers that do not match in the test result are not anywhere in the tests.

@kevingranade
Copy link
Member

There's a "test" just above the efficiency test that generates median values for you, whenever we know a test is going to shift these values it's ok to just update them, though we'll want to eyeball the results to make sure they aren't unexpectedly impactful.

@Hirmuolio Hirmuolio marked this pull request as ready for review August 27, 2019 12:47
@Hirmuolio
Copy link
Contributor Author

All checks green and I think everything works.
This pull is ready to be looked at.

"using": [ [ "welding_standard", 10 ] ]
},
"removal": {
"skills": [ [ "mechanics", 4 ] ],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think mechanics 4 is a far too high requirement for a simple sawing off a hub.

},
"removal": {
"skills": [ [ "mechanics", 4 ] ],
"time": "45 m",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too much, IMO. Maybe 15 minutes?

},
"removal": {
"skills": [ [ "mechanics", 5 ] ],
"time": "60 m",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same for skills and time requirements.

data/json/vehicleparts/wheel.json Outdated Show resolved Hide resolved
data/json/vehicleparts/wheel.json Outdated Show resolved Hide resolved
@Hirmuolio
Copy link
Contributor Author

Hirmuolio commented Aug 28, 2019

Moved the metal wheel to obsolete (it is not used anywhere in json, mods or c++).
Reduced remove/repair durations and skill requirements.

  • Mechanics 1 to remove light hub.
  • mechanics 3 to remove medium/heavy hub.
  • 15/20 min to remove medium/heavy hub.
  • Mechanics 2 to repair medium/heavy hubs.
  • Added mechanics 0 as skill requirement for wheels so you get some xp from changing tires.

@Night-Pryanik
Copy link
Contributor

Added mechanics 0 as skill requirement for wheels so you get some xp from changing tires.

изображение

@Hirmuolio
Copy link
Contributor Author

That failed test is not related to this pull I think.

@kevingranade kevingranade merged commit 8263694 into CleverRaven:master Aug 30, 2019
@ZhilkinSerg
Copy link
Contributor

When loading one of savegames:

 DEBUG    : init_vehicles: 'Animal Control Truck' part 'wheel_wide_steerable'(43) can't be installed to 2,-1
 
 FUNCTION : static void vehicle_prototype::finalize()
 FILE     : src/veh_type.cpp
 LINE     : 1081

@Hirmuolio Hirmuolio deleted the Hirmuolio-wheel-rework branch September 2, 2019 08:02
misterprimus pushed a commit to misterprimus/Cataclysm-DDA that referenced this pull request Sep 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` Game: Balance Balancing of (existing) in-game features. [JSON] Changes (can be) made in JSON Vehicles Vehicles, parts, mechanics & interactions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants