[Valathil]

Guys this is great story for any programmer. I turned compiler optimization on in debug to get accurate profiling readings and a strange warning message popped up.

Code: [Select]

Weapon 'Harpoon#Weak' requires the "player allowed" flag, but it's not listed!  Adding it by default.
Offending code:

Code: [Select]

if ( !(wip->wi_flags & WIF_PLAYER_ALLOWED) && stristr(wip->name, "#weak") ) {
            int idx = -1;
            char non_weak[NAME_LENGTH];

            strncpy(non_weak, wip->name, strlen(wip->name) - 5);
            idx = weapon_info_lookup(non_weak);

            // only add the flag if the non-weak version is also player-allowed
            if ( (idx >= 0) && (Weapon_info[idx].wi_flags & WIF_PLAYER_ALLOWED) ) {
                Warning(LOCATION, "Weapon '%s' requires the \"player allowed\" flag, but it's not listed!  Adding it by default.\n", wip->name);
                wip->wi_flags |= WIF_PLAYER_ALLOWED;
            }
        }
So i investigate and found something really awesome. A heisenbug! Means a bug that disappears when you try to debug it. strncpy doesnt terminate a string by itself it only does it when the to be copied string ends before the number of chars given is reached. So when it passes the non_weak string to find the nonweak weapon its unterminated. It doesnt find the weapon and no warning is generated. But when i turn on optimizations something awesome happens. By a one in a million fluke there is a 0 in memory right where the string "Harpoon" ends, its terminated correctly it finds the weapon and bam a warning is generated. So i patched the function and looky look there pop up 3 more warnings. So what should i do about that do we fix the tables or just surpress the warning. i dont even know why you want to enable player allowed if its weak wouldnt this show in the weapon name would look pretty stupid if you ask me. It didnt appear till now cause there was the bug in the check function. here is the patch for anyone interested

Code: [Select]

Index: weapons.cpp
===================================================================
--- weapons.cpp (Revision 7344)
+++ weapons.cpp (Arbeitskopie)
@@ -3332,11 +3332,13 @@
  first_cmeasure_index = i;
 
  // if we are a "#weak" weapon then popup a warning if we don't have the "player allowed" flag set
- if ( !(wip->wi_flags & WIF_PLAYER_ALLOWED) && stristr(wip->name, "#weak") ) {
+ char* end = stristr(wip->name, "#weak");
+ if ( !(wip->wi_flags & WIF_PLAYER_ALLOWED) && end ) {
  int idx = -1;
  char non_weak[NAME_LENGTH];
 
- strncpy(non_weak, wip->name, strlen(wip->name) - 5);
+ memset(non_weak,0,NAME_LENGTH); // Valathil: strncpy doesnt explicity terminate the string it copies
+ strncpy(non_weak, wip->name, end - wip->name); // Valathil: Previous version fails on Weapons like WeaponName#weak#shivan
  idx = weapon_info_lookup(non_weak);
 
  // only add the flag if the non-weak version is also player-allowed


[Timerlane]

i dont even know why you want to enable player allowed if its weak wouldnt this show in the weapon name would look pretty stupid if you ask me.

To my knowledge, anything after a hash mark(#) in a weapon/ship name is never displayed(with the exception of the 'x weapon is not compatible with this ship' popup on the ship loadout screen, which always gives the full name). This is why you'll occasionally see two sets of Cyclops torpedoes available in a mission loadout screen(one is the regular Cyclops, one is the Cyclops#short, the latter of which is actually used in place of the regular Cyclops in the early parts of the retail FS2 campaign).

It's also an easy way to set up a ship with the same 'name' as another, but with different stats; for example, FSPort uses an 'SF Dragon#Weakened' for the one you use in "Playing Judas", but it only displays "SF Dragon" in the ship selection screen and all other relevant in-game displays.

That said, I imagine there might be occasions where someone might want to substitute #weak weapons instead of the originals(to intentionally 'nerf' the player), but probably not very often.

[Goober5000]

Well spotted, Valathil!  I'm pretty sure this is my code, and it looks like I forgot to take my own advice about using memset with strncpy.  I've committed a slightly modified version of your patch to SVN.

The fact that there is a 0 terminating the "Harpoon" string in debug probably isn't a one-in-a-million fluke; it's very likely that debug mode is simply initializing all arrays to 0s.

As Timerlane said, it's often useful to assign #weak variants of weapons to the player or to AI for mission balancing purposes.  In fact, the very reason for this code was that in the FS1 weapons table, Interceptor is allowed as a player weapon but Interceptor#weak is not.  This caused loadout issues in FSPort during a training mission that gave the player the Interceptor#weak missile.

[Nuke]

i dont even know why you want to enable player allowed if its weak wouldnt this show in the weapon name would look pretty stupid if you ask me.

To my knowledge, anything after a hash mark(#) in a weapon/ship name is never displayed(with the exception of the 'x weapon is not compatible with this ship' popup on the ship loadout screen, which always gives the full name). This is why you'll occasionally see two sets of Cyclops torpedoes available in a mission loadout screen(one is the regular Cyclops, one is the Cyclops#short, the latter of which is actually used in place of the regular Cyclops in the early parts of the retail FS2 campaign).

It's also an easy way to set up a ship with the same 'name' as another, but with different stats; for example, FSPort uses an 'SF Dragon#Weakened' for the one you use in "Playing Judas", but it only displays "SF Dragon" in the ship selection screen and all other relevant in-game displays.

That said, I imagine there might be occasions where someone might want to substitute #weak weapons instead of the originals(to intentionally 'nerf' the player), but probably not very often.

i once suggested a feature to define equivalent weapons for weapon variants such as #weak or ship specific versions (like externally mounted missiles). essentially you have an $equivalent to: tag with the name(s) of a weapon(s) that are for the most part the same. so if you try to load a cyclops into a ship which can only carry cyclops#short, or a cyclops#shot into a ship that can only carry a cyclops, the interface will equip the first weapon in the $equivalent to: list that is compatible with the ship and bank you are equipping it to. the user does not need to know that the weapon they are equipping has several variants, and they only see one of the variants in the loadout list.

[The E]

One unfortunate side effect of this is that loading retail data now outputs warnings, which is something we really do not want. I would recommend downgrading the warning to a log print.

[Valathil]

Code: [Select]

The fact that there is a 0 terminating the "Harpoon" string in debug probably isn't a one-in-a-million fluke; it's very likely that debug mode is simply initializing all arrays to 0s. In debug theres no 0, its in optimized so yes its a fluke thats why i love it so much.