2019-10-14 12:50 EDT


View Issue Details Jump to Notes ] Related Changesets ]
IDProjectCategoryView StatusLast Update
0002153FSSCPAIpublic2014-07-17 22:50
ReporterThe_E 
Assigned ToMageKing17 
PrioritynormalSeverityminorReproducibilityalways
StatusresolvedResolutionfixed 
Product Version3.6.12 RC1 
Target Version3.7.2Fixed in Version3.7.2 
Summary0002153: FRED doesn't warn about wingmen with illegal weapons, causing confusion when the mission loads
DescriptionNot 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.
TagsNo tags attached.
Attached Files
  • ? file icon 2153a.fs2 (9,253 bytes) 2012-12-17 23:05
  • ? file icon 2153b.fs2 (9,267 bytes) 2012-12-17 23:05
  • patch file icon 2153.patch (587 bytes) 2012-12-18 07:40 -
    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 file icon 2153.patch (587 bytes) 2012-12-18 07:40 +
  • patch file icon fredview.cpp.patch (1,619 bytes) 2014-07-17 01:34 -
    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;
    +				}
    +			}
     		}
     	}
     
    
    patch file icon fredview.cpp.patch (1,619 bytes) 2014-07-17 01:34 +

-Relationships
+Relationships

-Notes

~0011782

Fury (reporter)

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.

~0014499

Goober5000 (administrator)

Last edited: 2012-12-17 03:01

View 2 revisions

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.

~0014507

MjnMixael (manager)

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.

~0014511

CommanderDJ (developer)

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.

~0014513

CommanderDJ (developer)

Last edited: 2012-12-18 07:41

View 2 revisions

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.

~0014519

MjnMixael (manager)

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.

~0014538

Goober5000 (administrator)

Hang on. Why is the change to missionweaponchoice.cpp needed? That same line is present in the retail code.

~0015972

MjnMixael (manager)

Bump as I've just run into issues in this area. Any chance we can come to an agreement on how to handle it?

~0016029

Goober5000 (administrator)

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.

~0016035

MageKing17 (developer)

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.

~0016039

Goober5000 (administrator)

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...

~0016057

MageKing17 (developer)

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).

~0016058

CommanderDJ (developer)

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.

~0016078

Goober5000 (administrator)

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.

~0016079

MageKing17 (developer)

Last edited: 2014-07-16 21:34

View 3 revisions

I have no idea how the issue interacts with multiplayer at all.

EDIT: Updated patch to correct -1 issue, at least.

~0016080

Goober5000 (administrator)

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.

~0016081

MageKing17 (developer)

Last edited: 2014-07-16 22:38

View 3 revisions

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.

~0016082

Goober5000 (administrator)

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.

~0016084

MageKing17 (developer)

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.

~0016091

Goober5000 (administrator)

Looks good. Patch committed.

~0016092

Goober5000 (administrator)

Changed the ticket description to reflect the actual issue that was fixed by the commit.
+Notes

+Related Changesets

-Issue History
Date Modified Username Field Change
2010-03-16 07:31 The_E New Issue
2010-03-16 07:31 The_E Status new => assigned
2010-03-16 07:31 The_E Assigned To => Sushi_CW
2010-03-16 07:37 Fury Note Added: 0011782
2012-12-17 03:01 Goober5000 Note Added: 0014499
2012-12-17 03:01 Goober5000 Assigned To Sushi_CW =>
2012-12-17 03:01 Goober5000 Status assigned => confirmed
2012-12-17 03:01 Goober5000 Note Edited: 0014499 View Revisions
2012-12-17 23:05 MjnMixael File Added: 2153a.fs2
2012-12-17 23:05 MjnMixael File Added: 2153b.fs2
2012-12-17 23:09 MjnMixael Note Added: 0014507
2012-12-18 05:13 CommanderDJ Assigned To => CommanderDJ
2012-12-18 05:13 CommanderDJ Status confirmed => assigned
2012-12-18 07:15 CommanderDJ Note Added: 0014511
2012-12-18 07:34 CommanderDJ File Added: 2153.patch
2012-12-18 07:36 CommanderDJ Note Added: 0014513
2012-12-18 07:40 CommanderDJ File Deleted: 2153.patch
2012-12-18 07:40 CommanderDJ File Added: 2153.patch
2012-12-18 07:41 CommanderDJ Note Edited: 0014513 View Revisions
2012-12-18 14:40 MjnMixael Note Added: 0014519
2012-12-19 00:29 Goober5000 Note Added: 0014538
2014-06-30 12:03 MjnMixael Note Added: 0015972
2014-07-02 10:07 Goober5000 Target Version => 3.7.2
2014-07-06 23:40 Goober5000 Note Added: 0016029
2014-07-07 04:10 MageKing17 Note Added: 0016035
2014-07-07 20:11 Goober5000 Note Added: 0016039
2014-07-09 22:55 MageKing17 File Added: fredview.cpp.patch
2014-07-09 22:55 MageKing17 Assigned To CommanderDJ => MageKing17
2014-07-09 22:55 MageKing17 Status assigned => code review
2014-07-09 22:56 MageKing17 Note Added: 0016057
2014-07-09 22:58 MageKing17 File Deleted: fredview.cpp.patch
2014-07-09 22:58 MageKing17 File Added: fredview.cpp.patch
2014-07-09 23:03 CommanderDJ Note Added: 0016058
2014-07-16 21:24 Goober5000 Note Added: 0016078
2014-07-16 21:26 MageKing17 Note Added: 0016079
2014-07-16 21:33 MageKing17 File Deleted: fredview.cpp.patch
2014-07-16 21:33 MageKing17 File Added: fredview.cpp.patch
2014-07-16 21:33 MageKing17 Note Edited: 0016079 View Revisions
2014-07-16 21:34 MageKing17 Note Edited: 0016079 View Revisions
2014-07-16 22:12 Goober5000 Note Added: 0016080
2014-07-16 22:25 MageKing17 Note Added: 0016081
2014-07-16 22:34 MageKing17 File Deleted: fredview.cpp.patch
2014-07-16 22:34 MageKing17 File Added: fredview.cpp.patch
2014-07-16 22:34 MageKing17 Note Edited: 0016081 View Revisions
2014-07-16 22:37 MageKing17 File Deleted: fredview.cpp.patch
2014-07-16 22:37 MageKing17 File Added: fredview.cpp.patch
2014-07-16 22:38 MageKing17 Note Edited: 0016081 View Revisions
2014-07-16 23:28 Goober5000 Note Added: 0016082
2014-07-17 01:34 MageKing17 File Deleted: fredview.cpp.patch
2014-07-17 01:34 MageKing17 File Added: fredview.cpp.patch
2014-07-17 01:36 MageKing17 Note Added: 0016084
2014-07-17 22:48 Goober5000 Changeset attached => fs2open trunk r10924
2014-07-17 22:49 Goober5000 Note Added: 0016091
2014-07-17 22:49 Goober5000 Status code review => resolved
2014-07-17 22:49 Goober5000 Resolution open => fixed
2014-07-17 22:49 Goober5000 Fixed in Version => 3.7.2
2014-07-17 22:50 Goober5000 Note Added: 0016092
2014-07-17 22: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
+Issue History