View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0000617 | FSSCP | gameplay | public | 2005-11-13 19:05 | 2012-01-09 18:06 |
| Reporter | starman01 | Assigned To | taylor | ||
| Priority | normal | Severity | text | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Summary | 0000617: Directive text isn't displayed ingame | ||||
| Description | Here is the problem : When you create a directive-SEXP (the one with the red blip before the event) AND use a "is-event-true" trigger in it, the directive will not show up (in the white letters) ingame in the directive box. BUT it is displayed (in blue or whatever hud-colour you use) once the directive is true or false. I guess this is only a minor problem for you guys :) | ||||
| Tags | No tags attached. | ||||
|
|
I'm having some trouble with directive text showing too soon on the fourth training mission as well. It gives you the C-2-1-1 directive right when it starts. |
|
|
Who created this SEXP? |
|
|
It's a retail sexp. It's probably a bug in the event system. IIRC taylor said a few days ago that he had a lead on it. |
|
|
Has this been fixed? |
|
|
*cough *bump* cough* Any progress in this matter ? I'm still too dump to use variables to fix my problem, and with this bug solved I could received the wished effect in a very simple SEXP. :-) edited on: 01-27-06 12:22 |
|
|
Is the same behavior displayed in retail? |
|
|
I'm not fully sure, but I think it is that way. |
|
|
Taylor, weren't you working on this a few months back? |
|
|
I recently noticed this too in one of my missions. As far as I know, it didn't occur in retail FS2. |
|
|
I don't think I was working on this particular thing, but something similar. Someone will have to get me a test mission so I can work on this one though. |
|
|
Think I've got this one nailed, expect a test build over the weekend when I've had time to test it properly for backwards compatibility. |
|
|
* BUMP * Any update on the progress of this fix? |
|
|
Still working on it. Trickier than I thought. I know how to make it display properly now at least, I just need to make it display without breaking anything else. |
|
|
I don't like the sound of that. Maybe we should start pulling old builds and find out when this bug was introduced. |
|
|
Yeah. Reason I haven't posted anything for committing yet, not safe just using a hack without knowing the underlying reason. Reason it's complicated to fix properly is that the sexp itself (OP_EVENT_TRUE_DELAY) is not at fault, as it is identical to the retail code where this does not happen. Somewhere in the event system a change has been made that prevents this identical code from correctly identifying an event as being current, and that's what I'm trying to track down. [Edit] PS, about committing: Anonymous account obviously won't let me. Should I drop you an email about it so I can get my earlier fixes in? (features I was working on are on hold until I get some of these bugs worked out) edited on: 06-04-06 14:51 |
|
|
* BUMP * Any further update on this? It would be nice to have this fixed for 3.6.9. If Shade can provide more info on what he has found then I'll help track it down with him. Otherwise I really need a test mission that I can reliably replicate this with and then I'll take a look at it. |
|
|
(<-- Shade, problem with PW on old account after move to scp site. Thankfully it still sends me emails so I saw you posted) It has stumped me so far, and unfortunately I haven't had time for coding (or anything, for that matter, but sleep and work) recently so I doubt I'll be able to change that before 3.6.9. What I can tell you is that it is not the actual SEXP that is at fault - I've compared both OP_EVENT_TRUE/FALSE_DELAY and sexp_event_delay_status with retail, line by line, and they are identical. I tried fiddling with the evaluation for setting sexp_useful_number to 0 in the OP part, thinking that how this was handled might have changed elsewhere, but it simply changes the problem rather than solving it (directive not showing up until completed/failed rather than showing up too early. That was expected but I was grasping for straws). And that's about where I got stuck, as when I tried to figure out exactly how the whole sexp_useful_number thing figures into the greater whole my head kinda exploded. It doesn't seem to be used for anything but this sexp though. [Edit] One thing of possible note is that I actually managed to recreate this bug in retail while I was working on it (I'll attach the mission). So are we absolutely sure it's not supposed to be this way? I haven't seen anyone conclusively state that yes, this worked differently in retail. [Edit: Um, or I would attach, but uploads seem to be disabled atm. I'll attach it in the SCP forum on HLP instead] edited on: 08-29-06 14:20 edited on: 08-29-06 14:22 edited on: 08-29-06 14:25 |
|
|
Now wait a minute. If it has exactly the same behavior on retail as in SCP, then we can't change it. We have to maintain compatability. I had thought this was a situation where something got changed from retail to SCP. |
|
|
The problem is that the event is never considered current and so is skipped over. This is traced directly to the Sexp_special_number thing since the event is only ever considered current if it's been evaluated as true. That's the only time that it will display. Not setting Sexp_special_number to 0 should change nothing game wise, but it could very well throw up a bunch of directives that probably shouldn't be there. Goober may want to correct me on this, but I think that a proper fix would be to make a new sexp for this, which can always be current, or extent the current sexp to give the option of always current or not. In neither case will this change how the event is evaluated in a true/false way, only whether it's a current event. About the only real change to how it works will be that it gets a timestamp as soon as it's current rather than as soon as it's true. It would still have to evaluated as true or false to give an actual result, having it current only relates to how/whether it's displayed via the UI to the player. |
|
|
I figured as much. I'm just worried that it might start showing directives that were supposed to be hidden, like you said. On the other hand, since this is definitely a bug, any campaign worth its salt should have worked around it. I can't think of any situation where changing this would cause unwanted behavior, but that doesn't mean one doesn't exist. It might be worth putting out a test build, or even a test RC, with this bug fixed. We could ask several people to play through different campaigns to see if anything's different. Adding a new sexp, say directive-test, would be the least invasive solution, but I'm averse to adding duplicate sexps if they can be at all avoided. |
|
|
I've tested a few of the original campaign missions and none of them have shown a problem. This is a specific circumstance though, since the change only affects is-event-true when used with a directive. In general there should be no change involved since nothing else really has interface related code for this to deal with. And as you say, the missions where this might be an issue should have been dealt with originally. I'm up for a test build, so I'll do one up for Windows and give it out to a few people and mods to test with and we'll see what happens. Based on what I've tested and seen in the code though, I don't see this change being a problem, and it would fix the bug with no currently perceivable side-effects. |
|
|
The only reports I've had of the test build messing up are with FSPort. No bad reports from other mods or missions, they all appear to work as normal. An easy test of the FSPort issue can be found with sm1-01a. "Return to base 2" will show up when the mission starts, and shows you the order to leave. The training missions are also prone to this same thing. I assume that this is something that we can fix without much issue. All of these cases see to use an "and", so I think (without being the sexp guru) we can check for that if the event-true check is part of a set then it will use the old behavior. You know the sexp code better though, so is that doable without much trouble? |
|
|
Ugh. Setting aside the question of whether it's doable (I think it is), take a look at what's happening. This is a use of this behavior that I hadn't considered. The directive is acting like a set of chained events: as soon as the "Mission End" event becomes true, the directive becomes valid. As soon as Alpha 1 departs, the directive is satisfied. So regardless of how poor a design decision this is, it's established precedent that's probably used in a bunch of FS1-era missions at least. (Do you see any comments in the code that indicate that this was what they were trying to do? It's quite possible that this was added before chaining.) Conversely, we can't simply change behaviors based on an AND sexp because that leaves us with the original problem. A player could AND two event checks together, or he could use an event coupled with an is-departed-delay. This galls me, but I think we'll have to add the directive sexp. |
|
|
Changing anything regarding sexp_useful_number will definitely mess up things in certain missions. I tried it and went through a whole load of them hoping it was the solution, and it's what I was referring to when I wrote "I know how to make it display properly now at least, I just need to make it display without breaking anything else.". The problem goes deeper than that, and frankly I'm not entirely sure it's fixable. As far as I could tell, this particular SEXP and the underlying functions that support it just aren't designed to take into account 'unresolved' in addition to true and false, and it will take some major reworking to make it function like normal directives. The sexp_useful_number is a major hack to begin with, and seems to only be there because whoever made these functions was pressed for time and could think of no other way to quickly make it work. It can be quite easily FREDded around though, so frankly, as this does seem to be retail behaviour, I'd say leave it as is. If someone absolutely needs a SEXP with the other behaviour, my vacation time is almost here now so I should be able to add that in, but given that one can FRED around the problem I currently fail to see the need. |
|
|
I did look for comments after I noticed what was going on, but I didn't really see anything of interest. It just looks like the Sexp_useful_number thing was a hack to get around this issue specifically, and possibly extended to be used for other things. I can see no other reason that it would have issue. It does look like this is a FS1 era change though, and there just aren't that many FS1 era comments in the code still. I tried playing around with AND to consider it a special chain (via a SFN_* flag) but that was so ugly that there is no way in hell that I'd commit that. The extra sexp might just have to be the way to go, however painful that may be. FREDing around the problem is messy (or so I'm told), so that's not a valid option as far as I'm concerned. A new sexp isn't the greatest solution, but I think it's better than the alternative. |
|
|
FREDding around the problem isn't messy at all. I should know; I handle this sort of thing on a regular basis with ST:R. It's inconvenient, but it's not hard. [Looking at the reporter] Ugh, should have known this was a WCS thing. No offense, but I think the WCS guys should learn how to use FRED. They tend to solve their problems as if they were looking for a sledgehammer to swat a mosquito. |
|
|
Indeed. There are several way around it, none of them particularly messy or convoluted. |
|
|
So, do we just pass this off as something WCS needs to deal with, or go with the new sexps? I've already got the sexps coded up (exactly the same as is-event-*, but with is-directive-*, to reduce new code handling) so I need to know if I should just scrap it or not. Or, if a new sexp is wanted, whether it needs to be more specialized than just as a UI related fix. I have to say that I think this is just one of those crazy [V] things, and I don't really see a reason that we absolutely have to make FREDers work around it rather than offer another (perhaps more straightforward) solution. Regardless, this is by no means my area of interest nor expertise, so I'm just going to stand by and do what I'm told on this one. |
|
|
I don't mind a new sexp with the requested behaviour. But changing the old one in the way we're currently discussing (as I understand it) could potentially affect a few [V] missions by making directives appear too early. edited on: 09-01-06 15:56 |
|
|
WCS does need to deal with this, but the new sexps would be nice and quite convenient. However I don't think we need to make entire new sexps. What about simply tacking on an optional argument to is-event-* that would specify the new behavior? So we'd have: is-event-true-delay --"Some Event" --0 --true The optional boolean argument would determine whether the event uses the directive behavior or not. If you could manage this, taylor, I think it would be the best solution. :) |
|
|
Works for me, I'd prefer just modifying the current is-event-* sexps as well. My main focus right now is trying to get the Theora player in working order (just needs audio, plus a good bit cleanup) but I should be able to take care of the sexp changes before the weekend is over, should only take a couple of minutes. |
|
|
Fixered. |
|
fs2open: fs2_open_3_6_9 r3501 2006-09-08 02:10 Ported: N/A Details Diff |
fix for Mantis bug 0000617 (with a very big assist from Shade who did most of the initial work on this) |
Affected Issues 0000617 |
|
| mod - /branches/fs2_open_3_6_9/fs2_open/code/parse/sexp.cpp | Diff File | ||
|
fs2open: trunk r3508 2006-09-08 02:18 Ported: N/A Details Diff |
fix for Mantis bug 0000617 (with a very big assist from Shade who did most of the initial work on this) |
Affected Issues 0000617 |
|
| mod - /trunk/fs2_open/code/parse/sexp.cpp | Diff File | ||
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2005-11-13 19:05 | starman01 | New Issue | |
| 2005-11-13 21:13 | WMCoolmon | Note Added: 0003818 | |
| 2005-11-25 16:05 | Inquisitor | Description Updated | |
| 2005-11-25 16:05 | Inquisitor | Note Added: 0003934 | |
| 2005-11-25 21:17 | Goober5000 | Note Added: 0003937 | |
| 2006-01-09 19:21 | phreak | Note Added: 0004171 | |
| 2006-01-27 17:22 | starman01 | Note Added: 0004510 | |
| 2006-01-27 17:22 | starman01 | Note Edited: 0004510 | |
| 2006-01-29 18:45 | phreak | Note Added: 0004548 | |
| 2006-01-31 19:40 | starman01 | Note Added: 0004593 | |
| 2006-02-01 06:06 | Goober5000 | Note Added: 0004602 | |
| 2006-02-01 06:06 | Goober5000 | Assigned To | => taylor |
| 2006-02-01 06:06 | Goober5000 | Status | new => assigned |
| 2006-02-06 08:14 | CP5670 | Note Added: 0004672 | |
| 2006-03-16 23:53 | taylor | Note Added: 0005148 | |
| 2006-04-26 19:13 | Shade | Note Added: 0005393 | |
| 2006-06-01 18:30 | taylor | Note Added: 0005694 | |
| 2006-06-01 18:30 | taylor | Assigned To | taylor => Shade |
| 2006-06-04 16:01 | Shade | Note Added: 0005758 | |
| 2006-06-04 18:33 | Goober5000 | Note Added: 0005761 | |
| 2006-06-04 18:48 | Shade | Note Added: 0005763 | |
| 2006-06-04 18:51 | Shade | Note Edited: 0005763 | |
| 2006-08-29 03:28 | taylor | Note Added: 0006512 | |
| 2006-08-29 17:28 | Shade_alt | Note Added: 0006517 | |
| 2006-08-29 18:20 | Shade_alt | Note Edited: 0006517 | |
| 2006-08-29 18:22 | Shade_alt | Note Edited: 0006517 | |
| 2006-08-29 18:25 | Shade_alt | Note Edited: 0006517 | |
| 2006-08-29 20:25 | Goober5000 | Note Added: 0006519 | |
| 2006-08-29 21:04 | taylor | Note Added: 0006520 | |
| 2006-08-29 23:14 | Goober5000 | Note Added: 0006521 | |
| 2006-08-29 23:56 | taylor | Note Added: 0006522 | |
| 2006-09-01 05:05 | taylor | Note Added: 0006525 | |
| 2006-09-01 12:39 | Goober5000 | Note Added: 0006527 | |
| 2006-09-01 16:43 | Shade_alt | Note Added: 0006528 | |
| 2006-09-01 17:36 | taylor | Note Added: 0006529 | |
| 2006-09-01 18:21 | Goober5000 | Note Added: 0006530 | |
| 2006-09-01 19:10 | Shade | Note Added: 0006531 | |
| 2006-09-01 19:40 | taylor | Note Added: 0006532 | |
| 2006-09-01 19:50 | Shade | Note Added: 0006533 | |
| 2006-09-01 19:56 | Shade | Note Edited: 0006533 | |
| 2006-09-01 20:23 | Goober5000 | Note Added: 0006534 | |
| 2006-09-02 05:39 | taylor | Note Added: 0006535 | |
| 2006-09-05 19:01 | taylor | Assigned To | Shade => taylor |
| 2006-09-08 05:06 | taylor | Status | assigned => resolved |
| 2006-09-08 05:06 | taylor | Resolution | open => fixed |
| 2006-09-08 05:06 | taylor | Note Added: 0006559 | |
| 2012-01-05 22:55 | taylor | Changeset attached | => Import 2012-01-05 17:52:05 fs2_open_3_6_9 r3501 |
| 2012-01-05 22:55 | taylor | Changeset attached | => Import 2012-01-05 17:52:05 trunk r3508 |
| 2012-01-09 18:06 | taylor | Changeset attached | => fs2open_websvn fs2_open_3_6_9 r3501 |
| 2012-01-09 18:06 | taylor | Changeset attached | => fs2open_websvn trunk r3508 |
