View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0001098 | FSSCP | multiplayer | public | 2006-10-09 02:04 | 2008-10-23 04:46 |
Reporter | FUBAR-BDHR | Assigned To | taylor | ||
Priority | high | Severity | crash | Reproducibility | random |
Status | resolved | Resolution | fixed | ||
Fixed in Version | 3.6.10 | ||||
Summary | 0001098: Assert: (Weapon_info[Weapons[homing_object->instance].weapon_info_index].wi_flags & WIF_BOMB) || (Weapon_info[Weapons[homing_obj | ||||
Description | Well 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 | ||||
Tags | No tags attached. | ||||
2006-10-09 02:04
|
|
2006-10-09 02:19
|
|
|
Second time tonight this time in ravana rescue. |
2006-10-09 02:37
|
|
2006-10-13 22:29
|
|
|
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? |
|
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. |
|
weapon_home() seems to have a similar assertion Assert(Weapon_info[Weapons[hobjp->instance].weapon_info_index].wi_flags & WIF_BOMB); |
|
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. |
|
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. |
2008-07-01 01:14
|
|
2008-07-01 01:14
|
|
2008-09-12 19:07
|
|
|
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. |
|
Hopefully this can be sorted out when Kara or someone can take a look at it, or else bump it back down in priority. |
|
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. |
|
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). |
|
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. |
|
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? |
|
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. :) |
|
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. |
|
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. |
2008-10-19 14:46
|
1098.diff (5,183 bytes)
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 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. |
|
Looks good to me. |
|
Fixered. |
Date Modified | Username | Field | Change |
---|---|---|---|
2006-10-09 02:04 | FUBAR-BDHR | New Issue | |
2006-10-09 02:04 | FUBAR-BDHR | File Added: Media-10-08-06b.rar | |
2006-10-09 02:19 | FUBAR-BDHR | File Added: Media-10-08-06c.rar | |
2006-10-09 02:20 | FUBAR-BDHR | Note Added: 0006832 | |
2006-10-09 02:37 | FUBAR-BDHR | File Added: Media-10-08-06d.rar | |
2006-10-13 22:29 | FUBAR-BDHR | File Added: Media-10-13-06a.rar | |
2007-09-29 17:47 | karajorma | Note Added: 0008547 | |
2007-10-01 07:11 | FUBAR-BDHR | Note Added: 0008549 | |
2007-10-04 21:39 | karajorma | Note Added: 0008554 | |
2007-12-03 01:48 | karajorma | Assigned To | taylor => karajorma |
2007-12-03 01:52 | karajorma | Note Added: 0008722 | |
2008-07-01 01:13 | FUBAR-BDHR | Note Added: 0009428 | |
2008-07-01 01:14 | FUBAR-BDHR | File Added: Core2Quad-06-30-08a.rar | |
2008-07-01 01:14 | FUBAR-BDHR | File Added: Media-06-30-08a.rar | |
2008-09-12 19:07 | FUBAR-BDHR | File Added: fs2_open.log | |
2008-09-12 19:08 | FUBAR-BDHR | Note Added: 0009662 | |
2008-10-09 16:40 | chief1983 | Note Added: 0009896 | |
2008-10-09 16:40 | chief1983 | Priority | normal => high |
2008-10-09 19:44 | karajorma | Note Added: 0009905 | |
2008-10-18 18:56 | taylor | Note Added: 0010035 | |
2008-10-18 19:25 | FUBAR-BDHR | Note Added: 0010038 | |
2008-10-18 21:33 | taylor | Note Added: 0010039 | |
2008-10-18 23:40 | taylor | Note Added: 0010041 | |
2008-10-19 03:45 | taylor | Note Added: 0010042 | |
2008-10-19 07:50 | karajorma | Note Added: 0010045 | |
2008-10-19 14:46 | taylor | File Added: 1098.diff | |
2008-10-19 14:53 | taylor | Note Added: 0010047 | |
2008-10-20 07:40 | taylor | Assigned To | karajorma => taylor |
2008-10-20 19:18 | karajorma | Note Added: 0010067 | |
2008-10-23 04:46 | taylor | Status | assigned => resolved |
2008-10-23 04:46 | taylor | Fixed in Version | => 3.6.10 |
2008-10-23 04:46 | taylor | Resolution | open => fixed |
2008-10-23 04:46 | taylor | Note Added: 0010100 |