2018-02-18 23:37 EST


View Issue Details Jump to Notes ]
IDProjectCategoryView StatusLast Update
0001098FSSCPmultiplayerpublic2008-10-23 00:46
ReporterFUBAR-BDHR 
Assigned Totaylor 
PriorityhighSeveritycrashReproducibilityrandom
StatusresolvedResolutionfixed 
Product Version 
Target VersionFixed in Version3.6.10 
Summary0001098: Assert: (Weapon_info[Weapons[homing_object->instance].weapon_info_index].wi_flags & WIF_BOMB) || (Weapon_info[Weapons[homing_obj
DescriptionWell that was cut off. Full message is: Assert: (Weapon_info[Weapons[homing_object->instance].weapon_info_index].wi_flags & WIF_BOMB) || (Weapon_info[Weapons[homing_object->instance].weapon_info_index].wi_flags & WIF_CMEASURE)
Second time I've got this error. Might be related to error report 773 that is marked fixed. Received while client during a RI
TagsNo tags attached.
Attached Files
  • rar file icon Media-10-08-06b.rar (26,996 bytes) 2006-10-08 22:04
  • rar file icon Media-10-08-06c.rar (26,382 bytes) 2006-10-08 22:19
  • rar file icon Media-10-08-06d.rar (29,140 bytes) 2006-10-08 22:37
  • rar file icon Media-10-13-06a.rar (45,518 bytes) 2006-10-13 18:29
  • rar file icon Core2Quad-06-30-08a.rar (20,866 bytes) 2008-06-30 21:14
  • rar file icon Media-06-30-08a.rar (13,625 bytes) 2008-06-30 21:14
  • log file icon fs2_open.log (66,102 bytes) 2008-09-12 15:07
  • diff file icon 1098.diff (5,183 bytes) 2008-10-19 10:46 -
    Index: code/network/multimsgs.cpp
    ===================================================================
    --- code/network/multimsgs.cpp	(revision 4893)
    +++ code/network/multimsgs.cpp	(working copy)
    @@ -7774,6 +7786,9 @@
     	if ( !(Weapon_info[wp->weapon_info_index].wi_flags & WIF_HOMING) )
     		return;
     
    +	// default the subsystem
    +	t_subsys = -1;
    +
     	// get the homing signature.  If this weapon isn't homing on anything, then sent 0 as the
     	// homing signature.
     	homing_signature = 0;
    @@ -7782,7 +7797,6 @@
     		homing_signature = homing_object->net_signature;
     
     		// get the subsystem index.
    -		t_subsys = -1;
     		if ( (homing_object->type == OBJ_SHIP) && (wp->homing_subsys != NULL) ) {
     			int s_index;
     
    @@ -7835,7 +7849,14 @@
     	}
     
     	if ( homing_object->type == OBJ_WEAPON ) {
    -		Assert((Weapon_info[Weapons[homing_object->instance].weapon_info_index].wi_flags & WIF_BOMB) || (Weapon_info[Weapons[homing_object->instance].weapon_info_index].wi_flags & WIF_CMEASURE));
    +		int flags = Weapon_info[Weapons[homing_object->instance].weapon_info_index].wi_flags;
    +
    +	//	Assert( (flags & WIF_BOMB) || (flags & WIF_CMEASURE) );
    +
    +		if ( !((flags & WIF_BOMB) || (flags & WIF_CMEASURE)) ) {
    +			nprintf(("Network", "Homing object is invalid for homing update\n"));
    +			return;
    +		}
     	}
     
     	wp->homing_object = homing_object;
    @@ -8219,7 +8240,11 @@
     	if((Player_obj != NULL) && (Player_obj == objp)){		
     		return;
     	}
    -		
    +
    +	if ( (rand_val >= NPERM_SIG_MIN) && (rand_val <= NPERM_SIG_MAX) ) {
    +		multi_set_network_signature((ushort)rand_val, MULTI_SIG_NON_PERMANENT);
    +	}
    +
     	// make it so ship can fire right away!
     	Ships[objp->instance].cmeasure_fire_stamp = timestamp(0);
     	if ( objp == Player_obj ){		
    Index: code/network/multiutil.cpp
    ===================================================================
    --- code/network/multiutil.cpp	(revision 4893)
    +++ code/network/multiutil.cpp	(working copy)
    @@ -501,7 +501,7 @@
     	// as are debris and asteroids since they don't die very often.  It would be vary rare for this
     	// value (the permanent signature) to wrap.  For now, this condition is an error condition
     	if ( what_kind == MULTI_SIG_SHIP ) {
    -		if ( Next_ship_signature == 0 ){
    +		if ( Next_ship_signature < SHIP_SIG_MIN ){
     			Next_ship_signature = SHIP_SIG_MIN;
     		}
     
    @@ -514,7 +514,7 @@
     
     		// signature stuff for asteroids.
     	} else if ( what_kind == MULTI_SIG_ASTEROID ) {
    -		if ( Next_asteroid_signature == 0 ){
    +		if ( Next_asteroid_signature < ASTEROID_SIG_MIN ){
     			Next_asteroid_signature = ASTEROID_SIG_MIN;
     		}
     
    @@ -526,7 +526,7 @@
     
     		// signatures for debris
     	} else if ( what_kind == MULTI_SIG_DEBRIS ) {
    -		if ( Next_debris_signature == 0 ){
    +		if ( Next_debris_signature < DEBRIS_SIG_MIN ){
     			Next_debris_signature = DEBRIS_SIG_MIN;
     		}
     
    @@ -538,12 +538,12 @@
     
     		// signature stuff for weapons and other expendable things.
     	} else if ( what_kind == MULTI_SIG_NON_PERMANENT ) {
    -		if ( Next_non_perm_signature == 0 ){
    +		if ( Next_non_perm_signature < NPERM_SIG_MIN ){
     			Next_non_perm_signature = NPERM_SIG_MIN;
     		}
     
     		sig = Next_non_perm_signature++;
    -		if ( Next_non_perm_signature == NPERM_SIG_MAX ){
    +		if ( (Next_non_perm_signature < NPERM_SIG_MIN) || (Next_non_perm_signature == NPERM_SIG_MAX) ) {
     			Next_non_perm_signature = NPERM_SIG_MIN;
     		}
     	} else {
    @@ -559,22 +559,22 @@
     ushort multi_get_next_network_signature( int what_kind )
     {
     	if ( what_kind == MULTI_SIG_SHIP ) {
    -		if ( Next_ship_signature == 0 )
    +		if ( Next_ship_signature < SHIP_SIG_MIN )
     			Next_ship_signature = SHIP_SIG_MIN;
     		return Next_ship_signature;
     
     	} else if ( what_kind == MULTI_SIG_DEBRIS ) {
    -		if ( Next_debris_signature == 0 )
    +		if ( Next_debris_signature < DEBRIS_SIG_MIN)
     			Next_debris_signature = DEBRIS_SIG_MIN;
     		return Next_debris_signature;
     
     	} else if ( what_kind == MULTI_SIG_ASTEROID ) {
    -		if ( Next_asteroid_signature == 0 )
    +		if ( Next_asteroid_signature < ASTEROID_SIG_MIN )
     			Next_asteroid_signature = ASTEROID_SIG_MIN;
     		return Next_asteroid_signature;
     
     	} else if ( what_kind == MULTI_SIG_NON_PERMANENT ) {
    -		if ( Next_non_perm_signature == 0 )
    +		if ( Next_non_perm_signature < NPERM_SIG_MIN )
     			Next_non_perm_signature = NPERM_SIG_MIN;
     		return Next_non_perm_signature;
     
    @@ -600,7 +600,7 @@
     		Assert( (signature >= ASTEROID_SIG_MIN) && (signature <= ASTEROID_SIG_MAX) );
     		Next_asteroid_signature = signature;
     	} else if ( what_kind == MULTI_SIG_NON_PERMANENT ) {
    -		Assert( (signature >= NPERM_SIG_MIN) && (signature <= NPERM_SIG_MAX) );
    +		Assert( (signature >= NPERM_SIG_MIN) /*&& (signature <= NPERM_SIG_MAX)*/ );
     		Next_non_perm_signature = signature;
     	} else
     		Int3();			// get Allender
    Index: code/ship/ship.cpp
    ===================================================================
    --- code/ship/ship.cpp	(revision 4893)
    +++ code/ship/ship.cpp	(working copy)
    @@ -10043,7 +10042,7 @@
     		// the new way of doing things
     		// if(Netgame.debug_flags & NETD_FLAG_CLIENT_FIRING){
     		if(Game_mode & GM_MULTIPLAYER){
    -			send_NEW_countermeasure_fired_packet( objp, cmeasure_count, arand );
    +			send_NEW_countermeasure_fired_packet( objp, cmeasure_count, /*arand*/Objects[cobjnum].net_signature );
     		}
     	}
     
    
    diff file icon 1098.diff (5,183 bytes) 2008-10-19 10:46 +

-Relationships
+Relationships

-Notes

~0006832

FUBAR-BDHR (developer)

Second time tonight this time in ravana rescue.

~0008547

karajorma (administrator)

I've just seen this one while playing Rebels & Renegades in multiplayer. At first glance it seems to me like someone marked bombs and countermeasures as homing weapons but missed out that missiles can home too.

Right?

~0008549

FUBAR-BDHR (developer)

It's been so long since I've played FS2 or any game for that matter I can't remember what the circumstances behind this were.

~0008554

karajorma (administrator)

weapon_home() seems to have a similar assertion

Assert(Weapon_info[Weapons[hobjp->instance].weapon_info_index].wi_flags & WIF_BOMB);

~0008722

karajorma (administrator)

Hmmmmm. Strange. If I add that assertion to the packet building function it won't assert. Looks like the packet is leaving the server fine but is being built up again incorrectly.

I'll have to look into this further.

~0009428

FUBAR-BDHR (developer)

Was doing some testing with the Scoring-Changes build and received this a couple of times. Saved the logs from both the server and client side.

~0009662

FUBAR-BDHR (developer)

Uploaded a new log file from Kara's TeamChatBuild.7z. Looks like some more debug stuff was added so I hope this helps. Was hosting on a standalone. Standalone did not crash.

~0009896

chief1983 (administrator)

Hopefully this can be sorted out when Kara or someone can take a look at it, or else bump it back down in priority.

~0009905

karajorma (administrator)

This one is causing me real problems because the packet seems to be leaving the server fine but by the time it reaches the client the net_signature is invalid.

I will try to take another look at it soon.

~0010035

taylor (administrator)

The net_signature isn't really invalid, it is just assigned to a different object. Every time this Assert() gets hit it's when it's trying to home in on a cmeasure. This really goes back to the fact that counter measures were never valid network objects in the first place. When they were converted to be weapons they started getting valid signatures, but that opens it up to all sorts of problems.

I haven't confirmed it yet, but I think it is due specifically to lifetime of the cmeasure being so short. You can end up with the weapon net signatures getting out of sync. I'm going to attempt to test that out later tonight, but it's going to be difficult to say whether it will be 100% fixed or not. The fact that the cmeasures don't always cause this indicates that it's not lifetime that's the problem, but there is still some situation that causes the signatures to get out of sync, so we just have to start ruling things out.

The Assert() just needs to be followed up with a check to make sure that the game doesn't further pass an invalid object through. This is to prevent badness in weapon_home() (as noted by the Assert() it tends to hit there).

~0010038

FUBAR-BDHR (developer)

I remember back in retail there was a bug with dual fired Cycs and Helios. If one of the pair was shot down both would disappear from the client side. Been meaning to see if this still exists. Anyway would something like that cause this? Basically a bomb that still exists on the host side but the client doesn't register it as there anymore? He may be able to see it (can't remember the details) but can't shoot it or anything. I did noticed the missions where this happened the most would tend to have a lot of bombs to shoot down. Of course they have a lot of everything else too.

Again I don't even know if that bug still exists in FS2_Open. Just that WIF_BOMB makes me think of it every time I see it.

~0010039

taylor (administrator)

It shouldn't be related. This bug is caused specifically by the new cmeasure code.

I gave the lifetime thing a check, and ruled it out. But I did figure out that indeed the signatures get out of sync, and it appears to be specifically because of the cmeasures. It appears that when a weapon is typically triggered through the network, the server adds the signature to the packet, then the clients will sync that signature before creating the weapon on their end. This makes sure that the weapon should have the same signature on the client that the host has for it. The old cmeasure code didn't give valid signatures to their objects since there wasn't any need to ever send it over the network (there was just a fired/fire packet, very basic). The new code treats it as a real weapon however, which means that it needs to keep track of the signatures now.

The only way that I see if fix this properly is to bump the server/client version for multi and fix the packet to send the signature to sync with. The alternative fix is to create a new packet for cmeasures, but this will still ultimately break older clients since it still won't sync weapon signatures properly (as both client and host must have this fix in order for it to work). The alternative really isn't a fix in other words, it just addresses the cause while ignoring the result.

Do you see another way to deal with this karajorma?

~0010041

taylor (administrator)

I think that I might have figured out a way to fix this without breaking anything, without needing a version bump, and without requiring a new packet. You would still need updated builds for client and host in order for the fix to work, but the fix wouldn't break older builds, and older builds wouldn't hurt newer builds.

It might be tomorrow before I can actually code it up and test it out, but if it works then I think that it should effectively solve our problem. :)

~0010042

taylor (administrator)

Ok, my initial tests show that it is fixed, but not 100%. The cmeasure signatures do stay in sync across host and client, but there are rare instances of duplicate signatures which can cause the same Assert() due to it getting the wrong object. This duplicate signature issue seems to be due to the fact that primary weapons also don't have signature syncing. Secondary weapons do sync however, and now that cmeasures do as well, the various objects are kept in sync overall if not entirely.

I believe that the duplicate signature problem was there in retail as well, but since secondary weapons don't typically target other weapons, it is something that rarely becomes an issue. So, I guess we should just accept it as-is for now and maybe have another look at fixing the duplicate sig problem when we really decided to break the netcode compatibility. For now I think that we should just go with this fix, remove the Assert(), and handle the case with an early return instead for the rare instances that still require it.

These changes will be in my next multi_test build, so if the testers sign off on it, and no devs disagree, then I'll commit at some point next week.

~0010045

karajorma (administrator)

If you can post a diff of the change then I can take a look to see if I can see another way past the problem.

~0010047

taylor (administrator)

Diff attached.

The fix is to use the rand_val int that it sends over with the cmeasure packet to store the net_signature. The actual value of rand_val doesn't matter since it's use as a random value for velocity and lifetime values that can be identical between client and host. The value is just an objnum, which means that it is always lower than MAX_OBJECTS. This allows us to get away with setting it to a larger number (the signature, which is a minimum of 32768) to indicate the fix to new clients, and older clients don't really care what the value is so long as it's >= -1.

The changes to multiutil.cpp are just to fix some bad checks that V made for the signature values and isn't directly related to the fix for this bug.

~0010067

karajorma (administrator)

Looks good to me.

~0010100

taylor (administrator)

Fixered.
+Notes

-Issue History
Date Modified Username Field Change
2006-10-08 22:04 FUBAR-BDHR New Issue
2006-10-08 22:04 FUBAR-BDHR File Added: Media-10-08-06b.rar
2006-10-08 22:19 FUBAR-BDHR File Added: Media-10-08-06c.rar
2006-10-08 22:20 FUBAR-BDHR Note Added: 0006832
2006-10-08 22:37 FUBAR-BDHR File Added: Media-10-08-06d.rar
2006-10-13 18:29 FUBAR-BDHR File Added: Media-10-13-06a.rar
2007-09-29 13:47 karajorma Note Added: 0008547
2007-10-01 03:11 FUBAR-BDHR Note Added: 0008549
2007-10-04 17:39 karajorma Note Added: 0008554
2007-12-02 20:48 karajorma Assigned To taylor => karajorma
2007-12-02 20:52 karajorma Note Added: 0008722
2008-06-30 21:13 FUBAR-BDHR Note Added: 0009428
2008-06-30 21:14 FUBAR-BDHR File Added: Core2Quad-06-30-08a.rar
2008-06-30 21:14 FUBAR-BDHR File Added: Media-06-30-08a.rar
2008-09-12 15:07 FUBAR-BDHR File Added: fs2_open.log
2008-09-12 15:08 FUBAR-BDHR Note Added: 0009662
2008-10-09 12:40 chief1983 Note Added: 0009896
2008-10-09 12:40 chief1983 Priority normal => high
2008-10-09 15:44 karajorma Note Added: 0009905
2008-10-18 14:56 taylor Note Added: 0010035
2008-10-18 15:25 FUBAR-BDHR Note Added: 0010038
2008-10-18 17:33 taylor Note Added: 0010039
2008-10-18 19:40 taylor Note Added: 0010041
2008-10-18 23:45 taylor Note Added: 0010042
2008-10-19 03:50 karajorma Note Added: 0010045
2008-10-19 10:46 taylor File Added: 1098.diff
2008-10-19 10:53 taylor Note Added: 0010047
2008-10-20 03:40 taylor Assigned To karajorma => taylor
2008-10-20 15:18 karajorma Note Added: 0010067
2008-10-23 00:46 taylor Status assigned => resolved
2008-10-23 00:46 taylor Fixed in Version => 3.6.10
2008-10-23 00:46 taylor Resolution open => fixed
2008-10-23 00:46 taylor Note Added: 0010100
+Issue History