Source Code Project Mantis - FSSCP
View Issue Details
0002667FSSCPSEXPspublic2012-06-17 15:442012-07-01 21:25
Assigned ToGoober5000 
Platformx64OSWindows 7OS Version
Product Version3.6.13 
Target Version3.6.14Fixed in Version3.6.14 
Summary0002667: Repeating chained events always repeat
DescriptionIn retail FS2 (as well as FS1), repeating chained events don't repeat if the next event of the chain has been triggered. (Note that this does not apply to the first event of a chain, since it is not a chained event.)

However, r8603 (which resolved Mantis 0000082) changed this behavior so that such events always repeat, regardless of the status of the next event of the chain. This change potentially breaks missions that rely on retail behavior.

One minor example of this is found in the first FS1 training mission. At one point in the mission, the player is supposed to match speed with the instructor. If he doesn't do so, the instructor will say "Match my speed now" and return to the first waypoint. However, because of the change made by r8603, this happens even if the player is matching speed.
Steps To ReproduceThe attached mission lets you test this. At 0:02, a message will display. That message should not repeat at 0:15. If it does, that indicates incorrect behavior.
Additional InformationThis bug is also present in 3.6.14 RC6 (can someone please add that to the Product Version list?).
TagsNo tags attached.
related to 0000082resolved Goober5000 Repeat count sexp does not work reliably 
Attached Files? ChainedEventTest.fs2 (3,730) 2012-06-17 15:44
patch chained.patch (480) 2012-06-18 01:31

2012-06-18 01:31   
(Last edited: 2012-06-18 01:32)
It looks like Karajorma's r8603 did a little bit more than fix the bug; it also removed part of the chaining mechanism. The attached patch restores it without reverting the timestamp fix. See if that works.

2012-06-18 01:44   
(Last edited: 2012-06-18 01:46)
That's not a good fix at all.

This needs a mod.tbl setting that means repeating events can actually repeat the way the FREDder requested them to, same as I did when I fixed arguments to loop in the way most FREDders expect them to. Having events not work properly simply because they didn't work properly in retail is a bad idea. We should give the user the decision about whether or not they want to support the old broken code, not foist the choice on them.

Oh and I'm highly suspicious about that patch too. I removed that part of the chaining code for when you have 3 events in a row which are chained.

2012-06-18 02:13   
This patch actually fixes the repeat count the way Volition originally intended it, and I've confirmed it in a test. It turns out that none of us, including the SCP coders, really understood the chaining mechanism until Yarn pointed out the behavior of Training Mission 1.

The chaining mechanism is something like having a "current event pointer" that always allows a *single event* in the event chain to repeat, but no others. If the next event in the chain becomes true, then *that event* will be allowed to repeat, and the first event will quit repeating. If any future events then become true, then they will be allowed to repeat, and so on. In this case the mechanism really is like a "chain", because it joins all the events into one conceptual group.

To see an example of the correct usage of a chained repeat ("correct" in the sense that Volition intended, anyway) take a look at the "Match Speed" event in btm-01 in FS1 or the FSPort. This requires learning about the rare feature of training-msg which allows a secondary message to be played if the event repeats. The event is set to repeat two times, playing both training messages, but it is intended for the second message to *not* play if the player successfully matches speed. This is accomplished by having the repeating event stop repeating if the subsequent event in the chain becomes true.
2012-06-18 02:15   
In fact, and in light of this, we now need to re-evaluate whether it's actually correct to swap "timestamp" with "satisfied_time" in your fix for 0000082. This would depend on a precise understanding of the difference between those two fields.
2012-06-18 02:32   
Ugh. That's a pretty horrible implementation of the way most people will expect chains to work.

Like I said, better to add a mod.tbl setting and let people choose the version they want.
2012-06-19 01:13   
It's an eminently reasonable way for chains to work. Just because it doesn't meet your expectations, or CP5670's expectations, doesn't mean it's "horrible". The problem is not that the code is wrong; the problem is that nobody realized what the intended behavior was supposed to be.

That said, a mod.tbl setting is a reasonable way to control the behavior of chains. But the default will have to be Volition's old code because existing missions rely on that behavior.
2012-06-19 20:57   
That's what I've said from the start.
2012-06-19 23:47   
You certainly do a very good job of arguing the opposite of what you want as the default, then. :p
2012-06-21 10:03   
(Last edited: 2012-06-21 10:06)
Check my first post on the subject. I flat out said we need a mod.tbl setting to allow the V version to be the default.

Anyway, I should take this one over.

2012-06-23 06:31   
Fix committed to trunk@8901.
2012-07-01 21:25   
Fix committed to fs2_open_3_6_14@8959.

Issue History
2012-06-17 15:44YarnNew Issue
2012-06-17 15:44YarnFile Added: ChainedEventTest.fs2
2012-06-18 01:24Goober5000Relationship addedrelated to 0000082
2012-06-18 01:31Goober5000Note Added: 0013671
2012-06-18 01:31Goober5000Assigned To => Goober5000
2012-06-18 01:31Goober5000Statusnew => acknowledged
2012-06-18 01:31Goober5000Target Version => 3.6.14
2012-06-18 01:31Goober5000File Added: chained.patch
2012-06-18 01:32Goober5000Note Edited: 0013671bug_revision_view_page.php?bugnote_id=13671#r150
2012-06-18 01:44karajormaNote Added: 0013672
2012-06-18 01:44karajormaNote Edited: 0013672bug_revision_view_page.php?bugnote_id=13672#r152
2012-06-18 01:46karajormaNote Edited: 0013672bug_revision_view_page.php?bugnote_id=13672#r153
2012-06-18 02:13Goober5000Note Added: 0013673
2012-06-18 02:15Goober5000Note Added: 0013674
2012-06-18 02:32karajormaNote Added: 0013675
2012-06-19 01:13Goober5000Note Added: 0013676
2012-06-19 20:57karajormaNote Added: 0013679
2012-06-19 23:47Goober5000Note Added: 0013681
2012-06-21 10:03karajormaNote Added: 0013691
2012-06-21 10:05karajormaAssigned ToGoober5000 => karajorma
2012-06-21 10:05karajormaStatusacknowledged => assigned
2012-06-21 10:06karajormaNote Edited: 0013691bug_revision_view_page.php?bugnote_id=13691#r157
2012-06-23 06:31Goober5000Changeset attached => fs2open trunk r8901
2012-06-23 06:31Goober5000Note Added: 0013709
2012-06-23 06:31Goober5000Statusassigned => resolved
2012-06-23 06:31Goober5000Resolutionopen => fixed
2012-06-23 06:32Goober5000Assigned Tokarajorma => Goober5000
2012-06-23 06:32Goober5000Fixed in Version => 3.6.14
2012-07-01 21:25ZacamChangeset attached => fs2open fs2_open_3_6_14 r8959
2012-07-01 21:25ZacamNote Added: 0013770