Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#4310 closed defect (fixed)

[PATCH] OOS on rejoin - Survival of the fittest

Reported by: elexis Owned by: elexis
Priority: Release Blocker Milestone: Alpha 21
Component: UI & Simulation Keywords: patch
Cc: Patch:

Description

In two games with r18902 a client rejoined and got out of sync right after defeat.

The OOS can be reproduced with the attached replay and equivalent scenario map and the rejointet tool #4242.

Attachments (2)

oos_survival.7z (166.5 KB ) - added by elexis 7 years ago.
survivalofthefittest_triggers.patch (2.0 KB ) - added by Itms 7 years ago.

Download all attachments as: .zip

Change History (9)

by elexis, 7 years ago

Attachment: oos_survival.7z added

comment:1 by elexis, 7 years ago

Priority: Should HaveMust Have

The players are not defeated for the rejoined client!

Reproduce: I can reproduce the issue by rejoining after the first wave spawned, just before it defeats players. Press F9 and type Engine.SetSimRate(20); on all clients to fast forward or just set time in InitializeEnemyWaves to something short in milliseconds and attackerCount to something that doesn't depend on the time.

You have to test with 2 or more players, as one player will have won immediately, thus infinite enemy waves spawn but only standing around.

Interpretation: The OwnershipChange of the CivicCenter is triggered for both players, but TriggerHelper.DefeatPlayer is only executed for the host, not for the rejoined client, which must mean that cmpTrigger.playerCivicCenter differs for the two clients. Since that variable is constructed in InitGame, testing for that revealed that this method isn't executed for the rejoined client at all. Looking at the code, we can easily find the reason: cmpTrigger.DoAfterDelay(0, "InitGame", {}); that means it will init that array on turn 0, everyone who rejoined later gets the empty object. Argh.

comment:2 by elexis, 7 years ago

Correction after some discussions with Itms and more debugging:

The Trigger component does serialize playerCivicCenter. The rejoined client executes survivalofthefittest_triggers.js, then deserializes the components. Thus the DoAfterDelay part is never executed for the rejoined client and playerCivicCenter = {} is not responsible for that variable being empty. This can be verified by patching:

Index: binaries/data/mods/public/simulation/components/Trigger.js
===================================================================
--- binaries/data/mods/public/simulation/components/Trigger.js	(revision 18902)
+++ binaries/data/mods/public/simulation/components/Trigger.js	(working copy)
@@ -32,10 +32,43 @@ Trigger.prototype.Init = function()
 	// Each event has its own set of actions determined by the map maker.
 	for (let eventName of this.eventNames)
 		this["On" + eventName + "Actions"] = {};
 };
 
+Trigger.prototype.Serialize = function()
+{
+	let state = {};
+	for (var key in this)
+	{
+		if (!this.hasOwnProperty(key))
+			continue;
+
+		if (typeof this[key] == "function")
+			continue;
+		//warn("serializing: " + key + " = " + uneval(this[key]));
+		state[key] = this[key];
+	}
+	return state;
+};
+
+Trigger.prototype.Deserialize = function(data)
+{
+	for (let key in data)
+	{
+		if (!data.hasOwnProperty(key))
+			continue;
+
+		if (key == "playerCivicCenter")
+			warn("deserializing: " + key + " = " + uneval(data[key]));
+		this[key] = data[key];
+	}
+};
+
 Trigger.prototype.RegisterTriggerPoint = function(ref, ent)
 {
 	if (!this.triggerPoints[ref])
 		this.triggerPoints[ref] = [];
 	this.triggerPoints[ref].push(ent);

comment:3 by Itms, 7 years ago

Keywords: patch review added
Summary: OOS on rejoin - Survival of the fittest[PATCH] OOS on rejoin - Survival of the fittest

Found the actual issue: the code does not use the correct syntax for accessing the trigger component. Inside the prototype functions, this should be used.

The syntax used at the end of the file should only be used there, because that code is called upon script files loading, not during the game. Using cmpTrigger probably populates an outdated version of the component object, or even goes into the void when doing that on a rejoined client.

comment:4 by elexis, 7 years ago

Owner: set to elexis
Resolution: fixed
Status: newclosed

In 18909:

Fix an OOS on rejoin on survival of the fittest. Patch by Itms, fixes #4310.

The code kept the reference to the trigger component before deserialization,
thus accessed the empty civic center array instead of the deserialized one.

refering to the actual Trigger component instead of

comment:5 by elexis, 7 years ago

In 18912:

Prevent out of scope usage of cmpTrigger, which would result in an OOS on rejoin, refs #4310.

comment:6 by elexis, 7 years ago

Keywords: review removed
Priority: Must HaveRelease Blocker

Thanks for the patch Itms, what a nasty bug again! Also edited wiki:Triggers.

comment:7 by elexis, 7 years ago

In 19552:

Use let keyword for cmpTrigger consistently, so that this reference to the component before deserialization can't be reused inside the Trigger component after deserialization which would cause an OOS on rejoin / savegame loading as experienced prior rP18909.

Differential Revision: https://code.wildfiregames.com/D195
Refs #4310, rP18909, rP18912
Reviewed By: Sandarac

Note: See TracTickets for help on using tickets.