View Issue Details

IDProjectCategoryView StatusLast Update
0002604FSSCPgameplaypublic2012-05-02 11:35
Reporterm_m Assigned Tom_m  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.6.13 
Summary0002604: Corkscrew/Swarm firing process doesn't check the used missile bank
DescriptionWhen switching a corkscrew/swarm missile to another corkscrew/swarm missile while the first is still firing then second bank will fire the rest of the missiles which are left to be shot from the first weapon fire.
Additional InformationCan be fixed by saving the index of the weapon bank the current swarm was fired from and checking if the current weapon bank the the saved one are the same. See attached patch
TagsNo tags attached.

Activities

m_m

2012-02-11 10:38

developer  

corkscrewSwarm.patch (4,178 bytes)   
Index: code/ship/ship.cpp
===================================================================
--- code/ship/ship.cpp	(revision 8412)
+++ code/ship/ship.cpp	(working copy)
@@ -4964,6 +4964,10 @@
 
 	// corkscrew missile stuff
 	shipp->next_corkscrew_fire = 1;
+	
+	// Missile bank indexes to avoid firing different swarm/corkscrew missiles
+	shipp->swarm_misile_bank = -1;
+	shipp->corkscrew_missile_bank = -1;
 
 	// field for score
 	shipp->score = sip->score;
@@ -10645,6 +10649,7 @@
 		} else {
 			shipp->num_swarm_missiles_to_fire += wip->swarm_count;
 		}
+		shipp->swarm_misile_bank = shipp->weapons.current_secondary_bank;
 		return 1;		//	Note: Missiles didn't get fired, but the frame interval code will fire them.
 	}
 
@@ -10652,7 +10657,8 @@
 	if ( (wip->wi_flags & WIF_CORKSCREW) && !allow_swarm ) {
 		//phreak 11-9-02 
 		//changed this from 4 to custom number defined in tables
-		shipp->num_corkscrew_to_fire = (ubyte)(shipp->num_corkscrew_to_fire + (ubyte)wip->cs_num_fired);		
+		shipp->num_corkscrew_to_fire = (ubyte)(shipp->num_corkscrew_to_fire + (ubyte)wip->cs_num_fired);
+		shipp->corkscrew_missile_bank = shipp->weapons.current_secondary_bank;
 		return 1;		//	Note: Missiles didn't get fired, but the frame interval code will fire them.
 	}	
 
Index: code/ship/ship.h
===================================================================
--- code/ship/ship.h	(revision 8412)
+++ code/ship/ship.h	(working copy)
@@ -504,6 +504,7 @@
 	char targeting_laser_bank;						// -1 if not firing, index into polymodel gun points if it _is_ firing
 	// corkscrew missile stuff
 	ubyte num_corkscrew_to_fire;						// # of corkscrew missiles lef to fire
+	int corkscrew_missile_bank;
 	// END PACK
 
 	// targeting laser info
@@ -610,6 +611,7 @@
 	int	next_swarm_fire;					// timestamp of next swarm missile to fire
 	int	next_swarm_path;					// next path number for swarm missile to take
 	int	num_turret_swarm_info;			// number of turrets in process of launching swarm
+	int swarm_misile_bank;				// The missilebank the swarm was originally launched from
 
 	int	group;								// group ship is in, or -1 if none.  Fred thing
 	int	death_roll_snd;					// id of death roll sound, may need to be stopped early	
Index: code/weapon/corkscrew.cpp
===================================================================
--- code/weapon/corkscrew.cpp	(revision 8412)
+++ code/weapon/corkscrew.cpp	(working copy)
@@ -85,15 +85,25 @@
 	swp = &sp->weapons;
 	if ( swp->current_secondary_bank == -1 ) {
 		sp->num_corkscrew_to_fire = 0;
+		sp->corkscrew_missile_bank = -1;
 		return;
 	}
 
+	// If the missile bank is a different one then stop firing the corkscrew
+	if (swp->current_secondary_bank != sp->corkscrew_missile_bank)
+	{
+		sp->num_corkscrew_to_fire = 0;
+		sp->corkscrew_missile_bank = -1;
+		return;
+	}
+
 	weapon_info_index = swp->secondary_bank_weapons[swp->current_secondary_bank];
 	Assert( weapon_info_index >= 0 && weapon_info_index < MAX_WEAPON_TYPES );
 
 	// if current secondary bank is not a corkscrew missile, return
 	if ( !(Weapon_info[weapon_info_index].wi_flags & WIF_CORKSCREW) ) {
 		sp->num_corkscrew_to_fire = 0;
+		sp->corkscrew_missile_bank = -1;
 		return;
 	}
 
Index: code/weapon/swarm.cpp
===================================================================
--- code/weapon/swarm.cpp	(revision 8412)
+++ code/weapon/swarm.cpp	(working copy)
@@ -102,15 +102,25 @@
 	swp = &sp->weapons;
 	if ( swp->current_secondary_bank == -1 ) {
 		sp->num_swarm_missiles_to_fire = 0;
+		sp->swarm_misile_bank = -1;
 		return;
 	}
 
+	// If the missile bank is a different one then stop firing the corkscrew
+	if (swp->current_secondary_bank != sp->swarm_misile_bank)
+	{
+		sp->num_swarm_missiles_to_fire = 0;
+		sp->swarm_misile_bank = -1;
+		return;
+	}
+
 	weapon_info_index = swp->secondary_bank_weapons[swp->current_secondary_bank];
 	Assert( weapon_info_index >= 0 && weapon_info_index < MAX_WEAPON_TYPES );
 
 	// if current secondary bank is not a swarm missile, return
 	if ( !(Weapon_info[weapon_info_index].wi_flags & WIF_SWARM) ) {
 		sp->num_swarm_missiles_to_fire = 0;
+		sp->swarm_misile_bank = -1;
 		return;
 	}
 
corkscrewSwarm.patch (4,178 bytes)   

niffiwan

2012-02-24 02:30

developer   ~0013347

m_m, I had a look at the patch and it seems to work as intended. However, I was wondering if we should aim for a slightly different outcome here. Currently when you switch banks the corkscrew/swarm missiles stop firing. Would it be worthwhile changing this so that either

1) the bank keeps firing until it was out of missiles, or a new corkscrew/swarm bank started firing

2) the bank keeps firing regardless, so in theory you could have three banks (is the #define MAX_BANKS or something?) of corkscrew/swarm missiles firing at once - I thought that would look cool :D, but you'd need to store the firing state of each bank separately.

What do you think?

m_m

2012-02-24 08:01

developer   ~0013351

This actually sounds like a new feature but it is definitely worth taking a look at. The second solution would be very cool but you will have to be able to control the maximum number of swarm/corkscrew missiles that are allowed to fire simultaneously to preserver the retail behavior of only one one bank firing.
When doing that the firing state of one swarm/corkscrew missile should actually be stored in its own struct or class to make working with it easier so we don't have 3 or 6 individual arrays.

niffiwan

2012-02-27 09:08

developer   ~0013362

To preserve retail behavior, would we use an optional flag in the ship.tbl? Something like "$Max Secondary Bank Fire"? This would default to one if not specified.

m_m

2012-02-27 10:29

developer   ~0013363

Yes, that was exactly what I thought.

niffiwan

2012-03-04 04:19

developer   ~0013378

I used your patch as a base to implement option 1 from above as a prerequisite for implementing option 2. Although, with my testing so far it seems that it'll be quite hard to actually get three banks firing at once anyway (unless you table the missiles to fire a ridiculous number at once, or if they have a large $Swarm Wait). With default hornets and this patch, when I switch banks and fire again, the 1st bank usually gets through firing 6 of 8 missiles anyway before the next bank starts firing. I think there's a fire delay when switching banks that's getting in the way somewhere :-)

Anyway - I'm keen to receive some feedback on this. Thanks!

Echelon9

2012-03-04 12:54

developer   ~0013381

Niffiwan, something is going odd with your most recent patch file.
It's missing the revision number, which I think is related to the fact the diff is being done from a folder *above* the top level SVN checkout.

Could you reproduce the diff and I'll take a look.

niffiwan

2012-03-04 21:09

developer   ~0013387

Last edited: 2012-03-05 10:21

Ah yes - the joys of using git with svn :-) My git-to-svn patch conversion script must have stuffed up, I'll redo the patch tonight.

EDIT: new patch attached, this one has the correct revision number

niffiwan

2012-03-05 10:18

developer  

corkscrewSwarmOneBankContinuesFiring.patch (7,914 bytes)   
Index: fs2_open/code/ship/ship.cpp
===================================================================
--- fs2_open/code/ship/ship.cpp    (revision 8574)
+++ fs2_open/code/ship/ship.cpp    (working copy)
@@ -4969,6 +4969,10 @@ void ship_set(int ship_index, int objnum, int ship_type)
 	// corkscrew missile stuff
 	shipp->next_corkscrew_fire = 1;
 
+	// Missile bank indexes to avoid firing different swarm/corkscrew missiles
+	shipp->swarm_missile_bank = -1;
+	shipp->corkscrew_missile_bank = -1;
+
 	// field for score
 	shipp->score = sip->score;
 
@@ -10580,8 +10584,16 @@ int ship_fire_secondary( object *obj, int allow_swarm )
 
 	num_fired = 0;		// tracks how many missiles actually fired
 
-	bank = swp->current_secondary_bank;
-	if ( bank < 0 ) {
+	// niffiwan: allow swarm/corkscrew bank to keep firing if current bank changes
+	if (shipp->swarm_missile_bank != -1 && allow_swarm) {
+		bank = shipp->swarm_missile_bank;
+	} else if (shipp->corkscrew_missile_bank != -1 && allow_swarm) {
+		bank = shipp->corkscrew_missile_bank;
+	} else {
+		bank = swp->current_secondary_bank;
+	}
+
+	if ( bank < 0 || bank > sip->num_secondary_banks ) {
 		return 0;
 	}
 
@@ -10624,7 +10636,8 @@ int ship_fire_secondary( object *obj, int allow_swarm )
 		return 0;
 	}
 
-	if ( swp->current_secondary_bank < 0 ){
+	// niffiwan: 04/03/12: duplicate of a check approx 100 lines above - not needed?
+	if ( bank < 0 ){
 		return 0;
 	}
 
@@ -10702,10 +10715,11 @@ int ship_fire_secondary( object *obj, int allow_swarm )
 	if ( (wip->wi_flags & WIF_SWARM) && !allow_swarm ) {
 		Assert(wip->swarm_count > 0);
 		if(wip->swarm_count <= 0){
-			shipp->num_swarm_missiles_to_fire += SWARM_DEFAULT_NUM_MISSILES_FIRED;
+			shipp->num_swarm_missiles_to_fire = SWARM_DEFAULT_NUM_MISSILES_FIRED;
 		} else {
-			shipp->num_swarm_missiles_to_fire += wip->swarm_count;
+			shipp->num_swarm_missiles_to_fire = wip->swarm_count;
 		}
+		shipp->swarm_missile_bank = bank;
 		return 1;		//	Note: Missiles didn't get fired, but the frame interval code will fire them.
 	}
 
@@ -10713,7 +10727,8 @@ int ship_fire_secondary( object *obj, int allow_swarm )
 	if ( (wip->wi_flags & WIF_CORKSCREW) && !allow_swarm ) {
 		//phreak 11-9-02 
 		//changed this from 4 to custom number defined in tables
-		shipp->num_corkscrew_to_fire = (ubyte)(shipp->num_corkscrew_to_fire + (ubyte)wip->cs_num_fired);		
+		shipp->num_corkscrew_to_fire = (ubyte)(shipp->num_corkscrew_to_fire + (ubyte)wip->cs_num_fired);
+		shipp->corkscrew_missile_bank = bank;
 		return 1;		//	Note: Missiles didn't get fired, but the frame interval code will fire them.
 	}	
 
@@ -10875,8 +10890,8 @@ int ship_fire_secondary( object *obj, int allow_swarm )
 		if ( Weapon_info[weapon].launch_snd != -1 ) {
 			snd_play( &Snds[Weapon_info[weapon].launch_snd], 0.0f, 1.0f, SND_PRIORITY_MUST_PLAY );
 			swp = &Player_ship->weapons;
-			if (swp->current_secondary_bank >= 0) {
-				wip = &Weapon_info[swp->secondary_bank_weapons[swp->current_secondary_bank]];
+			if (bank >= 0) {
+				wip = &Weapon_info[swp->secondary_bank_weapons[bank]];
 				if (Player_ship->flags & SF_SECONDARY_DUAL_FIRE){
 					joy_ff_play_secondary_shoot((int) (wip->cargo_size * 2.0f));
 				} else {
@@ -10933,10 +10948,12 @@ done_secondary:
 
 		if (shipp->num_swarm_missiles_to_fire > 1) {
 			shipp->num_swarm_missiles_to_fire = 1;
+			shipp->swarm_missile_bank = -1;
 		}
 
 		if (shipp->num_corkscrew_to_fire > 1) {
 			shipp->num_corkscrew_to_fire = 1;
+			shipp->corkscrew_missile_bank = -1;
 		}
 	}
 
@@ -10954,8 +10971,8 @@ done_secondary:
 	//then it would have no firedelay. and then add 250 ms of delay. in effect, this way there is no penalty if there is any firedelay remaning in
 	//the next valid bank. the delay is there to prevent things like Trible/Quad Fire Trebuchets.
 	//
-	// niffiwan: only try to switch banks if object has multiple banks
-	if ( (obj->flags & OF_PLAYER_SHIP) && (swp->secondary_bank_ammo[bank] <= 0) && (swp->num_secondary_banks >= 2) ) {
+	// niffiwan: only try to switch banks if object has multiple banks, and firing bank is the current bank
+	if ( (obj->flags & OF_PLAYER_SHIP) && (swp->secondary_bank_ammo[bank] <= 0) && (swp->num_secondary_banks >= 2) && (bank == swp->current_secondary_bank) ) {
 		// niffiwan: call ship_select_next_secondary instead of ship_select_next_valid_secondary_bank
 		// ensures all "extras" are dealt with, like animations, scripting hooks, etc
 		if (ship_select_next_secondary(obj) ) {			//DTP here we switch to the next valid bank, but we can't call weapon_info on next fire_wait
Index: fs2_open/code/ship/ship.h
===================================================================
--- fs2_open/code/ship/ship.h    (revision 8574)
+++ fs2_open/code/ship/ship.h    (working copy)
@@ -508,6 +508,7 @@ typedef struct ship {
 	char targeting_laser_bank;						// -1 if not firing, index into polymodel gun points if it _is_ firing
 	// corkscrew missile stuff
 	ubyte num_corkscrew_to_fire;						// # of corkscrew missiles lef to fire
+	int corkscrew_missile_bank;
 	// END PACK
 
 	// targeting laser info
@@ -614,6 +615,7 @@ typedef struct ship {
 	int	next_swarm_fire;					// timestamp of next swarm missile to fire
 	int	next_swarm_path;					// next path number for swarm missile to take
 	int	num_turret_swarm_info;			// number of turrets in process of launching swarm
+	int swarm_missile_bank;				// The missilebank the swarm was originally launched from
 
 	int	group;								// group ship is in, or -1 if none.  Fred thing
 	int	death_roll_snd;					// id of death roll sound, may need to be stopped early	
Index: fs2_open/code/weapon/corkscrew.cpp
===================================================================
--- fs2_open/code/weapon/corkscrew.cpp    (revision 8574)
+++ fs2_open/code/weapon/corkscrew.cpp    (working copy)
@@ -78,22 +78,24 @@ void cscrew_maybe_fire_missile(int shipnum)
 
 	// make sure we're supposed to be firing some missiles
 	if ( sp->num_corkscrew_to_fire <= 0 ){
+		sp->corkscrew_missile_bank = -1;
 		return;
 	}
 
-	// make sure we have a valid weapon band
+	// make sure we have a valid weapon bank
 	swp = &sp->weapons;
-	if ( swp->current_secondary_bank == -1 ) {
+	if ( sp->corkscrew_missile_bank == -1 ) {
 		sp->num_corkscrew_to_fire = 0;
 		return;
 	}
 
-	weapon_info_index = swp->secondary_bank_weapons[swp->current_secondary_bank];
+	weapon_info_index = swp->secondary_bank_weapons[sp->corkscrew_missile_bank];
 	Assert( weapon_info_index >= 0 && weapon_info_index < MAX_WEAPON_TYPES );
 
 	// if current secondary bank is not a corkscrew missile, return
 	if ( !(Weapon_info[weapon_info_index].wi_flags & WIF_CORKSCREW) ) {
 		sp->num_corkscrew_to_fire = 0;
+		sp->corkscrew_missile_bank = -1;
 		return;
 	}
 
Index: fs2_open/code/weapon/swarm.cpp
===================================================================
--- fs2_open/code/weapon/swarm.cpp    (revision 8574)
+++ fs2_open/code/weapon/swarm.cpp    (working copy)
@@ -96,21 +96,24 @@ void swarm_maybe_fire_missile(int shipnum)
 	Assert(shipnum >= 0 && shipnum < MAX_SHIPS );
 	sp = &Ships[shipnum];
 
-	if ( sp->num_swarm_missiles_to_fire <= 0 )
+	if ( sp->num_swarm_missiles_to_fire <= 0 ) {
+		sp->swarm_missile_bank = -1;
 		return;
+	}
 
 	swp = &sp->weapons;
-	if ( swp->current_secondary_bank == -1 ) {
+	if ( sp->swarm_missile_bank == -1 ) {
 		sp->num_swarm_missiles_to_fire = 0;
 		return;
 	}
 
-	weapon_info_index = swp->secondary_bank_weapons[swp->current_secondary_bank];
+	weapon_info_index = swp->secondary_bank_weapons[sp->swarm_missile_bank];
 	Assert( weapon_info_index >= 0 && weapon_info_index < MAX_WEAPON_TYPES );
 
-	// if current secondary bank is not a swarm missile, return
+	// if swarm secondary bank is not a swarm missile, return
 	if ( !(Weapon_info[weapon_info_index].wi_flags & WIF_SWARM) ) {
 		sp->num_swarm_missiles_to_fire = 0;
+		sp->swarm_missile_bank = -1;
 		return;
 	}
 

Echelon9

2012-03-05 12:01

developer   ~0013390

Looks good niffiwan. Gets a tick from me.

niffiwan

2012-03-06 10:08

developer   ~0013396

Fix committed to trunk@8585.

niffiwan

2012-05-02 11:35

developer   ~0013502

Fix committed to fs2_open_3_6_14@8718.

Related Changesets

fs2open: trunk r8585

2012-03-06 05:09

niffiwan


Ported: N/A

Details Diff
Fix for Mantis 0002604: using m_m's patch, allow swarm/corkscrew bank to keep firing if current bank changes Affected Issues
0002604
mod - /trunk/fs2_open/code/weapon/swarm.cpp Diff File
mod - /trunk/fs2_open/code/weapon/corkscrew.cpp Diff File
mod - /trunk/fs2_open/code/ship/ship.h Diff File
mod - /trunk/fs2_open/code/ship/ship.cpp Diff File

fs2open: fs2_open_3_6_14 r8718

2012-05-02 07:36

niffiwan


Ported: N/A

Details Diff
Backport: Trunk r8585; Fix for Mantis 0002604: using m_m's patch, allow swarm/corkscrew bank to keep firing if current bank changes Affected Issues
0002604
mod - /branches/fs2_open_3_6_14/code/weapon/swarm.cpp Diff File
mod - /branches/fs2_open_3_6_14/code/weapon/corkscrew.cpp Diff File
mod - /branches/fs2_open_3_6_14/code/ship/ship.h Diff File
mod - /branches/fs2_open_3_6_14/code/ship/ship.cpp Diff File

Issue History

Date Modified Username Field Change
2012-02-11 10:38 m_m New Issue
2012-02-11 10:38 m_m File Added: corkscrewSwarm.patch
2012-02-20 14:49 m_m Assigned To => m_m
2012-02-20 14:49 m_m Status new => assigned
2012-02-20 14:49 m_m Status assigned => code review
2012-02-24 02:30 niffiwan Note Added: 0013347
2012-02-24 08:01 m_m Note Added: 0013351
2012-02-27 09:08 niffiwan Note Added: 0013362
2012-02-27 10:29 m_m Note Added: 0013363
2012-03-04 04:13 niffiwan File Added: corkscrewSwarmOneBankContinuesFiring.patch
2012-03-04 04:19 niffiwan Note Added: 0013378
2012-03-04 12:54 Echelon9 Note Added: 0013381
2012-03-04 21:09 niffiwan Note Added: 0013387
2012-03-05 10:18 niffiwan File Deleted: corkscrewSwarmOneBankContinuesFiring.patch
2012-03-05 10:18 niffiwan File Added: corkscrewSwarmOneBankContinuesFiring.patch
2012-03-05 10:20 niffiwan Note Edited: 0013387
2012-03-05 10:21 niffiwan Note Edited: 0013387
2012-03-05 12:01 Echelon9 Note Added: 0013390
2012-03-06 10:08 niffiwan Changeset attached => fs2open trunk r8585
2012-03-06 10:08 niffiwan Note Added: 0013396
2012-03-06 10:08 niffiwan Status code review => resolved
2012-03-06 10:08 niffiwan Resolution open => fixed
2012-05-02 11:35 niffiwan Changeset attached => fs2open fs2_open_3_6_14 r8718
2012-05-02 11:35 niffiwan Note Added: 0013502