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

[Graf Zahl]

FYI, I committed the rest of this PR but left out the self check for now.

And what a check really should do is something along:

Code: Select all

if (self != stateowner)
{
    if (variable_address outside of range for AActor)
    {
        throw an error;
    }
}

But can it be done in a less costly manner? Your attempt of a fix adds a lot of code for each variable access which may actually cause performance issues because this can happen so often.


I hope that for the real scripting language a better solution can be found.

[Leonard2]

Updated the PR: I simplified the check and fixed the problem I mentioned previously.

[Major Cooke]

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.

I didn't see it as abuse since randi made it possible. >.>;

[Graf Zahl]

It still was a bad, shortsighted decision then.

[Major Cooke]

So, curious, by abort, does that mean it shuts down the game (go to console) or does it just reject doing that specific thing from happening?

[Matt]

From all the complaints so far, it seems that only that particular function aborts.

Was this supposed to break projectiles that use uservars in their damage definition? If so, why, given they don't involve weapons or custominventory?

[Graf Zahl]

In that case, probably because their calling info is not set up properly.

[Matt]

What would be proper?

Both this

Code: Select all

actor zimpball:doomimpball replaces doomimpball{
	var int user_blah;
	damage(user_blah+1)
	states{
	spawn:
		TNT1 A 0 nodelay A_SetUserVar(user_blah,100)
		goto super::spawn
	}
}

and this

Code: Select all

actor zimpball:doomimpball replaces doomimpball{
	var int user_blah;
	damage(1+user_blah)
	states{
	spawn:
		TNT1 A 0 nodelay{
			user_blah=100;
		}goto super::spawn
	}
}

trigger the error and abort.

[Graf Zahl]

That should work, so please report a new bug.

[Leonard2]

[examples here]

I think I already know what's happening.
This doesn't actually trigger because of your decorate code, I think it triggers when the damage is calculated (which is done within the VM) and since there is no stateowner pointer passed to the damage vmfunction call the error is triggered.
To test this you could just put the damage on a missile and make it explode, see if the error shows.

[randi]

Was this supposed to break projectiles that use uservars in their damage definition? If so, why, given they don't involve weapons or custominventory?

This FxClassMember change is generating bad code for damage expressions, because those don't have a callingstate parameter, but it generates code to try to compare against it anyway.

[Leonard2]

This isn't the only place where damage expressions are problematic with this anyways:

Code: Select all

actor D : rocket
{
	Damage (bool(A_JumpIf(true, "crashes")))
}

The actor D will crash the game when it explodes.

[randi]

The self == stateowner check is now restricted to action functions, so it won't crash Damage functions. The other problem has been fixed too.

Using A_SetUserVar, that works from weapons to player.

I don't see why that shouldn't work as described, since it works on self, just like every other function called from a weapon state. Don't consider A_SetUserVar and operator= as equivalent, since they're not. The former is resolved at compile time, and the latter is resolved at runtime.

Code: Select all

actor zimpball:doomimpball replaces doomimpball{
	var int user_blah;
	damage(1+user_blah)
	states{
	spawn:
		TNT1 A 0 nodelay{
			user_blah=100;
		}goto super::spawn
	}
}

Looking at the code generated for the spawn state here, I think it should be safe to omit the self==stateowner check for any actors that aren't inventory items, since those are the only ones that could possibly be called with them different.