Skip to content

Opinionated Cleanup #65

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.

Sign up for GitHub

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

Draft
simplyWiri wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Opinionated Cleanup #65

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fc07f36
Remove old 1.0 code, we don't need this - we bundle the assembly.
simplyWiri Aug 26, 2025
5fce287
Add debug information, creates PDB's - enables the use of debuggers
simplyWiri Aug 26, 2025
8e7544e
Fix hauling behaviours relating to storage buildings
simplyWiri Aug 26, 2025
34cf64d
Correctly bail early when we run out of inventory space on the pawn. …
simplyWiri Aug 26, 2025
6920f09
Make the 'HaulToInventory' class more linearly readable, refactor toi…
simplyWiri Aug 26, 2025
2245566
Remove re-implementation of vanilla, functions - nets a 10x improveme…
simplyWiri Aug 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file modified 1.6/Assemblies/IHoldMultipleThings.dll
View file
Open in desktop
Binary file not shown.
Binary file modified 1.6/Assemblies/PickUpAndHaul.dll
View file
Open in desktop
Binary file not shown.
2 changes: 1 addition & 1 deletion Source/IHoldMultipleThings/IHoldMultipleThings.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<LangVersion>10.0</LangVersion>
<PlatformTarget>AnyCPU</PlatformTarget>
<Optimize>true</Optimize>
<DebugType>none</DebugType>
<DebugType>full</DebugType>
<ErrorReport>prompt</ErrorReport>
<OutputPath>..\..\1.6\Assemblies\</OutputPath>
<AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>
Expand Down
11 changes: 0 additions & 11 deletions Source/IHoldMultipleThingsv1.0/IHoldMultipleThings.cs
View file
Open in desktop

This file was deleted.

53 changes: 0 additions & 53 deletions Source/IHoldMultipleThingsv1.0/IHoldMultipleThingsv10.csproj
View file
Open in desktop

This file was deleted.

36 changes: 0 additions & 36 deletions Source/IHoldMultipleThingsv1.0/Properties/AssemblyInfo.cs
View file
Open in desktop

This file was deleted.

8 changes: 0 additions & 8 deletions Source/PickUpAndHaul.sln
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,6 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "PickUpAndHaul", "PickUpAndH
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "IHoldMultipleThings", "IHoldMultipleThings\IHoldMultipleThings.csproj", "{E1536A54-D289-41FA-9D0B-8A2F6812C7FA}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "PickUpAndHaulv10", "PickUpAndHaulv1.0\PickUpAndHaulv10.csproj", "{2480F13E-E9ED-44EC-A5DE-C821676904E7}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "IHoldMultipleThingsv10", "IHoldMultipleThingsv1.0\IHoldMultipleThingsv10.csproj", "{0F7BA6C3-BF3C-42D5-9268-593B4AF8FC12}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand All @@ -25,10 +21,6 @@ Global
{E1536A54-D289-41FA-9D0B-8A2F6812C7FA}.Debug|Any CPU.Build.0 = Debug|Any CPU
{E1536A54-D289-41FA-9D0B-8A2F6812C7FA}.Release|Any CPU.ActiveCfg = Release|Any CPU
{E1536A54-D289-41FA-9D0B-8A2F6812C7FA}.Release|Any CPU.Build.0 = Release|Any CPU
{2480F13E-E9ED-44EC-A5DE-C821676904E7}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{2480F13E-E9ED-44EC-A5DE-C821676904E7}.Release|Any CPU.ActiveCfg = Release|Any CPU
{0F7BA6C3-BF3C-42D5-9268-593B4AF8FC12}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{0F7BA6C3-BF3C-42D5-9268-593B4AF8FC12}.Release|Any CPU.ActiveCfg = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
Expand Down
87 changes: 60 additions & 27 deletions Source/PickUpAndHaul/JobDriver_HaulToInventory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,17 @@
namespace PickUpAndHaul;
public class JobDriver_HaulToInventory : JobDriver
{
private static WorkGiver_HaulToInventory worker;
public static WorkGiver_HaulToInventory Worker
{
get
{
return worker ??=
DefDatabase<WorkGiverDef>.AllDefsListForReading.First(wg => wg.Worker is WorkGiver_HaulToInventory).Worker
as WorkGiver_HaulToInventory;
}
}

public override bool TryMakePreToilReservations(bool errorOnFailed)
{
Log.Message($"{pawn} starting HaulToInventory job: {job.targetQueueA.ToStringSafeEnumerable()}:{job.countQueue.ToStringSafeEnumerable()}");
Expand All @@ -11,27 +22,54 @@ public override bool TryMakePreToilReservations(bool errorOnFailed)
return pawn.Reserve(job.targetQueueA[0], job) && pawn.Reserve(job.targetB, job);
}

//get next, goto, take, check for more. Branches off to "all over the place"
public override IEnumerable<Toil> MakeNewToils()
{
var takenToInventory = pawn.TryGetComp<CompHauledToInventory>();
var getNextTarget = Toils_JobTransforms.ExtractNextTargetFromQueue(TargetIndex.A);

// do
{
// get next item
yield return getNextTarget;
// bail if we are too heavy
yield return CheckForOverencumberedForCombatExtended();
// walk to the thing
yield return GotoTarget();
// place it in our inventory
yield return PickupTarget();
}
// while
yield return Toils_Jump.JumpIf(getNextTarget, () => !job.targetQueueA.NullOrEmpty());

var wait = Toils_General.Wait(2);
// search for more items around the pawns current position to haul (in case new items spawned while we were
// hauling), and queue up a new haul job if we find one.
yield return SearchMoreHaulables();

var nextTarget = Toils_JobTransforms.ExtractNextTargetFromQueue(TargetIndex.A); //also does count
yield return nextTarget;
// queue up unload inventory job to happen immediately after the current job, ends the job.
yield return EnqueueUnloadJob();
}

yield return CheckForOverencumberedForCombatExtended();
private static List<Thing> TempListForThings { get; } = new();

// Make the pawn path to the next `Thing` pointed to by `TargetA`
public Toil GotoTarget()
{
var gotoThing = new Toil
{
initAction = () => pawn.pather.StartPath(TargetThingA, PathEndMode.ClosestTouch),
defaultCompleteMode = ToilCompleteMode.PatherArrival
};
gotoThing.FailOnDespawnedNullOrForbidden(TargetIndex.A);
yield return gotoThing;

return gotoThing;
}

var takeThing = new Toil
// Make the pawn pick up the `Thing` pointed to by `TargetA`.
// pre: The pawn must be at a cell, where they can touch `TargetA`
public Toil PickupTarget()
{
var inventoryComp = pawn.TryGetComp<CompHauledToInventory>();

return new Toil
{
initAction = () =>
{
Expand All @@ -51,9 +89,9 @@ public override IEnumerable<Toil> MakeNewToils()
if (countToPickUp > 0)
{
var splitThing = thing.SplitOff(countToPickUp);
var shouldMerge = takenToInventory.GetHashSet().Any(x => x.def == thing.def);
var shouldMerge = inventoryComp.GetHashSet().Any(x => x.def == thing.def);
actor.inventory.GetDirectlyHeldThings().TryAdd(splitThing, shouldMerge);
takenToInventory.RegisterHauledItem(splitThing);
inventoryComp.RegisterHauledItem(splitThing);

if (ModCompatibilityCheck.CombatExtendedIsActive)
{
Expand All @@ -69,26 +107,27 @@ public override IEnumerable<Toil> MakeNewToils()
if (haul?.TryMakePreToilReservations(actor, false) ?? false)
{
actor.jobs.jobQueue.EnqueueFirst(haul, JobTag.Misc);
EndJobWith(JobCondition.Succeeded);
}
actor.jobs.curDriver.JumpToToil(wait);
}
}
};
yield return takeThing;
yield return Toils_Jump.JumpIf(nextTarget, () => !job.targetQueueA.NullOrEmpty());
}

//Find more to haul, in case things spawned while this was in progess
yield return new Toil
// Search around the pawns current position for more haulables to haul to our inventory.
public Toil SearchMoreHaulables()
{
return new Toil
{
initAction = () =>
{
var haulables = TempListForThings;
haulables.Clear();
haulables.AddRange(pawn.Map.listerHaulables.ThingsPotentiallyNeedingHauling());
var haulMoreWork = DefDatabase<WorkGiverDef>.AllDefsListForReading.First(wg => wg.Worker is WorkGiver_HaulToInventory).Worker as WorkGiver_HaulToInventory;

Job haulMoreJob = null;
var haulMoreThing = WorkGiver_HaulToInventory.GetClosestAndRemove(pawn.Position, pawn.Map, haulables, PathEndMode.ClosestTouch,
TraverseParms.For(pawn), 12, t => (haulMoreJob = haulMoreWork.JobOnThing(pawn, t)) != null);
TraverseParms.For(pawn), 12, t => (haulMoreJob = Worker.JobOnThing(pawn, t)) != null);

//WorkGiver_HaulToInventory found more work nearby
if (haulMoreThing != null)
Expand All @@ -102,14 +141,11 @@ public override IEnumerable<Toil> MakeNewToils()
}
}
};
}

//maintain cell reservations on the trip back
//TODO: do that when we carry things
//I guess that means TODO: implement carrying the rest of the items in this job instead of falling back on HaulToStorageJob
yield return TargetB.HasThing ? Toils_Goto.GotoThing(TargetIndex.B, PathEndMode.ClosestTouch)
Copy link
Contributor Author

@simplyWiri simplyWiri Aug 26, 2025

Choose a reason for hiding this comment

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

I completely deleted this code, because it didn't make much sense to me. TargetB is not where anything is guaranteed to be dropped off at - the Unload job driver owns that responsibility - maintaining reservations seems moot, if that was a desired feature - wouldn't you do this in the Unload JobDriver?

Copy link
Contributor Author

@simplyWiri simplyWiri Aug 31, 2025

Choose a reason for hiding this comment

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

I think this was load-bearing, I've noticed a few edge cases where pawns are over-hauling when there is not enough space.

Copy link
Contributor Author

@simplyWiri simplyWiri Sep 3, 2025

Choose a reason for hiding this comment

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

Mm. I think this is a more general issue:

When we use a storage building, we store a (StoreTarget, CellAllocation) pair. Where CellAllocation is presumed to be for a particular item. This means we store a int capacity. This capacity is only correct for a particular item, not any item.

This means, for example - if we haul leather, to a cell with 225 capacity (3x free cells) - and we only have, i.e. 10 leather. We would then see 215 free slots.

If we then went to haul a bunch of single-slot items, i.e. apparel. We could attempt to haul ~215 individual apparel items into this slot - when we should only be able to store 2. Because fundamentally we are storing things by cell.

: Toils_Goto.GotoCell(TargetIndex.B, PathEndMode.ClosestTouch);

yield return new Toil //Queue next job
public Toil EnqueueUnloadJob()
{
return new Toil //Queue next job
{
initAction = () =>
{
Expand All @@ -126,11 +162,8 @@ public override IEnumerable<Toil> MakeNewToils()
}
}
};
yield return wait;
}

private static List<Thing> TempListForThings { get; } = new();

/// <summary>
/// the workgiver checks for encumbered, this is purely extra for CE
/// </summary>
Expand Down
2 changes: 1 addition & 1 deletion Source/PickUpAndHaul/PickUpAndHaul.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<LangVersion>10.0</LangVersion>
<PlatformTarget>AnyCPU</PlatformTarget>
<Optimize>true</Optimize>
<DebugType>none</DebugType>
<DebugType>full</DebugType>
<ErrorReport>prompt</ErrorReport>
<OutputPath>..\..\1.6\Assemblies\</OutputPath>
<AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>
Expand Down
Loading