Sync rename dialogs (sync IRenameable.RenamableLabel setter) #443
Conversation
This file contains 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
Synced all implementations of `IRenameable.RenamableLabel` setter for types that can be synced. I've decided to only apply the sync worker for types that can be synced for safety, as the rename dialogs are of low enough importance that encountering errors or other issues because of them would not be desirable. After all, if rename dialog is not synced then it should not cause any issues besides a minor visual desync (unless there's a mod that does something special with the name, which I've not really encountered). I've wrapped the portion of the code registering it in a double call to `LongEventHandler.ExecuteWhenFinished` to ensure it runs after MP Compat's late patches, ensuring all Sync Workers will be registered by it at this point. Because of no logging when there's `IRenameable` that can't be synced, I've included a debug action which will list all types implementing `IRenameable.RenamableLabel` setter, separated into synced and unsynced ones. This will allow us to easily check if we need to create a sync worker for any of the `IRenameable` implementations.
`Dialog_RenameBuildingStorage_CreateNew:OnRenamed` creates a storage group, which requires syncing on top of renaming. No other vanilla dialog needs syncing dialogs like those, but some mods may require it as well.
This was referenced Apr 21, 2024
SokyranTheDragon
added a commit
to SokyranTheDragon/Multiplayer-Compatibility
that referenced
this pull request
May 23, 2024
This should fix the issues I've found so far, along with a handful of changes/improvements. Vanilla Expanded Framework: - Slightly modified the modular patching - Changed where the patch method is called, so it'll include the try/catch block in late patches as well - Added a message log behind (for debug builds only) that will log what is being patched before applying the patch - Vanilla Furniture Expanded patch was moved to late patch to prevent issues due to the mod accessing DefOfs in the patched methods Vanilla Events Expanded: - Stopped pushing/popping the RNG state where it's not needed - I've originally made those patches when I didn't fully understand how MP and RimWorld work, so I assumed that those would be needed Vanilla Factions Expanded - Empire: - Added a null map check to the quest generation - This is an issue with the mod itself which will happen if `TestRun` method is called for a faction without maps (Vanilla-Expanded/VanillaFactionsExpanded-Empire#8) - After deeper investigation I've realized it always happens in MP due to MP's `FactionRepeater` trying to repeat a quest generation for all factions, including spectator faction (which doesn't have any maps) Vanilla Furniture Expanded - Security: - Stopped pushing/popping the RNG state where it's not needed - Same reason as for Vanilla Events Expanded - Changed patch target from `Draw` to `DrawAt` Vanilla Psycasts Expanded: - Changed the patching to work similar as Vanilla Expanded Framework - made it modular so if a single module fails then it won't break everything after it - Removed an empty method (PatchCurrentMapUsage) - Renamed PatchGizmosAndFlecks to PatchMotesAndFlecks - Slightly reorganized where some of the patches are - Completely reworked psyset renaming code (won't do anything meaningful until mering rwmt/Multiplayer#443) - Added `Dialog_RenamePsysetMp` class, which will handle renaming and allow MP to sync it - Added `PsysetRenameHolder` class, which hold the psyset and psycasting hediff and will be used for syncing psysets - We cannot sync psysets by themselves as they don't store any reference to their parent, and we need a workaround by using the hediff to sync them - Changed psycasting ITab code to create our rename dialog instead of the VPE one, as well as making the code pass the hediff to the constructor
notfood
pushed a commit
to rwmt/Multiplayer-Compatibility
that referenced
this pull request
May 24, 2024
This should fix the issues I've found so far, along with a handful of changes/improvements. Vanilla Expanded Framework: - Slightly modified the modular patching - Changed where the patch method is called, so it'll include the try/catch block in late patches as well - Added a message log behind (for debug builds only) that will log what is being patched before applying the patch - Vanilla Furniture Expanded patch was moved to late patch to prevent issues due to the mod accessing DefOfs in the patched methods Vanilla Events Expanded: - Stopped pushing/popping the RNG state where it's not needed - I've originally made those patches when I didn't fully understand how MP and RimWorld work, so I assumed that those would be needed Vanilla Factions Expanded - Empire: - Added a null map check to the quest generation - This is an issue with the mod itself which will happen if `TestRun` method is called for a faction without maps (Vanilla-Expanded/VanillaFactionsExpanded-Empire#8) - After deeper investigation I've realized it always happens in MP due to MP's `FactionRepeater` trying to repeat a quest generation for all factions, including spectator faction (which doesn't have any maps) Vanilla Furniture Expanded - Security: - Stopped pushing/popping the RNG state where it's not needed - Same reason as for Vanilla Events Expanded - Changed patch target from `Draw` to `DrawAt` Vanilla Psycasts Expanded: - Changed the patching to work similar as Vanilla Expanded Framework - made it modular so if a single module fails then it won't break everything after it - Removed an empty method (PatchCurrentMapUsage) - Renamed PatchGizmosAndFlecks to PatchMotesAndFlecks - Slightly reorganized where some of the patches are - Completely reworked psyset renaming code (won't do anything meaningful until mering rwmt/Multiplayer#443) - Added `Dialog_RenamePsysetMp` class, which will handle renaming and allow MP to sync it - Added `PsysetRenameHolder` class, which hold the psyset and psycasting hediff and will be used for syncing psysets - We cannot sync psysets by themselves as they don't store any reference to their parent, and we need a workaround by using the hediff to sync them - Changed psycasting ITab code to create our rename dialog instead of the VPE one, as well as making the code pass the hediff to the constructor
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Synced all implementations of
IRenameable.RenamableLabelsetter for types that can be synced.I've decided to only apply the sync worker for types that can be synced for safety, as the rename dialogs are of low enough importance that encountering errors or other issues because of them would not be desirable. After all, if rename dialog is not synced then it should not cause any issues besides a minor visual desync (unless there's a mod that does something special with the name, which I've not really encountered).
I've wrapped the portion of the code registering it in a double call to
LongEventHandler.ExecuteWhenFinishedto ensure it runs after MP Compat's late patches, ensuring all Sync Workers will be registered by it at this point.Because of no logging when there's
IRenameablethat can't be synced, I've included a debug action which will list all types implementingIRenameable.RenamableLabelsetter, separated into synced and unsynced ones. This will allow us to easily check if we need to create a sync worker for any of theIRenameableimplementations.On top of that, I've also synced
Dialog_RenameBuildingStorage_CreateNew:OnRenamed. It creates a storage group, which requires syncing on top of renaming. No other vanilla dialog needs syncing dialogs like those, but some mods may require it as well.