Conversation
base.ShouldSkip returning true (pawn ineligible) was short-circuiting only when the RHS was also true. Using && meant ineligible pawns could still receive haul-urgently jobs when the feature was enabled and designations existed. The correct predicate is skip if base says skip OR if there is nothing to do.
The searchPool rotation was fully computed (TakeRandom, RemoveAll) but the return statement ignored it and re-queried all designations, making the rotation dead code. Also materialized thingsOut to a HashSet so RemoveAll is O(n) instead of O(n*m) via lazy IEnumerable re-evaluation.
RemoveAllDesignationsOfDef(Haul) removed every haul designation on the entire map. Replace with TryRemoveDesignationOn(__instance, ...) which removes only the designation on the clicked thing and is a safe no-op if none exists.
defs.Contains() inside a Where predicate re-enumerated the entire deferred IEnumerable chain on every element probe. Materializing to a HashSet (or reusing it if the caller already passed an IReadOnlySet) reduces each lookup to O(1).
All four Designate* methods were passing a deferred Select().Distinct() chain to OfDefs, which would be re-evaluated for every element in the list. Materializing to ToHashSet() at the call site makes intent explicit and ensures the set is computed exactly once per call.
!IsFullyGrown caused the tool to accept immature plants, directly contradicting the CutGrown name and label. Removing the negation makes the designator correctly accept only fully-grown plants.
…erying designations
…nOn instead of RemoveAllDesignationsOfDef
…all sites in Plant_Patches
…Grown check in Designator_CutGrown
…pat) IReadOnlySet<T> is .NET 5+ only and does not exist in .NET Framework 4.8. HashSet<Def> preserves the same cast-or-materialise optimisation.
…stale tests
3-1 FilterUtils.MakeFilter: replace Aggregate nested closures with ToList()+Any(),
eliminating O(n)-deep stack growth on large selections.
3-2 JobDriver_StripFinishOff.DoExecution: move BodiesStripped increment to after
base.DoExecution so the record reflects an actual body, not a living pawn.
3-3 Designator_SelectSimilar: rename `filter` field to `filterIgnoreStuff` to match
the semantics of the stuff-ignoring cache slot (declaration, ref assignment, reset).
3-4 FilterUtilsTest: add missing KeyzAllowUtilities.Tests.csproj and rewrite both
NotFogged tests to call the real extension-method API — the old static
(IntVec3, Map) signature never existed, preventing the project from compiling.
…s, null guards, private readonly
- 4-1: Replace static seenThings HashSet with local var in RenderHighlight
(Designator_HaulUrgently, Designator_FinishOff) — eliminates leak on exception
- 4-2: Delete 3 unused const fields from JobDriver_StripFinishOff and JobDriver_FinishOff
- 4-3: Delete 2 dead Lazy<FieldInfo/MethodInfo> fields from DesignationCategoryDef_Patch
- 4-4: Delete self-referential dead Lazy<FieldInfo> property from Designator_HarvestGrown
- 4-5: Replace magic strings in WorkGiver_FinishOff.JobOnThing with .Translate() calls;
add KAU_NotAPawn and KAU_NotFriendlyOrDowned to English keyed XML
- 4-6: Standardise null guards in Toils_Haul_Patch.PlaceHauledThingInCell and
PlaceCarriedThingInCellFacing to match DepositHauledThingInContainer baseline
- 4-7: FilterUtils._GetMapRect → private static readonly _getMapRect; GetMapRect → private
DesignateAnyOnScreen and DesignateAnyOnMap were called without checkIfHarvestable=false in the non-tree Harvest All action. The default (true) caused the HarvestableNow guard to filter out every immature crop, making the gizmo designate nothing. Pass false explicitly, matching the tree Harvest All Wood variant.
|
Qodana for .NET50 new problems were found
💡 Qodana analysis was run in the pull request mode: only the changed files were checked Detected 4 dependenciesThird-party software listThis page lists the third-party software dependencies used in KeyzAllowUtilities
Contact Qodana teamContact us at qodana-support@jetbrains.com
|
The test project (and main library) previously targeted net48 only. On macOS, dotnet test hangs indefinitely trying to execute the resulting Windows PE32 binary — net48 requires Mono or Windows to run. Fix by adding net8.0 as a second target framework: - KeyzAllowUtilities.csproj: net48;net8.0, game DLL refs gated to net48, net8.0 uses macOS game DLLs (Private=False), net8.0 output redirected to bin/$(Configuration)/net8.0/ so Assemblies/ stays net48-only - KeyzAllowUtilities.Tests.csproj: net48;net8.0, game DLL refs gated to net48, net8.0 copies macOS game DLLs to test output (Private=True) Run tests: dotnet test 1.6/Source/KeyzAllowUtilities.Tests/KeyzAllowUtilities.Tests.csproj --framework net8.0
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Testing some cleanup and refactoring