2018-03-23 15:38 EDT

View Issue Details Jump to Notes ] Related Changesets ]
IDProjectCategoryView StatusLast Update
0002667FSSCPSEXPspublic2012-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.
Attached Files
  • ? file icon ChainedEventTest.fs2 (3,730 bytes) 2012-06-17 15:44
  • patch file icon chained.patch (480 bytes) 2012-06-18 01:31 -
    Index: code/mission/missiongoals.cpp
    --- code/mission/missiongoals.cpp	(revision 8898)
    +++ code/mission/missiongoals.cpp	(working copy)
    @@ -896,6 +896,10 @@
     				sindex = -1;  // bypass evaluation
    +		if ((event < Num_mission_events - 1) && Mission_events[event + 1].result && (Mission_events[event + 1].chain_delay >= 0)){
    +			sindex = -1;  // bypass evaluation
    +		}
     	if (sindex >= 0) {
    patch file icon chained.patch (480 bytes) 2012-06-18 01:31 +

related to 0000082resolvedGoober5000 Repeat count sexp does not work reliably 



Goober5000 (administrator)

Last edited: 2012-06-18 01:32

View 2 revisions

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.


karajorma (administrator)

Last edited: 2012-06-18 01:46

View 3 revisions

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.


Goober5000 (administrator)

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.


Goober5000 (administrator)

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.


karajorma (administrator)

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.


Goober5000 (administrator)

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.


karajorma (administrator)

That's what I've said from the start.


Goober5000 (administrator)

You certainly do a very good job of arguing the opposite of what you want as the default, then. :p


karajorma (administrator)

Last edited: 2012-06-21 10:06

View 2 revisions

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.


Goober5000 (administrator)

Fix committed to trunk@8901.


Zacam (administrator)

Fix committed to fs2_open_3_6_14@8959.

+Related Changesets

-Issue History
Date Modified Username Field Change
2012-06-17 15:44 Yarn New Issue
2012-06-17 15:44 Yarn File Added: ChainedEventTest.fs2
2012-06-18 01:24 Goober5000 Relationship added related to 0000082
2012-06-18 01:31 Goober5000 Note Added: 0013671
2012-06-18 01:31 Goober5000 Assigned To => Goober5000
2012-06-18 01:31 Goober5000 Status new => acknowledged
2012-06-18 01:31 Goober5000 Target Version => 3.6.14
2012-06-18 01:31 Goober5000 File Added: chained.patch
2012-06-18 01:32 Goober5000 Note Edited: 0013671 View Revisions
2012-06-18 01:44 karajorma Note Added: 0013672
2012-06-18 01:44 karajorma Note Edited: 0013672 View Revisions
2012-06-18 01:46 karajorma Note Edited: 0013672 View Revisions
2012-06-18 02:13 Goober5000 Note Added: 0013673
2012-06-18 02:15 Goober5000 Note Added: 0013674
2012-06-18 02:32 karajorma Note Added: 0013675
2012-06-19 01:13 Goober5000 Note Added: 0013676
2012-06-19 20:57 karajorma Note Added: 0013679
2012-06-19 23:47 Goober5000 Note Added: 0013681
2012-06-21 10:03 karajorma Note Added: 0013691
2012-06-21 10:05 karajorma Assigned To Goober5000 => karajorma
2012-06-21 10:05 karajorma Status acknowledged => assigned
2012-06-21 10:06 karajorma Note Edited: 0013691 View Revisions
2012-06-23 06:31 Goober5000 Changeset attached => fs2open trunk r8901
2012-06-23 06:31 Goober5000 Note Added: 0013709
2012-06-23 06:31 Goober5000 Status assigned => resolved
2012-06-23 06:31 Goober5000 Resolution open => fixed
2012-06-23 06:32 Goober5000 Assigned To karajorma => Goober5000
2012-06-23 06:32 Goober5000 Fixed in Version => 3.6.14
2012-07-01 21:25 Zacam Changeset attached => fs2open fs2_open_3_6_14 r8959
2012-07-01 21:25 Zacam Note Added: 0013770
+Issue History