Unexpected behavior in override of Weapon.GetUpState

[Player701]

Check out this ZScript:

Code: Select all

class TestPistol : Pistol
{
    override state GetUpState()
    {
        if (owner)
        {
            owner.A_PlaySound("misc/w_pkup", CHAN_WEAPON);
        }
        
        return Super.GetUpState();
    }
}

If you give yourself this weapon via the in-game console or by launching GZDoom with these parameters:

Code: Select all

gzdoom.exe -file zscript.txt -warp 1 +give testpistol

the sound plays correctly.

However, when going to another map, this will not work as expected. Instead, one of the two things will happen depending on GZDoom version:

1) GZDoom 3.6.0 crashes with a very fatal error.
2) GZDoom g3.7pre-4-gcc8112f88 doesn't crash, but the sound is still not heard.

Expected behavior: the sound should be heard regardless of the context GetUpState is called in.

It is interesting to note that if TestPistol is assigned as a starting weapon, there is no crash (as well as no sound) when starting a map.

[Graf Zahl]

Crash aside, this is a clear "Don't do that!" situation. As a general rule, "Get..." functions should never implement side effects! To play a sound when a weapon is raised, give the weapon an "UpSound".

[Player701]

Well, in my use case it is logically more correct if the sound is first heard when the weapon appears on the screen. Also, it is intended to be played in a loop, so UpSound will probably not work. And sometimes the sound does not need to be played at all.

In other words: I need a valid entry point to call arbitrary code as soon as the weapon appears on the screen. Trying to use the first frame of the Raise state for that yields results similar to the ones described above (sometimes crashing and sometimes not doing anything at all).

[Nash]

Crash aside, this is a clear "Don't do that!" situation. As a general rule, "Get..." functions should never implement side effects! To play a sound when a weapon is raised, give the weapon an "UpSound".

There is no way for anyone to know this. What is "don't do that", what is "you can do that". There's no reference anywhere on what can or cannot be done.

In OP's case, even I would have done it that way.

[Graf Zahl]

GetUpState is not that point. What this function does is retrieving the state of the weapon raise sequence. You have no guarantees that it gets called where you need it - and even less that only where you need it.
GetUpState is called in 3 places:

player_t::BringUpWeapon
PostMorphWeapon
Resurrect

For what you want, BringUpWeapon is the only relevant one, and it actually does play the UpSound. The other two do not but they are special cases a regular player never goes through during normal play.

@Nash: A bit of thinking might help. The function is called "GetUpState". So it does what it says: It "gets" the state. In this case the name implies its function. And in case you are not sure: Looking where some code gets used can quickly tell you if what you plan may be viable.

[Player701]

GetUpState is not that point. What this function does is retrieving the state of the weapon raise sequence. You have no guarantees that it gets called where you need it - and even less that only where you need it.
GetUpState is called in 3 places:

player_t::BringUpWeapon
PostMorphWeapon
Resurrect

For what you want, BringUpWeapon is the only relevant one, and it actually does play the UpSound. The other two do not but they are special cases a regular player never goes through during normal play.

Unfortunately, BringUpWeapon is not virtual, and even it it were, the code would be coupled to the player class and not to the weapon itself, which would be semantically wrong.

I guess I will implement my logic in the Ready state then, even though it is not ideal for my use case (but close enough).

[Graf Zahl]

I guess I will implement my logic in the Ready state then, even though it is not ideal for my use case (but close enough).

I think now I understand why this didn't work. GetUpState retrieves the SELECT state, not READY!
But for the Ready state you'd run into even more problems when overriding its getter.

[Player701]

GetUpState retrieves the SELECT state, not READY!

I am aware of that, sorry if I didn't make it clear.

But for the Ready state you'd run into even more problems when overriding its getter.

No, not override the getter, but instead call the code in the state's frames (same place as A_WeaponReady etc.), this seems to work fine.

[Graf Zahl]

That's actually the proper way to do it...

[Player701]

That's actually the proper way to do it...

Yes, but I actually wanted to execute the code when the Select (raise) state plays, not the Ready state. And it seems there is no way to achieve that as of now, considering what you say and what I said above:

Trying to use the first frame of the Raise state for that yields results similar to the ones described above (sometimes crashing and sometimes not doing anything at all).

By "Raise", I meant Select, of course - sorry, confused the name with A_Raise function.

And that's why I said this solution wasn't ideal for my use case, because with it, some game time will pass after the weapon is raised and before the code is executed (though in my case it doesn't feel much different, but still, I don't like having to resort to workarounds).

[Graf Zahl]

I have absolutely no idea where your problem is. Do you want to play something after the weapon has been fully raised or when it starts rising?
+

[Player701]

I have absolutely no idea where your problem is. Do you want to play something after the weapon has been fully raised or when it starts rising?
+

Ideally, it's the latter, and it's not just "play something" but "execute arbitrary code which may or may not include playing a looped sound". Sorry if I couldn't make myself clear earlier.