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

Fix desyncs due to CompCableConnection motes, fixes #447 #481

Merged
merged 1 commit into from
Jul 14, 2024

Conversation

SokyranTheDragon
Copy link
Member

The method calls random whenever the mote is null or destroyed. There may be situations where the mote ends up being created late or destroyed for some players, but not the others. This is guaranteed to happen when playing with multiple maps, and seems like it may happen when the game is not paused when someone is joining.

Since the desync happens due to RNG call to randomize the mote position, this change should fix it by pushing/popping the RNG state before/after the method is called.

The method calls random whenever the mote is null or destroyed. There may be situations where the mote ends up being created late or destroyed for some players, but not the others. This is guaranteed to happen when playing with multiple maps, and seems like it may happen when the game is not paused when someone is joining.

Since the desync happens due to RNG call to randomize the mote position, this change should fix it by pushing/popping the RNG state before/after the method is called.
@SokyranTheDragon SokyranTheDragon added fix Fixes for a bug or desync. anomaly Fix or bugs relating to Anomaly (Not 1.5) labels Jul 6, 2024
@Zetrith Zetrith merged commit cd7504e into rwmt:master Jul 14, 2024
1 check passed
SokyranTheDragon added a commit to SokyranTheDragon/Multiplayer that referenced this pull request Jul 29, 2024
This should fix rwmt#447, rendering rwmt#481 unnecessary. I've decided to still leave that code as an extra safety precaution, but it should be now safe to revert those changes.

First, FixNullMotes patch was moved from generic "Patches" to "Determinism", as the patch itself changes some non-deterministic behavior.

As fot the changes, this should fix issues with Bioferrite Harvester and Electroharvester cables not creating motes after switching maps.

The current `FixNullMotes` patch would create a single mote per mote's `ThingDef.thingClass` and then reuse it. It ensured that the call to `MoteMaker.MakeStaticMote` never produced null, however the mote itself was more of a dummy mote (not initialized with any data).

This caused issue with the cables that would periodically attempt to spawn motes. If the mote would not be possible to spawn (drawn off-screen/on a different map), the MP patch would return the cached mote. The cable connection comp would then keep that mote forever, as it can only discard it if the mote is null or destroyed (which would never happen with the dummy note). Because of this, our dummy mote never ended up being discarded by the comp making the cable never draw another mote.

This PR ensures this doesn't happen by reworking "FixNullMotes" to actually spawn the motes, rather than returning a dummy mote. This is done by:
- Setting `makeOffscreen` to true in a prefix, skipping current map check
- Adding `& false` to `MoteCounter.Saturated` calls, allowing to spawn motes even if the mote counter is saturated
The check for off-screen motes was left as-is, as that one should be deterministic and safe.

Additional change call `Initialize` for both `Building_BioferriteHarvester` and `Building_Electroharvester`. Without it, the cables for those 2 aren't created after loading into the game (they're only created when rendered or a connection is added/removed). Since the cables aren't created the motes can't be created either, causing a discrepancy between players (unless they all had the cables created at the same time).

A final note on the original patch - it seems that it was made to fix desyncs caused by motes spawned by `CompAbilityEffect_Waterskip` (and potentially other issues at the time, but I can't find any mention of those). However, this specific issue seems to no longer be relevant as the waterskip now uses flecks rather than motes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
anomaly Fix or bugs relating to Anomaly (Not 1.5) fix Fixes for a bug or desync.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants