Page MenuHomeWildfire Games

Prevent Out-Of-Sync in TriggerScripts by preventing Out-Of-Scope references
ClosedPublic

Authored by elexis on Mar 6 2017, 6:21 AM.

Details

Summary

When a SP savegame is loaded or MP game rejoined, the Trigger system component is created, the Trigger Script then saves a reference to the Trigger component in cmpTrigger.
After that, the deserialization is done and the Trigger system component is replaced with the deserialized one, while cmpTrigger is not deserialized and thus stil keeps the reference to the old trigger component.
This bf has caused an Out-Of-Sync on survival of the fittest before, was figured out by Itms and fixed by two commits in #4310.

In order to prevent repetition of this mistake, we can put cmpTrigger variables into a local scope, thus the reference is annihilated upon first execution and
the prototype extensions won't be able anymore to reference to cmpTrigger altogether.

Test Plan

Observe that the patch only adds braces and few spaces and that the prototypes had no reference to cmpTrigger variables.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

elexis created this revision.Mar 6 2017, 6:21 AM
Vulcan added a subscriber: Vulcan.Mar 6 2017, 7:04 AM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/474/ for more details.

This would work by just changing var to let if we can get the SM45 upgrade done (currently blocked by having to figure out why something in the gui breaks silently).

See https://bugzilla.mozilla.org/show_bug.cgi?id=589199 and https://blog.mozilla.org/addons/2015/10/14/breaking-changes-let-const-firefox-nightly-44/.

Adding this additional scope seems slightly ugly if we can get away with not adding that and using let, but that doesn't work right now sadly.

Sandarac accepted this revision.May 9 2017, 5:40 AM
Sandarac added a subscriber: Sandarac.

This has already been done for CaptureTheRelic.js in rP19345.

binaries/data/mods/public/maps/scripts/Conquest.js
12 ↗(On Diff #723)

Remove this duplicate newline.

binaries/data/mods/public/maps/scripts/Regicide.js
115 ↗(On Diff #723)

Same here.

binaries/data/mods/public/maps/scripts/WonderVictory.js
86 ↗(On Diff #723)

Same here.

This revision is now accepted and ready to land.May 9 2017, 5:40 AM
This revision was automatically updated to reflect the committed changes.