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

Rework some PawnColumnWorker syncing #502

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Commits on Aug 25, 2024

  1. Rework some PawnColumnWorker syncing

    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.
    SokyranTheDragon committed Aug 25, 2024
    Configuration menu
    Copy the full SHA
    9c04a59 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    93fd1de View commit details
    Browse the repository at this point in the history

Commits on Oct 9, 2024

  1. Fixed syncing null Designation/DesignationManager

    I haven't considered how map syncing works when writing those 2 sync worker delegates, and made an error in the way they handle maps.
    
    Specifically, the issue here is that writing a null map may result in a non-null map when reading if another sync worker is syncing the map. Likewise, I believe attempting to sync a null map may have set the map to null for a different sync worker delegate, causing issues there.
    
    The change here is to, rather than sync the map itself (which we may have potentially synced as null and caused issues for other sync workers) we instead sync a bool that determines if the manager is null or has null map (and we then set the MpContext to use its map).
    SokyranTheDragon committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    2b75a74 View commit details
    Browse the repository at this point in the history