Re: [g2.2pre-2105-g8f53599]Crash to desktop: User variables?

[zrrion the insect]

I am currently working on using user variables and anon. loops to make conical bullet spread. I have run into an issue where the following code (or variations of the following code) will crash gzdoom to desktop and I have yet to find a way to make it do this consistently. On times that it will not crash when firing the effected weapons it will sometimes crash later on (often when the effected weapons are not even selected.) This happpens with the chaingun, shotgun, and supershotgun.

Code: Select all

SGIH B 4 Bright
{
    A_Light(1);
    //conical bullet spread
    user_i = 1;
    while (user_i <= 7)
    {
        user_BulletRadius = fRandom(0.0,5.6);
        user_BulletAngle = fRandom(0.0,359.9);
        A_FireBullets(user_BulletRadius * Cos(user_BulletAngle), user_BulletRadius * Sin(user_BulletAngle), -1, 5, "BulletPuff", FBF_EXPLICITANGLE);
        user_i += 1;
    }
    A_PlaySound ("weapons/shotgf", CHAN_WEAPON);//this plays even when it crashes, so it does make it through the loop
}

I have included both the mod and the crash report:
Mod in question
Crash report

[Blue Shadow]

[...] it would be extremely helpful to reduce this down as much as possible to an example wad.

With this file, the issue can manifest.

annoying_bug_test.pk3

All it contains is this DECORATE code. It's a stripped-down version of the shotgun's firing state from the mod linked to in the OP:

Code: Select all

ACTOR SG : Shotgun
{
    Weapon.SlotNumber 3

    var float user_ShotgunBulletAngle;
    var float user_ShotgunBulletRadius;

    States
    {
    Fire:
        SHTG A 3
        SHTG A 7
        {
            user_ShotgunBulletRadius = fRandom(0.0, 6.32);
            user_ShotgunBulletAngle = fRandom(0.0, 359.9);
            A_FireBullets(user_ShotgunBulletRadius * Cos(user_ShotgunBulletAngle), user_ShotgunBulletRadius * Sin(user_ShotgunBulletAngle), -1, 5, "BulletPuff", FBF_EXPLICITANGLE);
        }
        SHTG BC 5
        SHTG D 4
        SHTG CB 5
        SHTG A 3
        SHTG A 7 A_ReFire
        Goto Ready
    }
} 

If you don't experience anything immediately, don't give up thinking that everything is fine; it could take a bit to show up, as it is random. ZDoom could puke at you at any time.

While testing, ZDoom either quits abruptly, or crashes. Sometimes it just hangs and then "stops working".

The tests I did today were made with 2.9pre-1269-g017d1ce (32-bit). Unfortunately, I can't post a crash report as I didn't manage to get any today.

[Major Cooke]

User variables weren't supposed to work with weapons to begin with, were they? The thing is, they have to be owned by the player itself...

[Graf Zahl]

Yes. This, unfortunately is an unfixable problem with how DECORATE works. I wonder if scripting can handle this, because even there a weapon action function needs to run in the player's context.

The crash probably happens because the variables get resolved at compile time but when run in the VM access invalid data in the player.

[zrrion the insect]

When I tried to define the variables on the player and not the weapon the error I got was that they needed to be defined on the weapon, so I moved them there. If that isn't the case the error message should be changed.
Would I be able to define the user variables used by the weapons in the player instead and achieve the results that I want?

What prevents the player from being given access to the user variables of weapons and custom inventory items? It seems like that is something that you would want to have happen.

[Graf Zahl]

The issue here is that the weapon function is called with the player as the invoking actor, but defined within the context of the weapon. As a result during compilation it has access to the weapon's variables but during execution it does not. Likewise, while it would have access to the player's variables during execution, that's an impossibility because during compilation it knows nothing about them.

[zrrion the insect]

Would defining a variable of the same name on both the weapon and the player resolve the issues that are being experienced?

[Graf Zahl]

No, nothing guarantees that it ends up in the same position.

[Leonard2]

Which means that the problem was already existing when simply reading user variables but only started crashing now because of the ability to assign directly to them.

---

Code: Select all

        SHTG A 7
        {
            //user_ShotgunBulletRadius = fRandom(0.0, 6.32);
            //user_ShotgunBulletAngle = fRandom(0.0, 359.9);
            A_LogInt(user_ShotgunBulletRadius);
            //A_FireBullets(user_ShotgunBulletRadius * Cos(user_ShotgunBulletAngle), user_ShotgunBulletRadius * Sin(user_ShotgunBulletAngle), -1, 5, "BulletPuff", FBF_EXPLICITANGLE);
        }

The wiki was already stating this since long ago anyways:

User vars can be stored, but not retrieved, directly through DECORATE.

Now it should probably be change to something like "ONLY the A_SetUserVar/Array/Float actions will properly work."

Anyways here's a pull request that makes the VM able to properly abort on critical errors including this specific user variable case to avoid crashes.

[Graf Zahl]

I think the action function interface really needs a bit of refactoring so that this doesn't puke all over the place, even if it causes compatibility issues.

[Graf Zahl]

Can you give a short explanation what you did with FxSelf? It looks to me that you unconditonally check if the class matches, which would cause an abort in any weapon function this is used in. I do expect that there's some mods which actually access the player through this.

[Major Cooke]

I utilize A_SetUserVar for weapons setting user variables on the player directly in D4D.

[Leonard2]

Can you give a short explanation what you did with FxSelf? It looks to me that you unconditonally check if the class matches, which would cause an abort in any weapon function this is used in.

Yes. FxSelf is only ever used with FxClassMember which itself is only ever used to either read or write to a variable which was shown to be completely broken in this thread.
My reasoning was that any weapon/custominventory that relied on this already had unintended behavior and was probably broken.

I do expect that there's some mods which actually access the player through this.

But they aren't working properly to begin with so I thought aborting was just cleaner.
Besides the wiki was already stating that this just doesn't work.

EDIT:
I just realised non-user fields were also affected by this, that was a pretty stupid error, I will fix this in my pull request later on.
Is this what bothered you or were you also talking about user variables still?

[Major Cooke]

Using A_SetUserVar, that works from weapons to player.

[Graf Zahl]

I utilize A_SetUserVar for weapons setting user variables on the player directly in D4D.

Yeah, well, thank you for abusing something that was never meant to work that way...
The entire thing is broken, everybody knows it and yet people depend on the broken behavior, making a fix impossible. Ever since this got reported the first time the one issue that prevented us from addressing it is modders' insistence on abusing every single glitch they find.