Repository navigation
Conversation
Use matching Harmony 3 and RimWorld 1.7 reference packages. Make shared-reference registration idempotent and track simulation freezes at long-event update boundaries. Explicitly rerun static initializers on CoreCLR so reloads restore debug defaults and resolver caches, preventing empty progress-bar effects from breaking jobs. Add regression coverage for repeated resets.
|
I haven't looked through your PR, but I just noticed that you mention preserving 1.6 compatibility. 1.6 compatibility is unlikely to be needed in the source code. For back-compat we just clone the previous branch (so 1.5 if 1.6 is the main version supported), compile it, and include in the mod release. See |
| } | ||
|
|
||
| public int IncreasePlayerCount() => CurrentPlayerCount += 1; | ||
| public void ResetPlayerCount() => currentPlayerCount = 0; |
There was a problem hiding this comment.
These changes (re. currentPlayerCount) seem independent of the 1.7 upgrade?
There was a problem hiding this comment.
The issue was observed during the 1.7 testing
Two players both viewing map A:
- Its viewer count is 2.
- Multiplayer creates a join point by saving and reconstructing the game.
- Without serialization, the reconstructed map’s viewer count defaults to 0.
- Both players remain on map A, so there is no actual view change to rebuild the count.
The map now has two real viewers but zero recorded viewers. VTR treats it as unviewed and selects the less frequent update rate.
I guess the previous versions don't get any fixes with this approach. With the approach from this PR, every version will get the fix if it is compatible |
| Multiplayer.game.sync.TryAddStackTraceForDesyncLogRaw(logItem, managedDepth, managedHash); | ||
| acc++; | ||
| return; | ||
| } |
There was a problem hiding this comment.
Have you performed any performance tests for this code? It's called frequently and while it could've been implemented this way even in 1.6, it was not fast enough and impacted the game performance.
There was a problem hiding this comment.
Good point. I measured the managed collector and decided to allow user to select a tracing mode in the settings view.
Fixed in cf163a2:
- Fast records lightweight event context without capturing or formatting managed stacks.
- Slow explicitly enables detailed managed stack capture.
- None disables diagnostic event history.
- The selection is now available in Release settings and persists between sessions.
- RNG-based desync detection remains enabled in every mode.
I tested the production collector on native ARM64 with RimWorld 1.7.4990. Five rotated comparisons restored the same one-map, three-colonist save, warmed 60 ticks, then measured 2,000 world-plus-map ticks:
| Mode | Median elapsed time | Median allocated memory |
|---|---|---|
| None | 117 ms | 4.65 MB |
| Fast | 118 ms | 5.17 MB |
| Slow | 223 ms | 202.74 MB |
Fast was comparable to None within the observed timing variability. Slow retains the expensive diagnostics intentionally.
Sure, right now for an old version to get a fix you'd need to send a PR for the relevant branch (like |
Ok, makes sense, but I'd still keep it in the CI. I think it makes testing easier for other people when they can go to PR and download a ZIP with mod that can be tested. |
Having a zip available for download would be nice, but it's not related to 1.7 compatibility and as such, please move those changes to a separate PR where I can review them |
|
|
||
| return builder.ToString(); | ||
| } | ||
| public static string GetJittedMethodsString() => "JIT history is unavailable on CoreCLR."; |
There was a problem hiding this comment.
If it's unavailable, perhaps we could remove it?
| .AppendLine($"Thread Count|||{SystemInfo.processorCount}") | ||
| .AppendLine("\n###GPU Info###") | ||
| .AppendLine($"GPU Family|||{SystemInfo.graphicsDeviceVendor}") | ||
| .AppendLine("GPU Family|||Unavailable") |
There was a problem hiding this comment.
Probably could be removed if it's always going to be unavailable
| using System.Linq; | ||
| using System.Reflection; | ||
| using HarmonyLib; | ||
| using MethodInvoker = HarmonyLib.MethodInvoker; |
There was a problem hiding this comment.
In the future, we could consider using System.Reflection.MethodInvoker instead. Not neccessary for this PR
There was a problem hiding this comment.
While the changes in this file can improve correctness in case of an exception, I'm pretty sure these are unrelated to 1.7 compatibility. Please move to a separate PR
There was a problem hiding this comment.
Why is this needed? I think it's an alternative to a deleted Prepatcher patch
There was a problem hiding this comment.
RimWorld 1.7.4995 rev309 has added "Experimental back-compat support for Prepatcher dependent mods". I'd rather try to use that instead
There was a problem hiding this comment.
Do we need this or is it just a leftover from development?
Summary
BUILD FOR TESTING:
https://github.com/rwmt/Multiplayer/actions/runs/40774181436/artifacts/12854071942
Port Multiplayer to RimWorld 1.7’s .NET 10/CoreCLR runtime while preserving RimWorld 1.6 support.
Thanks to @mkiplm-svg for the initial RimWorld 1.7 port. Their commits provided the foundation for this work.
1. Separate builds by game version
Build 1.6 against .NET Framework 4.8 and Harmony 2, and 1.7 against .NET 10 and Harmony 3. Keep version-specific implementations in separate source folders while sharing common code.
Reason: Prevent incompatible game references, Unity types, and Harmony dependencies from leaking between builds.
2. Adapt runtime-dependent code to CoreCLR
Guard Mono-specific profiler calls, stack inspection, and unsafe method-layout access. Add managed stack traces, update changed Unity APIs, and correct generated synchronization IL. Avoid CoreCLR stack-trace differences producing false desync reports while retaining simulation-state and RNG checks.
Reason: Mono internals cannot safely be used on CoreCLR, and generated patches must produce valid IL for the new runtime.
3. Support ritual synchronization through Harmony 3
Patch concrete ritual widget methods at runtime on 1.7, preserving concrete-type dispatch and serializer registration. Retain the existing Prepatcher implementation for 1.6.
Reason: Support rituals without Prepatcher on 1.7 while preserving the established dependency model for 1.6.
4. Preserve stockpile caches and deterministic simulation hashing
Replace global
System.HashCode.Combinepatches with deterministic replacements at specific game call sites.The global patches changed hashing after RimWorld had populated tuple-keyed caches. This broke the stockpile filter cache, producing the duplicate-key error
(Root, ingredients)when opening the storage tab. Add regression coverage for dictionary lookup and removal after patching.Move temporary filter-context cleanup into Harmony finalizers so drawing exceptions cannot leave stale synchronization state behind.
Preserve pawn-biography hashing because RimWorld 1.7 already enables compatible string hashing.
Reason: Simulation hashes must be deterministic without changing the behavior of existing dictionary keys or leaving filter state behind after exceptions.
5. Restore consistent state across loading, rejoining, and hosting
Reason: Prevent stale defaults, duplicate registrations, incorrect update rates, and clients remaining frozen after failed loading events.
6. Make builds and compatibility checks repeatable
global.json.Reason: Keep build behavior consistent and make compatibility failures reproducible against packaged outputs.
Validation