Skip to content

Commit

Permalink
Rework some PawnColumnWorker syncing
Browse files Browse the repository at this point in the history
This change is primarily to fix `PawnColumnWorker_Designator` and `PawnColumnWorker_Sterilize` displaying confirmation dialogs, which caused issues as pressing the checkbox would open the dialog for all players and the dialogs themselves weren't synced. I've mentioned this issue in rwmt#429.

The first change is to stop syncing `PawnColumnWorker_Designator.SetValue` and `PawnColumnWorker_Sterilize.SetValue`, as those could display confirmation dialogs. Instead of those 2 methods, we instead sync:
- `DesignationManager.AddDesignation`
  - This one is not needed, but was included for consistency (and may come in handy for mod compat, it can be safely removed if desired)
- `DesignationManager.RemoveDesignation`
  - Called from `PawnColumnWorker_Designator.SetValue` when value is false
- `PawnColumnWorker_Designator.DesignationConfirmed`
  - This method calls `DesignationManager.AddDesignation` (along with another method), which is why that specific method is not needed
- `PawnColumnWorker_Sterilize.AddSterilizeOperation`
- `PawnColumnWorker_Sterilize.CancelSterilizeOperations`

This required adding extra sync worker delegates for `Designation` and `DesignationManager`.

By not syncing the `SetValue` method, it allows for a potential multiple calls to the other synced methods which generally don't have checks if the state already matches. This requires additional patches that cancel execution if it would cause issues (`PreventPawnTableDesignationErrors`, `PreventPawnTableMultipleSterilizeOperations`).

Finally, by not syncing the `SetValue` methods we don't call `SetDirty` on the pawn tables. To fix this I've added a method (`TryDirtyCurrentPawnTable`) which is called in post invoke for the synced methods, as well as after syncing designators, to cause the tables to re-sort their values. This will cause the tables to be re-sorted in a few extra situations (like when a different player modifies designators outside of pawn tables). It may be expanded to include more methods to cause the tables to be re-sorted when they normally wouldn't be in vanilla (if we so desire). Alternatively, this could be reduced or removed if we don't want it.
  • Loading branch information
SokyranTheDragon committed Aug 25, 2024
1 parent f0221d0 commit 9c04a59
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 4 deletions.
2 changes: 2 additions & 0 deletions Source/Client/AsyncTime/AsyncTimeComp.cs
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,8 @@ void RestoreState()
designator.DesignateThing(thing);
designator.Finalize(true);
}

SyncMethods.TryDirtyCurrentPawnTable(designator);
}
finally
{
Expand Down
42 changes: 42 additions & 0 deletions Source/Client/Syncing/Dict/SyncDictRimWorld.cs
Original file line number Diff line number Diff line change
Expand Up @@ -666,6 +666,48 @@ public static class SyncDictRimWorld
}
}
},
{
(SyncWorker sync, ref DesignationManager manager) =>
{
if (sync.isWriting)
sync.Write(manager?.map);
else
manager = sync.Read<Map>()?.designationManager;
}
},
{
(SyncWorker sync, ref Designation designation) =>
{
if (sync.isWriting)
{
sync.Write(designation?.designationManager);
if (designation?.designationManager != null)
{
sync.Write(designation.target);
sync.Write(designation.def);
}
}
else
{
var manager = sync.Read<DesignationManager>();
if (manager != null)
{
var target = sync.Read<LocalTargetInfo>();
var def = sync.Read<DesignationDef>();
// If the target has Thing, read designation by def for it.
if (target.HasThing)
designation = manager.DesignationOn(target.Thing, def);
// If the target doesn't have a Thing then it must have a cell,
// get the designation by def for that specific cell.
else
designation = manager.DesignationAt(target.Cell, def);
}
}
}, true // implicit
},
#endregion

#region ThingComps
Expand Down
55 changes: 51 additions & 4 deletions Source/Client/Syncing/Game/SyncMethods.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public static void Init()
SyncMethod.Register(typeof(Pawn_GuestTracker), nameof(Pawn_GuestTracker.ToggleNonExclusiveInteraction)).CancelIfAnyArgNull();
SyncMethod.Register(typeof(Zone), nameof(Zone.Delete));
SyncMethod.Register(typeof(BillStack), nameof(BillStack.AddBill)).ExposeParameter(0); // Only used for pasting
SyncMethod.Register(typeof(BillStack), nameof(BillStack.Delete)).CancelIfAnyArgNull();
SyncMethod.Register(typeof(BillStack), nameof(BillStack.Delete)).CancelIfAnyArgNull().SetPostInvoke(TryDirtyCurrentPawnTable);
SyncMethod.Register(typeof(BillStack), nameof(BillStack.Reorder)).CancelIfAnyArgNull();
SyncMethod.Register(typeof(Bill_Production), nameof(Bill_Production.SetStoreMode));
SyncMethod.Register(typeof(Bill_Production), nameof(Bill_Production.SetIncludeGroup));
Expand Down Expand Up @@ -89,10 +89,13 @@ public static void Init()
}
}

SyncMethod.Register(typeof(PawnColumnWorker_Designator), nameof(PawnColumnWorker_Designator.SetValue)).CancelIfAnyArgNull(); // Virtual but currently not overriden by any subclasses
SyncMethod.Register(typeof(DesignationManager), nameof(DesignationManager.AddDesignation)).ExposeParameter(0).SetPostInvoke(TryDirtyCurrentPawnTable); // Added designation will be freshly constructed, so we need to expose it rather than sync it
SyncMethod.Register(typeof(DesignationManager), nameof(DesignationManager.RemoveDesignation)).CancelIfAnyArgNull().SetPostInvoke(TryDirtyCurrentPawnTable);
SyncMethod.Register(typeof(PawnColumnWorker_Designator), nameof(PawnColumnWorker_Designator.DesignationConfirmed)).CancelIfAnyArgNull().SetPostInvoke(TryDirtyCurrentPawnTable); // Called from SetValue and confirmation dialog
SyncMethod.Register(typeof(PawnColumnWorker_FollowDrafted), nameof(PawnColumnWorker_FollowDrafted.SetValue)).CancelIfAnyArgNull();
SyncMethod.Register(typeof(PawnColumnWorker_FollowFieldwork), nameof(PawnColumnWorker_FollowFieldwork.SetValue)).CancelIfAnyArgNull();
SyncMethod.Register(typeof(PawnColumnWorker_Sterilize), nameof(PawnColumnWorker_Sterilize.SetValue)).CancelIfAnyArgNull(); // Will sync even without this, but this will set the column to dirty
SyncMethod.Register(typeof(PawnColumnWorker_Sterilize), nameof(PawnColumnWorker_Sterilize.AddSterilizeOperation)).CancelIfAnyArgNull().SetPostInvoke(TryDirtyCurrentPawnTable);
SyncMethod.Register(typeof(PawnColumnWorker_Sterilize), nameof(PawnColumnWorker_Sterilize.CancelSterilizeOperations)).CancelIfAnyArgNull().SetPostInvoke(TryDirtyCurrentPawnTable);
SyncMethod.Register(typeof(CompGatherSpot), nameof(CompGatherSpot.Active));

SyncMethod.Register(typeof(Building_Grave), nameof(Building_Grave.EjectContents));
Expand Down Expand Up @@ -367,7 +370,7 @@ public static void Init()
// Previously we synced the delegate which created the bill, but it has side effects to it.
// It can display confirmation like royal implant (no longer used?) or implanting IUD (if it would terminate pregnancy).
// On top of that, in case of implanting the Xenogerm recipe, it will open a dialog with list of available options.
SyncMethod.Register(typeof(HealthCardUtility), nameof(HealthCardUtility.CreateSurgeryBill));
SyncMethod.Register(typeof(HealthCardUtility), nameof(HealthCardUtility.CreateSurgeryBill)).SetPostInvoke(TryDirtyCurrentPawnTable);

// Comp explosive
SyncMethod.Register(typeof(CompExplosive), nameof(CompExplosive.StartWick)); // Called from Building_BlastingCharge (and some modded) gizmos
Expand Down Expand Up @@ -706,6 +709,50 @@ static void SyncTargeterInterruptingSelfCast(Verb verb, Pawn casterPawn)
job.verbToUse = verb;
casterPawn.jobs.StartJob(job, JobCondition.InterruptForced);
}

// By no longer syncing "SetValue" method for pawn column workers, the
// tables aren't made dirty when a value changes. This will ensure that
// the table is made dirty when its state changes, so it can be properly
// sorted. It'll also allow us to dirty the table when it normally wouldn't
// be in vanilla, causing the table to be re-sorted when it wouldn't be in
// vanilla. For example, we can cause the table to be re-sorted when something
// is designated outside the pawn table.
public static void TryDirtyCurrentPawnTable(object instance = null, object[] args = null)
{
// Make sure there's a currently open tab whose window has pawn table,
// and ensure the table is not null and one of the columns is sorted.
// Otherwise, there would be no point in making the table dirty,
// as it wouldn't change anything about the table.
if (Find.MainTabsRoot.OpenTab?.TabWindow is MainTabWindow_PawnTable { table: { SortingBy: not null } table })
{
// If the method was called on Designator or DesignationManager,
// set the DesignationDef from them as the instance. Instance
// is never Designation, so no point in getting def from it.
if (instance is Designator designator)
instance = designator.Designation;
else if (instance is DesignationManager && args is { Length: > 0 } && args[0] is Designation designation)
instance = designation.def;

// If the synced object is PawnColumnWorker, then we can dirty the current
// table based on if it's being sorted by the specific PawnColumnDef.
if (instance is PawnColumnWorker worker)
{
if (table.SortingBy == worker.def)
table.SetDirty();
}
// If we can get DesignationDef, we can check if the sorted column worker is
// designator worker and is the specific designation that
else if (instance is DesignationDef designation)
{
if (table.SortingBy.Worker is PawnColumnWorker_Designator des && des.DesignationType == designation)
table.SetDirty();
}
// If it's a different type, then we always dirty as we don't
// know if making the table dirty is required or not.
else
table.SetDirty();
}
}
}

}

0 comments on commit 9c04a59

Please sign in to comment.