LecrisUT / Rimworld-SurvivalTools-old

Survival Tools mod for RimWorld
MIT License
0 stars 2 forks source link

Fix Hammer needed for Hauling #4

Closed LecrisUT closed 4 years ago

LecrisUT commented 4 years ago

While digging into the code I found: https://github.com/Jellypowered/SurvivalTools/blob/fbd81a25528da30b2ebe71638bc29a71faa60af8/Source/SurvivalTools/SurvivalToolUtility.cs#L230-L239

As far as I understand it checks if using the tool actually improves performance: in the case of supply jobs, it does not. Need to check why it does not do so

LecrisUT commented 4 years ago

Tracked it down to xpath patches. E.g.: https://github.com/Jellypowered/SurvivalTools/blob/fbd81a25528da30b2ebe71638bc29a71faa60af8/1.1/Patches/Core/WorkGiverDefs/WorkGivers.xml#L17-L26

It patches jobs and their children. Hmm how to change the patching?

LecrisUT commented 4 years ago

I'm thinking of calculating whether the tool really changes the work speed of the WorkGiver. But I haven't tracked down how SurvivalTools changes the work speed itself. Can you point to the part of code that does so?

LecrisUT commented 4 years ago

Ok, here is what I've got. It requires a lot of work to decode the rest but I think I have a way to make it more safe: Current method:

  1. Declare in XML what WorkDef/WorkType have changed stats
  2. If Work is assigned than check if there are equipped tools

This creates a problem when a Haul WorkDef is a child of Construction WorkDef.

New method:

  1. Look in the list of WorkGiver
  2. Check if JobDef or JobDefDriver is in the list of Jobs that have been patched
  3. If yes check what stat was patched and add WorkGiver to list of jobs requiring tool

Why go this way? The JobDefDriver have to be patched in order to degrade the tools and add harvest yield or workspeed. Don't know about the latter. Naturally Haul jobs are not patched so it would not be affected.

What I have also learned while digging:

Problem: There are at least 2 other mods which mess with this: Fertile fields and Harvest yield patch. So the redefinition is basically nullified.

Jellypowered commented 4 years ago

I like your idea of rewriting how the checks are handled. In the future we should keep the possibility of adding other jobs not otherwise defined in the base, (Say if we wanted a "wheelbarrow" or "hand truck" for boosting carry capacity/hauling efficiency.)

LecrisUT commented 4 years ago

Ok. I'll update when I got something.

Tools for haulers will be tricky.

  1. Other jobs technically have haul jobs inside them. E.g. supply job in construction work type. So we might have the same problem from another angle. However we can hard code an exception for hauling.
  2. Hauling does not have a workspeed to easily search for. Pick up and haul would be necessary to add carry capacity. The implementation will need a lot of work, but I think it's doable if we think hard enough.

We'll revisit this after I've implemented the job searching method. @Jellypowered, do you know of any mod that patches in WorkGivers, preferably as a mod dependency?

LecrisUT commented 4 years ago

@Jellypowered. I have found a problem with the automatic patching method.

Right now the method can be like:

  1. Search the jobdriver database for stats: e.g. constructionSpeed. Save a list of drivers.
  2. Patch the drivers to use tools.
  3. Search the workgivers -> used jobdefs -> used jobdrivers. Require tools for those jobs.

Problems:

What we could do is:

Which method do you prefer?

Jellypowered commented 4 years ago

What would be less work? If you keep the current manual patching but implement an exception for construction work givers, you'd have to add more checks for building.

An exception list for job driver/status seems like a lot more code.

I'd probably lean more towards adding an exception to the construction workgivers?

LecrisUT commented 4 years ago

In the long run the automatic method is less work. I'm at the final stage of implementing work givers.

Right now I have it xml exposed so anyone can change it later. Will need some feedback on the naming.

I think there is another bug related to the manual patching were builders do nothing. Hopefully this also fixes it

LecrisUT commented 4 years ago

Update: The implementation of automatic patching works almost as intended. Adding jobs that are not affected by the workspeed are not counted towards the workgivers so will not require tools.

I still need to test if the patches are actually working as intended, clean up the code and will add a pull request. Good news is that it should work with any modded jobs now without having to create custom patches anymore (mostly).