View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0002153 | FSSCP | AI | public | 2010-03-16 11:31 | 2014-07-18 02:50 |
Reporter | The_E | Assigned To | MageKing17 | ||
Priority | normal | Severity | minor | Reproducibility | always |
Status | resolved | Resolution | fixed | ||
Product Version | 3.6.12 RC1 | ||||
Target Version | 3.7.2 | Fixed in Version | 3.7.2 | ||
Summary | 0002153: FRED doesn't warn about wingmen with illegal weapons, causing confusion when the mission loads | ||||
Description | Not sure if this is a bug or a feature request. Now, the issue is the following. If I give a hostile ship weapons that that ship can't normally carry (Helios on a Uly, for example), that ship is capable of using the equipped weapon. Friendly ships, however, get their secondary banks blanked on mission start, and are thus not able to use those same weapons, even if the mission designer intended them to. If a ship that has an altered loadout switches IFF from hostile to friendly in mid-mission, it keeps the altered loadout (and the ability to fire it). IMO, this is an asymmetry in behaviour that needs to be either documented and warned about, or be corrected. | ||||
Tags | No tags attached. | ||||
|
And as I said on IRC, if alternative behavior is added, it should be optional mission specs checkbox or something like that. We just can't take the risk of suddenly breaking mission balance if any mission designer accidentally left non-allowed weapons to be used in a mission. |
|
It's worth noting that friendly ships can use prohibited weapons if the mission is a scramble mission. The Bakha uses Helios bombs on both Bearbaiting and High Noon when it shouldn't be able to. In my opinion, the best way to solve this would be to add an additional routine to the FRED error checker so that FRED complains if any ship is using weapons it shouldn't be allowed to use. Doing anything else is just going to cause problems one way or another. |
|
|
|
|
|
Added 2 versions of the same mission. Has a hostile and friendly wing firing at a Rakshasa. Hostile wing is Seths forced to carry Helios bombs. Friendly wing is Ulysses forced to carry cyclops. One version is a scramble mission, per Goober's note. In my tests, during neither version did the friendly ships use the Cyclops weapons, they were forced to Rockeyes instead. Now, I think that if the FREDder has chosen to use certain weapons, they should be allowed.. at least in Scramble missions. I'd even be all for a game_settings flag that allows this behavior always. However, it doesn't seem to be working in my test scramble mission, so perhaps there is a legitimate bug going on here that will definitely affect retail. |
|
I can confirm MjnMixael's findings; in neither a scramble nor a regular mission do friendlies use Cyclops torpedoes, they're forced to use Rockeyes. My question for now is why we would allow this behaviour (except for retail compatibility) at all. If a FREDder wants a ship to use a certain weapon, they should allow that weapon in the ship's table entry; that's what the tables are for, aren't they? Adding overrides for such things doesn't make much sense to me. Hence IMHO FRED should probably complain about this if it is done. |
|
I've uploaded a patch to restore retail behaviour; I tested it with Mjn's missions and it seemed to fix the issue. I'll go about adding a warning to FRED's error checker if no one has any objections. EDIT: Patch modified to save us a needless function call. |
|
2153.patch (587 bytes)
Index: code/missionui/missionweaponchoice.cpp =================================================================== --- code/missionui/missionweaponchoice.cpp (revision 9446) +++ code/missionui/missionweaponchoice.cpp (working copy) @@ -1679,7 +1679,9 @@ } // ensure that there aren't any bogus weapons assigned by default - wl_cull_illegal_weapons(ship_class, wep, wep_count); + // In retail, illegal weapons are allowed if the mission is a scramble mission + if(!(The_mission.flags & MISSION_FLAG_SCRAMBLE)) + wl_cull_illegal_weapons(ship_class, wep, wep_count); } /** |
|
Patch fixes for scramble, so retail should now be fine. For the FRED warning, I would suggest it say something like "Weapon X not allowed on Ship Y. Weapon will be reset to default unless mission is a Scramble". (Alternatively, only have the warning trigger if the 'Scramble' is not checked for the mission. |
|
Hang on. Why is the change to missionweaponchoice.cpp needed? That same line is present in the retail code. |
|
Bump as I've just run into issues in this area. Any chance we can come to an agreement on how to handle it? |
|
I've reviewed the patch and it's a valid fix for the issue. However, what was bugging me back in 2012 is that the patch doesn't restore retail code that was modified; instead it adds an extra check to code that has been unchanged since retail. (In fact, looking through the retail code, I can't figure out why weapons *weren't* culled on scramble missions.) As far as actually handling this bug, I can go ahead and commit CommanderDJ's patch. CommanderDJ can also add the warning to FRED's error checker that he mentioned. |
|
It would appear that the reason the GVB Bakha is capable of mounting Helioses in Bearbaiting/High Noon is because the ships.tbl entry for the GVB Bakha lists "Helios" at the end of $Allowed PBanks. Yes, that's right, the Helios is listed as a valid primary weapon for the GVB Bakha. In fact, you can go ahead and mount Helioses on a Bakha normally, despite it (obviously) being a secondary weapon, not a primary. There's absolutely nothing special about a Bakha mounting Helioses, scramble mission or not. |
|
Ugh. The same thing is true in non-scramble retail missions as well. I wonder where I got the idea from then? It's been "common knowledge" for quite a number of years... |
|
I've submitted a patch that makes FRED complain about illegal weapons (on either a player ship or a ship in a starting wing, which should be the only ones subject to illegal weapon culling). |
|
Thanks for this. I wouldn't have had time to look at this for a few weeks yet, so I'm glad it's being attended to. |
|
MageKing17, your fredview.cpp patch doesn't handle the case where a ship is not part of a wing and therefore its wingnum is -1. Also, is the loadout problem applicable to TVT starting wings as well? If so, you should iterate through TVT_wings. |
|
I have no idea how the issue interacts with multiplayer at all. EDIT: Updated patch to correct -1 issue, at least. |
|
I mean, if weapons are culled from a player's wing, it stands to reason that would be true in multiplayer as much as single player. |
|
Well, it would seem logical, but the code governing this behavior wasn't really written in a "logical" fashion. Still, it should be fairly trivial to check TVT_wings as well, so might as well. EDIT: And done. EDIT: Whoops, the variable is TVT_wing_names. Fixed. |
|
Sorry, I forgot to specify that the TVT check should only be done if it's a multiplayer TVT game and the Starting_wings check should be done otherwise. (You can find examples of this dual-check in global_error_check_player_wings().) Because Zeta wing is a valid non-starting wing in single player. |
|
fredview.cpp.patch (1,619 bytes)
Index: code/fred2/fredview.cpp =================================================================== --- code/fred2/fredview.cpp (revision 10918) +++ code/fred2/fredview.cpp (working copy) @@ -2688,6 +2688,46 @@ internal_error("Invalid dockee point (\"%s\" initially docked with \"%s\")", Ships[i].ship_name, Ships[ship].ship_name); } } + + wing = Ships[i].wingnum; + bool is_in_loadout_screen = (ptr->type == OBJ_START); + if (!is_in_loadout_screen && wing >= 0) { + if ( multi && The_mission.game_type & MISSION_TYPE_MULTI_TEAMS ) + { + for (n = 0; n < MAX_TVT_WINGS; n++) { + if (!strcmp(Wings[wing].name, TVT_wing_names[n])) { + is_in_loadout_screen = true; + break; + } + } + } else { + for (n = 0; n < MAX_STARTING_WINGS; n++) { + if (!strcmp(Wings[wing].name, Starting_wing_names[n])) { + is_in_loadout_screen = true; + break; + } + } + } + } + if (is_in_loadout_screen) { + int illegal = 0; + z = Ships[i].ship_info_index; + for (n = 0; n < MAX_SHIP_PRIMARY_BANKS; n++){ + if (Ships[i].weapons.primary_bank_weapons[n] >= 0 && !Ship_info[z].allowed_weapons[Ships[i].weapons.primary_bank_weapons[n]]){ + illegal++; + } + } + + for (n = 0; n < MAX_SHIP_SECONDARY_BANKS; n++){ + if (Ships[i].weapons.secondary_bank_weapons[n] >= 0 && !Ship_info[z].allowed_weapons[Ships[i].weapons.secondary_bank_weapons[n]]){ + illegal++; + } + } + + if (illegal && error("%d illegal weapon(s) found on ship \"%s\"", illegal, Ships[i].ship_name)) { + return 1; + } + } } } |
|
My initial thought was to warn the FREDer either way in case they switch between single and multiplayer, but in hindsight, this is inconsistent with trying to only warn about ships FSO will cull illegal weapons off of, so... multiplayer check copied. |
|
Looks good. Patch committed. |
|
Changed the ticket description to reflect the actual issue that was fixed by the commit. |
Date Modified | Username | Field | Change |
---|---|---|---|
2010-03-16 11:31 | The_E | New Issue | |
2010-03-16 11:31 | The_E | Status | new => assigned |
2010-03-16 11:31 | The_E | Assigned To | => Sushi_CW |
2010-03-16 11:37 | Fury | Note Added: 0011782 | |
2012-12-17 08:01 | Goober5000 | Note Added: 0014499 | |
2012-12-17 08:01 | Goober5000 | Assigned To | Sushi_CW => |
2012-12-17 08:01 | Goober5000 | Status | assigned => confirmed |
2012-12-17 08:01 | Goober5000 | Note Edited: 0014499 | |
2012-12-18 04:05 | MjnMixael | File Added: 2153a.fs2 | |
2012-12-18 04:05 | MjnMixael | File Added: 2153b.fs2 | |
2012-12-18 04:09 | MjnMixael | Note Added: 0014507 | |
2012-12-18 10:13 | CommanderDJ | Assigned To | => CommanderDJ |
2012-12-18 10:13 | CommanderDJ | Status | confirmed => assigned |
2012-12-18 12:15 | CommanderDJ | Note Added: 0014511 | |
2012-12-18 12:34 | CommanderDJ | File Added: 2153.patch | |
2012-12-18 12:36 | CommanderDJ | Note Added: 0014513 | |
2012-12-18 12:40 | CommanderDJ | File Deleted: 2153.patch | |
2012-12-18 12:40 | CommanderDJ | File Added: 2153.patch | |
2012-12-18 12:41 | CommanderDJ | Note Edited: 0014513 | |
2012-12-18 19:40 | MjnMixael | Note Added: 0014519 | |
2012-12-19 05:29 | Goober5000 | Note Added: 0014538 | |
2014-06-30 16:03 | MjnMixael | Note Added: 0015972 | |
2014-07-02 14:07 | Goober5000 | Target Version | => 3.7.2 |
2014-07-07 03:40 | Goober5000 | Note Added: 0016029 | |
2014-07-07 08:10 | MageKing17 | Note Added: 0016035 | |
2014-07-08 00:11 | Goober5000 | Note Added: 0016039 | |
2014-07-10 02:55 | MageKing17 | File Added: fredview.cpp.patch | |
2014-07-10 02:55 | MageKing17 | Assigned To | CommanderDJ => MageKing17 |
2014-07-10 02:55 | MageKing17 | Status | assigned => code review |
2014-07-10 02:56 | MageKing17 | Note Added: 0016057 | |
2014-07-10 02:58 | MageKing17 | File Deleted: fredview.cpp.patch | |
2014-07-10 02:58 | MageKing17 | File Added: fredview.cpp.patch | |
2014-07-10 03:03 | CommanderDJ | Note Added: 0016058 | |
2014-07-17 01:24 | Goober5000 | Note Added: 0016078 | |
2014-07-17 01:26 | MageKing17 | Note Added: 0016079 | |
2014-07-17 01:33 | MageKing17 | File Deleted: fredview.cpp.patch | |
2014-07-17 01:33 | MageKing17 | File Added: fredview.cpp.patch | |
2014-07-17 01:33 | MageKing17 | Note Edited: 0016079 | |
2014-07-17 01:34 | MageKing17 | Note Edited: 0016079 | |
2014-07-17 02:12 | Goober5000 | Note Added: 0016080 | |
2014-07-17 02:25 | MageKing17 | Note Added: 0016081 | |
2014-07-17 02:34 | MageKing17 | File Deleted: fredview.cpp.patch | |
2014-07-17 02:34 | MageKing17 | File Added: fredview.cpp.patch | |
2014-07-17 02:34 | MageKing17 | Note Edited: 0016081 | |
2014-07-17 02:37 | MageKing17 | File Deleted: fredview.cpp.patch | |
2014-07-17 02:37 | MageKing17 | File Added: fredview.cpp.patch | |
2014-07-17 02:38 | MageKing17 | Note Edited: 0016081 | |
2014-07-17 03:28 | Goober5000 | Note Added: 0016082 | |
2014-07-17 05:34 | MageKing17 | File Deleted: fredview.cpp.patch | |
2014-07-17 05:34 | MageKing17 | File Added: fredview.cpp.patch | |
2014-07-17 05:36 | MageKing17 | Note Added: 0016084 | |
2014-07-18 02:48 | Goober5000 | Changeset attached | => fs2open trunk r10924 |
2014-07-18 02:49 | Goober5000 | Note Added: 0016091 | |
2014-07-18 02:49 | Goober5000 | Status | code review => resolved |
2014-07-18 02:49 | Goober5000 | Resolution | open => fixed |
2014-07-18 02:49 | Goober5000 | Fixed in Version | => 3.7.2 |
2014-07-18 02:50 | Goober5000 | Note Added: 0016092 | |
2014-07-18 02:50 | Goober5000 | Summary | Friendly ships can't fire weapons that are not allowed in ships.tbl loadout => FRED doesn't warn about wingmen with illegal weapons, causing confusion when the mission loads |