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

Update current sync methods and delegates #436

Conversation

SokyranTheDragon
Copy link
Member

  • Updated lambda ordinals for FloatMenuMakerMap and MechanitorUtility delegates
  • Changed syncing of carry to shuttle lambda delegate to a local func
  • Removed syncing of RoyalTitlePermitWorker_CallLaborers.CallLaborers, as it's no longer needed since it'll be synced from ITargetingSource.OrderForceTarget
  • Added a sync worker for RoyalTitlePermitWorker_CallLaborers, as using that permit would otherwise desync
  • Added syncing of non-virtual Verb.TryStartCastOn method
    • It's required for ability syncing, as it can be called from Ability.QueueCastingJob (the LocalTargetInfo variant) if verbProps.nonInterruptingSelfCast is true
  • Modified Targeter.BeginTargeting patch to not handle Verb.TryStartCastOn calls
    • This is due to the previous change handling syncing of this specific case
  • Updated SyncDelegates and SyncMethods classes to use C# 12's collection expression
  • Added a sync worker for IReloadableComp, as it's needed by one of the FloatMenuMakerMap delegates
    • We could change it in the future to Write/ReadWithImpl - however, it seems there's a very limited amount of vanilla types implementing this interface

- Updated lambda ordinals for `FloatMenuMakerMap` and `MechanitorUtility` delegates
- Changed syncing of carry to shuttle lambda delegate to a local func
- Removed syncing of `RoyalTitlePermitWorker_CallLaborers.CallLaborers`, as it's no longer needed since it'll be synced from `ITargetingSource.OrderForceTarget`
- Added a sync worker for `RoyalTitlePermitWorker_CallLaborers`, as using that permit would otherwise desync
- Added syncing of non-virtual `Verb.TryStartCastOn` method
  - It's required for ability syncing, as it can be called from `Ability.QueueCastingJob` (the `LocalTargetInfo` variant) if `verbProps.nonInterruptingSelfCast` is true
- Modified `Targeter.BeginTargeting` patch to not handle `Verb.TryStartCastOn` calls
  - This is due to the previous change handling syncing of this specific case
- Updated `SyncDelegates` and `SyncMethods` classes to use C# 12's collection expression
- Added a sync worker for `IReloadableComp`, as it's needed by one of the `FloatMenuMakerMap` delegates
  - We could change it in the future to `Write/ReadWithImpl` - however, it seems there's a very limited amount of vanilla types implementing this interface
@SokyranTheDragon SokyranTheDragon added fix Fixes for a bug or desync. 1.5 Fixes or bugs relating to 1.5 (Not Anomaly). labels Apr 7, 2024
@SokyranTheDragon
Copy link
Member Author

On a side note, do we need to sync all of those?:

const SyncContext mouseKeyContext = SyncContext.QueueOrder_Down | SyncContext.MapMouseCell;
SyncDelegate.Lambda(typeof(FloatMenuMakerMap), nameof(FloatMenuMakerMap.GotoLocationOption), 0).CancelIfAnyFieldNull().SetContext(mouseKeyContext); // Goto
SyncDelegate.Lambda(typeof(FloatMenuMakerMap), nameof(FloatMenuMakerMap.AddHumanlikeOrders), 0).CancelIfAnyFieldNull().SetContext(mouseKeyContext); // Arrest
SyncDelegate.Lambda(typeof(FloatMenuMakerMap), nameof(FloatMenuMakerMap.AddHumanlikeOrders), 6).CancelIfAnyFieldNull().SetContext(mouseKeyContext); // Rescue
SyncDelegate.Lambda(typeof(FloatMenuMakerMap), nameof(FloatMenuMakerMap.AddHumanlikeOrders), 7).CancelIfAnyFieldNull().SetContext(mouseKeyContext); // Capture slave
SyncDelegate.Lambda(typeof(FloatMenuMakerMap), nameof(FloatMenuMakerMap.AddHumanlikeOrders), 8).CancelIfAnyFieldNull().SetContext(mouseKeyContext); // Capture prisoner
SyncDelegate.Lambda(typeof(FloatMenuMakerMap), nameof(FloatMenuMakerMap.AddHumanlikeOrders), 9).CancelIfAnyFieldNull().SetContext(mouseKeyContext); // Carry to cryptosleep casket
SyncDelegate.LocalFunc(typeof(FloatMenuMakerMap), nameof(FloatMenuMakerMap.AddHumanlikeOrders), "CarryToShuttleAct").CancelIfAnyFieldNull().SetContext(mouseKeyContext); // Carry to shuttle
SyncDelegate.Lambda(typeof(FloatMenuMakerMap), nameof(FloatMenuMakerMap.AddHumanlikeOrders), 50).CancelIfAnyFieldNull().SetContext(mouseKeyContext); // Reload
SyncDelegate.Lambda(typeof(FloatMenuMakerMap), nameof(FloatMenuMakerMap.AddDraftedOrders), 3).CancelIfAnyFieldNull().SetContext(mouseKeyContext); // Drafted carry to bed
SyncDelegate.Lambda(typeof(FloatMenuMakerMap), nameof(FloatMenuMakerMap.AddDraftedOrders), 4).CancelIfAnyFieldNull().SetContext(mouseKeyContext); // Drafted carry to bed (arrest)
SyncDelegate.Lambda(typeof(FloatMenuMakerMap), nameof(FloatMenuMakerMap.AddDraftedOrders), 6).CancelIfAnyFieldNull().SetContext(mouseKeyContext); // Drafted carry to transport shuttle
SyncDelegate.Lambda(typeof(FloatMenuMakerMap), nameof(FloatMenuMakerMap.AddDraftedOrders), 7).CancelIfAnyFieldNull().SetContext(mouseKeyContext); // Drafted carry to cryptosleep casket

From my testing and digging into the code:

  • The delegate for Goto option creates FleckDefOf.FeedbackGoto, which I believe our patches to non-self feedback exclude - so it'll likely still need to be synced
    • We could potentially only sync FloatMenuMakerMap.PawnGotoAction
  • Both actions for carrying a pawn to shuttle require syncing, as they do some extra stuff
  • The remainder of the delegates seems to be synced through the calls to Pawn_JobTracker.TryTakeOrderedJob
    • Some of the drafter orders call ForbidUtility.SetForbidden which should get synced, but we may to include those as well to prevent 2 sync calls when 1 would have been enough
  • It doesn't seem like SyncContext.MapMouseCell is doing anything whatsoever here
    • Looking at some old RW code from around and before 1.0 it seems it would have a use, but I don't think it's the case anymore

@SokyranTheDragon
Copy link
Member Author

Also, as I've mentioned in #429, PawnColumnWorker_Designator:SetValue and PawnColumnWorker_Sterilize:SetValue will need some changes to how they're handled.

@Zetrith
Copy link
Member

Zetrith commented Apr 10, 2024

The FloatMenuMakerMap SyncDelegates can stay. TryTakeOrderedJob syncing is quite costly as it exposes the whole job (a few hundred vs just a few bytes without it).

@Zetrith Zetrith merged commit 87ff20a into rwmt:master Apr 10, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.5 Fixes or bugs relating to 1.5 (Not Anomaly). fix Fixes for a bug or desync.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants