View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0003004 | FSSCP | SEXPs | public | 2014-02-04 06:34 | 2014-02-09 05:07 |
| Reporter | GeneralBattuta | Assigned To | Goober5000 | ||
| Priority | urgent | Severity | crash | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| OS | Windows 7 | ||||
| Product Version | 3.7.1 | ||||
| Summary | 0003004: Alter-ship-flag (no subspace drive) removes subspace drives from support ships, causing crashes | ||||
| Description | If alter-ship-flag (no subspace drive) (true) (true) (Alpha 1) is called at mission start, any friendly support ship called in later in the mission has no subspace drive. If there are no ships with fighterbays present, the support ship cannot be ordered to depart. If there is a ship with a fighterbay present, the support ship will freeze in place and stop responding to orders. If all ships with fighterbays leave, the mission crashes instantly. | ||||
| Steps To Reproduce | See above. | ||||
| Tags | No tags attached. | ||||
|
|
|
|
|
Well the problem begins in Missionparse.cpp where we have this particular piece of stupidity if ( Ships[Player_obj->instance].flags2 & SF2_NO_SUBSPACE_DRIVE ) pobj->flags2 |= P2_SF2_NO_SUBSPACE_DRIVE; // support ships have no subspace drive when player has not subspace drive But I'm not sure what to do about it. Sure I can add an option to Mission Specs to ignore that code. But in the end what I'm essentially adding is a "Under these rare circumstances, Don't Crash" button |
|
|
What exactly is that accomplishing, anyway? I wonder at the thought process that went on that produced that piece of code; I would be strongly in favour of removing it and seeing what (if anything) breaks. |
|
|
Just taking a wild stab at it, but I wonder if it was meant to prevent a warp-nerfed player from calling in a support ship, and then ordering said ship to depart while in the middle of the reloading process. If the support ship doesn't undock before warping out, then the player would essentially get a free ride into subspace. What I don't know is if that's how the support ship works, but I can take a quick look at it. |
|
|
Nope, from what I can tell the support ship always seems to abort the rearming if you order it to depart in the middle. No idea then. |
|
|
Actually I suspect the reason for it is a lot more stupid. I suspect it was to make sure that FS1 support ships didn't have shields early on in the game. Similarly, if the player couldn't jump for some reason, the support ship shouldn't be able to. |
|
|
yeah - but it doesn't stop the support ship from jumping in, only stops it jumping out (I'm sorry, but we decided you're at risk of deserting so we only installed a one-use jump drive in your ship...) Goober added the jump drive thing in r314 - best ask him why I suppose: https://svn.icculus.org/fs2open/trunk/fs2_open/code/mission/missionparse.cpp?limit_changes=100&r1=314&r2=313&pathrev=314 The shields thing will be a problem for (e.g.) fsport though so that had probably better stay in. |
|
|
For TVWP and FSPort, we have missions where players may not have shields or subspace drives. In those cases, we better make sure the support ships doesn't have them either, otherwise it's anachronistic behavior. The shield check is from retail, and the subspace drive check was patterned off of it. The best way to fix this bug is to examine the support ship departure logic, find out where it's crashing, and make it more robust. |
|
|
But wouldn't the better solution be to define that in a more transparent way? The thing is, if I disable subspace or shields for a ship, I do not expect that to have ramifications for other vessels in the same mission. The way it is now is just random action-at-a-distance, which I think is not the right way to do it. |
|
|
Goob, you can't be serious. You want support ships to lose their subspace drives in every mission that uses mid-mission jumps? There are a LOT of these missions. If you want your support ships in TVWP and FSPort to not have subspace drives, do the responsible thing and alter-ship-flag them to NOT HAVE SUBSPACE DRIVES. This is a trivial exercise in FRED. Don't create an undocumented, invisible behavior that fucks over other mods. |
|
|
I bring up mid-mission jumps because the correct way to handle the player's subspace drive in those missions, according to the documentation facing the FREDder, is to alter-ship-flag the drive away. |
|
|
I have to agree there. If you want that behaviour in other mods, code it in. The shield code thing is a stupid hack. We shouldn't repeat V's stupid hacks just cause it works for the mod we work on. That doesn't mean we shouldn't trace out the cause of the crash too. But how can you seriously justify adding another hack like this one? |
|
|
You guys have really got to stop jumping to conclusions. Find out why the something is a certain way before you ignorantly attack it. The no-subspace-drive flag dates from 2003. It was coded with the entirely reasonable assumption that the ship really *did* have no subspace drive. The in-mission jump technique wasn't invented until *years* later. Although I'm flattered that people think of me as stroking a cat in an evil lair, thinking up ways to sabotage mods, that is quite far from the truth. On the subject of stupid hacks, I'd like to remind people that the support ship code uses a lot of them. Since support ships technically don't exist as part of the FREDded mission, the code has to use a lot of heuristics to find out how to create it. If you think about it, since the support ship is summoned by the player, it makes sense to create it based on what the player is flying. Case in point: the support ship is assumed to be the same species as the player, and I don't see anyone complaining about that. Now, I just realized that if a mission takes place in the T-V war or early Great War, then every ship on the player's team is going to lack shields or subspace drives, not just the player. So I'll change the heuristic so that the support ship will only lack shields or subspace drives if every ship on the player's team lacks shields or subspace drive at the time the mission is parsed. That will satisfy both the TVWP situation and the in-mission jump situation. |
|
|
As I pointed out on IRC, we're not leaping to conclusions: this CTD would've been present in 2003. And if our assumptions are quite far from the truth, why did you add an undocumented 'feature' that doesn't even work for the people who want it - since they have no way to know about it? There is a safe, reliable, INTENTIONAL way to achieve the objective: support ships with no jump drives when you WANT them to have no jump drive. There is also an undocumented, unsafe way to do it: the way it's implemented now. Even after your change - although the risks are reduced - you're still screwing over a lot of potential scenarios. What if Alpha 1 is the only friendly ship present? That's 'the whole team'. What if a friendly destroyer then arrives, our support ship is ordered to depart, and the friendly destroyer leaves? CTD. Let's say you fix the CTD. What if a FREDder wants to set up a scenario in which the player is all alone in a ship with no subspace drive and has to call a support ship to tow them into subspace? They can't execute this scenario via the 'rearm/repair' feature. There is no documentation to explain why. This feature should be removed. The objective of generating support ships who inherit traits from the player should be achieved by expanding FRED's control over support ship traits. As I said on IRC, I don't think anybody should be implementing undocumented, unmodifiable features with gameplay ramifications at this low level. This heuristic for support ship inheritance is inflexible to a degree that could actively hamper design in mods like Diaspora, where jump-capable support ships might well interact with full teams of Vipers without jump drives. |
|
|
The CTD didn't exist in 2003; I tested the feature pretty extensively before I added it. There must have been a regression bug introduced in the intervening time period. By the way, your characterization of this as an "undocumented, unmodifiable feature with gameplay ramifications at this low level" is entirely unfounded. The game engine is chock full of these assumptions because the engine started life intended for FreeSpace 2. It's not a general-purpose engine. You need to start with the frame of reference of FS2 and then find out how you can adapt it for other mods. You remind me of the Wing Commander team getting furious that enemy ships did not transmit distress calls when they were destroyed. It's *part of the original game*. If you want a deviation from the assumed behavior, we might be willing to accommodate you, as we did WCSaga, but don't come barging in and saying that the behavior that existed since 1998 is unjustified. |
|
|
A feature you added specifically for TVWP and FSPort is not a relic behavior from 1998, it's an intentional decision that even in the year 2003 prevented certain mission types (in the vanilla FreeSpace 2 setting!) from working correctly. I just provided an example of one of those mission types. Stop being defensive. Stop trying to characterize my objections to this design error as somehow rooted in 'a different frame of reference'. This feature does not work IN THE REFERENCE FRAME OF FREESPACE 2. It was not part of the original game because _you added it_. |
|
|
I'm not being defensive. And you are entirely out of line. You are accusing me of maliciously "screwing over" mods and missions with this feature, when I've proven that a) the behavior is consistent with Volition's code, and b) correct for standard and reasonable usage. And you are blaming TVWP and FSPort for "fucking over" a particular use of the feature that came years later, rewriting the history of events. Brainstorming edge cases and then attacking the feature for failing to satisfy those unlikely situations is dishonest and self-serving. And relying on an "undocumented side effect" for in-mission jumps (viz. the disabling of the Alt-J key) and then characterizing support ships having no drives as an "undocumented side effect" is hypocritical in the extreme. A reasonable person would ask why the state of affairs is a certain way, and then investigate how things can be changed to get from the current state to the desired state. Indeed, that's what everybody else on this ticket has been doing. But what you're doing is declaring that things should be a certain way, assuming /a priori/ that it's a done deal, and then attacking everything that doesn't fit your expectations. |
|
|
Goob, I've outlined why the current feature - an invisible, undocumented feature created for your benefit that fucks over other mods: these are all, as of the current moment, facts - is bad design, made it clear that I think we can improve it, and made it clear how. I'm not interested in rewriting events. That's your interpretation. The current state of affairs is a problem and needs to be fixed: that's what I'm interested in. Your accusation that I am brainstorming edge cases is inaccurate. We just HIT one of those cases in the course of making a mission using a mainstay technique outlined by one of the community's most popular and well-read FRED exports. Diaspora is another case I've presented where a main case, not an edge case, hits this error. You have clearly never tried to work an in-mission jump. Think about it for a moment. Why might disabling the Alt-J key be a bad idea a FREDder would not explore (even if it works; I don't know if it does) when there's an easier, more reliable alternative recommended by Axem available? You continue to attempt to speak to my character rather than the issue at hand. Please step back, focus on the code problem, and think about how to solve it. In your last post alone you racked up 'out of line', invented an accusation aimed at you personally rather than the feature, accused me of revisionism, accused me of self-serving dishonesty, accused me of hypocrisy, and accused me of being unreasonable. On HLP I'd already have reported your post for attempting to turn the conversation into an ad hominem. Here, I expect you to stay focused on discussing the feature and how to solve it. Step up. |
|
|
And I hope the difference between the intentional, plain, and documented use of a SEXP by the mission designer and a support ship's undocumented ability to inherit flags from the player's ship is clear. I REALLY hope this difference is clear. |
|
|
This "feature" absolutely HAS to be reworked in a more sane, controllable way. I do not care one tiny little bit about the needs of TVWP or FSPort from 11 years ago. No matter how you look at it, support ships losing subspace drive or shields /because the player ship did/ is not a valid course of action. FREDers need to know that the events they code up will not end up breaking the engine in hilarious ways, and they need to know the ramifications of any given command. As you said, Goober: FSO is based on an engine whose single purpose was to run FS2. /None of the assumptions that went into this mess is valid for FS2/. They are at best holdovers from FS1, which should have been removed in the interest of sanity. The only way to do this right is by manipulating the properties of the support ship, not by copying them from the player ship. I know this is a bit awkward, due to the support ship technically only existing on demand, but this is hardly an insurmountable hurdle to overcome. |
|
|
EDIT: in response to Battuta's previous post... So now you're admitting that when the conversation doesn't go your way, you would start reporting posts? This is another example of assuming the conclusion and then attacking everything contrary to it. Thank you for demonstrating my point for me. It's ironic that you're accusing me of making personal attacks when it is not me, but you that is making this personal. You are the one who is blaming the support ship behavior on me, personally, when I've demonstrated that I'm merely adhering to a pattern that Volition already established. You are the one who is accusing me of "screwing over" or "fucking over" other mods. And you are most certainly rewriting events. You are switching cause and effect and you are ascribing pre-existing standards to personal motivations. "Edge cases" does not refer to the support ship freezing, or the game crashing. I've already acknowledged those as bugs and stated that I will fix them. "Edge cases" refers to situations like calling in a support ship to tow the player out of the mission. I don't think that has ever been FREDded even in a mission where the player *does* have a subspace drive, which is the vast majority of cases. As for Diaspora, that is not part of the standard usage of the subspace drive flag because it's not part of the FreeSpace universe. By virtue of being a completely different mod, any deviations from FreeSpace's behavior cannot be assumed; they must be tested and, if necessary, deliberately changed. In any event, the Diaspora situation you describe is evidently *not* a main case because Diaspora has been in development for quite some time and this issue has never come up until now. As for "disabling the Alt-J key", that was imprecise phrasing. Specifically, the behavior is that pressing Alt-J does not produce any of the side-effects (such as a beep or error message) that it would be expected to produce. That is what is being relied on in the in-mission-jump sequence. The *primary* side effects, such as the changes in AI and landing behavior, are ignored. |
|
|
In response to The E: I've double-checked with the TVWP mod, and it turns out that none of the missions that contain the no-subspace-drive flag actually *use* support ships. So that particular line in the code can be removed with no adverse effects on released missions, and in its place we can do something like augment the set-support-ship sexp to set global support ship flags. For shields, though, there are already released missions that depend on this behavior. The most notable are the first several missions in FSPort before the GTA acquires shield, but it is also necessary to consider every FS2 mission that takes place in subspace. Derelict, for example, is a FS2-era mod that depends on this behavior. |
|
|
This is very disappointing. Watching an admin on HLP accuse a member of 'attacking everything contrary' for obeying the forum rules, which ask him - quite clearly - to report ad hominem attacks, shakes my faith in your ability to serve in the position. You persist in believing that comments aimed at a piece of code were somehow meant to describe your personal intentions. You've failed to step away from your personal concerns and focus on the problem. You've also failed to demonstrate a working knowledge of FRED. If the Alt-J key were disabled, Goob, it should be transparently obvious (and I just tested it to be sure) that THE ALT-J KEY IS DISABLED: it cannot be used to initiate the mid-mission jump! Your last attempt to create a special exception, involving the # key in wing names, created a problem. You tried to fix the problem by adding a second special exception. One of our missions now cannot be saved in FRED due to the setup of this second exception. Perhaps this regime of special-casing your special-cases is causing more problems then it's worth? You would apparently prefer to attribute concerns over your coding methodology to a conspiracy against you than to genuine concerns. Hahahaha, Christ. I'll leave Karajorma to explain to you why that situation never arose in Diaspora, and how it could. You haven't played Diaspora. I've spoken to other coders and received assurances this issue will be addressed. If you're unable to hold a civil dialogue on the topic please stand aside and let us work. |
|
|
Actually, I think my comment on the Alt-J key being ignored is unfair. It's probably trivial to make it work. e: or is it? Actually, no, I don't think it is. There's no way to achieve the alter-ship-flag (no subspace drive) effect _at the point of the mid-mission jump_ using it. |
|
|
So about the crash - I was wondering if there was something going on with ship-vanish and Ship_obj_list. i.e. maybe it's not clearing Ship_obj_list correctly, or maybe its trashing the subsystem list referenced by elements in Ships[]? The Objects[] array is also used in this code path so perhaps there's something not being cleaned up there? |
|
|
Battuta: Your persistence at twisting my language to suit your agenda is mystifying. You are continuing to mischaracterize my position and impugn my motivations. niffiwan: I haven't actually been able to test the mission yet, due to the need to refute Battuta's persistent lies. But I intend to work on it this evening. |
|
|
I'll take that as an apology. You've agreed to the need to rework this behavior, which is all that needs to happen here. |
|
|
*wtf* I have nothing to apologize for. You, on the other hand, have deliberately mischaracterized this entire converation, claimed that if this was on HLP you would report my post, implicitly questioned my suitability as an admin, denigrating my coding and FREDding abilities, and adopted a patronizing superiority complex. I am discontinuing the refutation of your points because you refuse to admit your errors, but I do not concede any of them. And I intend to fix the CTD and modify the support ship behavior, but not on account of your mendacious behavior. |
|
|
You got upset because you thought you were being accused of trying to sabotage other mods. That remark was aimed at the feature, not at your personal intentions. I know you didn't have any ill intent when you wrote the code, and as long as it's going to be redesigned, the situation's resolved. |
|
|
Goober, the behaviour of both the shields and the jump drives quite clearly violates the Unintended Consequences rule. Changing a flag on one ship absolutely should not have a knock-on effect to another ship. You're usually a champion of avoiding this sort of thing whenever anyone else suggests it, don't blind yourself to the awfulness of this implementation simply because you copied V's stupid mistake. It's quite clear that V added it that way simply as a hack to make sure that in early FS1 subspace missions, support ships wouldn't have shields. What they quite clearly should have done is simply added another option to the mission notes dialog. Just because V decided to add a stupid hack does not mean that we should follow suit. This is the exact same thinking that led to special hits using the stupid block variables system with all its attendant problems until I rewrote the whole damn thing and got rid of them. Features should be added with forethought as to how modders might use them. This hack doesn't do that and as such was a bad way to implement it. Diaspora has been mentioned several times precisely because the main ships in Diaspora lack jump drives, but refuelling ships are canon and would almost certainly have them. Had battuta not stumbled across this bug, Diaspora definitely would have the second we implement fuel ships. |
|
|
You make several good points. As I mentioned in my previous response to The E, I'll remove the hack that affected the no-subspace-drive flag. The cause of the CTD has been found and fixed; it was caused by a hole in an if() structure that allowed a negative array index access to occur. I ended up rewriting the function to look cleaner, and I've verified that it no longer CTDs. Next I'll take a look at the support ship freezing. |
|
|
Okay, all of the aspects of this report have been addressed. |
|
fs2open: trunk r10407 2014-02-05 22:07 Ported: N/A Details Diff |
Mantis 0003004: ship_get_ship_with_dock_bay was written in such a way that it allowed a negative array index to create all sorts of havoc |
Affected Issues 0003004 |
|
| mod - /trunk/fs2_open/code/ship/ship.cpp | Diff File | ||
|
fs2open: trunk r10428 2014-02-08 23:12 Ported: N/A Details Diff |
Mantis 0003004: standardize checks for whether ships can depart to docking bays, and whether departure goals are achievable, and whether a ship can be commanded to depart; also fix the default behavior if a ship attempts to warp out but is unable to |
Affected Issues 0003004 |
|
| mod - /trunk/fs2_open/code/ai/ai.h | Diff File | ||
| mod - /trunk/fs2_open/code/ai/aicode.cpp | Diff File | ||
| mod - /trunk/fs2_open/code/ai/aigoals.cpp | Diff File | ||
| mod - /trunk/fs2_open/code/hud/hudsquadmsg.cpp | Diff File | ||
| mod - /trunk/fs2_open/code/mission/missionparse.cpp | Diff File | ||
| mod - /trunk/fs2_open/code/ship/ship.cpp | Diff File | ||
| mod - /trunk/fs2_open/code/ship/ship.h | Diff File | ||
|
fs2open: trunk r10429 2014-02-08 23:13 Ported: N/A Details Diff |
Mantis 0003004: don't remove subspace drives from support ships |
Affected Issues 0003004 |
|
| mod - /trunk/fs2_open/code/mission/missionparse.cpp | Diff File | ||
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2014-02-04 06:34 | GeneralBattuta | New Issue | |
| 2014-02-04 06:34 | GeneralBattuta | File Added: reprosupportshipcrash.fs2 | |
| 2014-02-04 07:59 | karajorma | Assigned To | => karajorma |
| 2014-02-04 07:59 | karajorma | Status | new => assigned |
| 2014-02-04 08:59 | karajorma | Note Added: 0015589 | |
| 2014-02-04 09:23 | The_E | Note Added: 0015590 | |
| 2014-02-04 09:30 | Mongoose | Note Added: 0015591 | |
| 2014-02-04 09:49 | Mongoose | Note Added: 0015592 | |
| 2014-02-04 09:49 | karajorma | Note Added: 0015593 | |
| 2014-02-04 10:17 | niffiwan | Note Added: 0015594 | |
| 2014-02-04 15:24 | Goober5000 | Note Added: 0015595 | |
| 2014-02-04 15:32 | The_E | Note Added: 0015596 | |
| 2014-02-04 15:35 | GeneralBattuta | Note Added: 0015597 | |
| 2014-02-04 15:36 | GeneralBattuta | Note Added: 0015598 | |
| 2014-02-04 15:38 | karajorma | Note Added: 0015599 | |
| 2014-02-04 15:40 | karajorma | Note Edited: 0015599 | |
| 2014-02-04 19:05 | Goober5000 | Note Added: 0015600 | |
| 2014-02-04 19:06 | Goober5000 | Assigned To | karajorma => Goober5000 |
| 2014-02-04 19:06 | Goober5000 | Note Edited: 0015600 | |
| 2014-02-04 19:28 | GeneralBattuta | Note Added: 0015601 | |
| 2014-02-04 19:44 | Goober5000 | Note Added: 0015602 | |
| 2014-02-04 19:49 | GeneralBattuta | Note Added: 0015603 | |
| 2014-02-04 20:42 | Goober5000 | Note Added: 0015604 | |
| 2014-02-04 20:53 | GeneralBattuta | Note Added: 0015605 | |
| 2014-02-04 21:04 | GeneralBattuta | Note Added: 0015606 | |
| 2014-02-04 21:09 | The_E | Note Added: 0015607 | |
| 2014-02-04 21:27 | Goober5000 | Note Added: 0015608 | |
| 2014-02-04 21:28 | Goober5000 | Note Edited: 0015608 | |
| 2014-02-04 21:35 | Goober5000 | Note Added: 0015609 | |
| 2014-02-04 21:39 | GeneralBattuta | Note Added: 0015610 | |
| 2014-02-04 21:41 | GeneralBattuta | Note Added: 0015611 | |
| 2014-02-04 21:41 | niffiwan | Note Added: 0015612 | |
| 2014-02-04 21:41 | GeneralBattuta | Note Edited: 0015611 | |
| 2014-02-04 21:42 | GeneralBattuta | Note Edited: 0015611 | |
| 2014-02-04 21:58 | Goober5000 | Note Added: 0015613 | |
| 2014-02-04 22:02 | GeneralBattuta | Note Added: 0015614 | |
| 2014-02-04 22:12 | Goober5000 | Note Added: 0015615 | |
| 2014-02-04 22:16 | GeneralBattuta | Note Added: 0015616 | |
| 2014-02-05 04:50 | karajorma | Note Added: 0015617 | |
| 2014-02-06 04:00 | Goober5000 | Changeset attached | => fs2open trunk r10407 |
| 2014-02-06 04:02 | Goober5000 | Note Added: 0015618 | |
| 2014-02-09 05:05 | Goober5000 | Changeset attached | => fs2open trunk r10428 |
| 2014-02-09 05:06 | Goober5000 | Changeset attached | => fs2open trunk r10429 |
| 2014-02-09 05:07 | Goober5000 | Note Added: 0015620 | |
| 2014-02-09 05:07 | Goober5000 | Status | assigned => resolved |
| 2014-02-09 05:07 | Goober5000 | Resolution | open => fixed |
