[ZScript] DECORATE custominventories partly broken

[Major Cooke]

On the Decorate side of things, custominventory actors cannot have user vars anymore. It errors up claiming 'invalid access'.

For clarity, I've left out the pickup/use states. This is when the item's out in the world.

Code: Select all

Actor DMSoulFragment : CustomInventory
{
	var int user_count;
	var int user_scale;
	var int user_gs;
	var int user_tid;
	+INVENTORY.ALWAYSPICKUP
	//+FLOATBOB
	+NOGRAVITY
	+BRIGHT
	+NOTELESTOMP
	-TELESTOMP
	-COUNTITEM
	+DONTGIB
	Species "Permanent"
	Scale 0.0
	RenderStyle Add
	Inventory.PickupSound "reilsss/health/soul"
	Inventory.MaxAmount 0
	States
	{
	Spawn:
		TNT1 A 0 NoDelay A_SetUserVar(user_tid,tid)
		TNT1 A 0 Thing_ChangeTID(0,0)
		TNT1 A 10 A_Stop
		X099 A 0 A_SpawnItemEx("DMSoulTarget",0,0,0,0,0,0,0,SXF_NOCHECKPOSITION|SXF_SETMASTER|SXF_ORIGINATOR|SXF_TRANSFERTRANSLATION,0,tid)
		"####" "#" 0 A_JumpIf(z-floorz <= 10.0,"MoveUp")
		"####" "#" 0 A_CheckFloor("MoveUp")
		Goto ScalingUp
	MoveUp:
		"####" "#" 0 A_Warp(AAPTR_DEFAULT,0,0,10,0,WARPF_NOCHECKPOSITION|WARPF_TOFLOOR)
	ScalingUp:
		"####" "#" 1 A_SetScale((cos(user_scale-90)+1)*0.05)
		"####" "#" 0 A_SetUserVar("user_scale",user_scale+5)
		"####" "#" 0 A_JumpIf(user_scale >= 180, "Scale2")
		"####" "#" 0 A_JumpIf((user_count >= 35*15) && (ScaleX <= 0.0),"End")
		"####" "#" 0 A_JumpIf((user_count >= 35*15),2)
		"####" "#" 0 A_SetUserVar("user_count",user_count+1)
		"####" "#" 0 A_JumpIf(user_scale >= 359,1)
		Loop
	Scale2:
		"####" "#" 1 A_SetScale(((cos(user_scale-90)+1)*0.01)+0.05)
		"####" "#" 0 A_SetUserVar("user_scale",user_scale+5)
		"####" "#" 0 A_JumpIf((user_count >= (35*15)) && (ScaleX <= 0.0),"End")
		"####" "#" 0 A_JumpIf((user_count >= (35*15)),"L2")
		"####" "#" 0 A_SetUserVar("user_count",user_count+1)
		"####" "#" 0 A_JumpIf(user_scale >= 359,1)
		Loop
		"####" "#" 0 A_SetUserVar("user_scale",user_scale-359)
		"####" "#" 0 A_JumpIf(user_count < (35*15),"Scale2")
	L2:
		"####" "#" 1 A_JumpIf(ScaleX <= 0.0,"End")
		"####" "#" 0 A_SetScale(ScaleX-0.005)
		Loop
	End:
		"####" "#" 1 A_RemoveChildren(1,RMVF_EVERYTHING)
		Stop
	}
}

[Graf Zahl]

Damnit! And here I was hoping nobody would do such crap because disabling the user vars was the only way to fix the insanity with the self pointer.

It's either this or back to square one. Both won't go.

[Major Cooke]

Oh... It had to be the user vars on the Decorate side? And are you talking about this and this? Because I was only trying to utilize them on the actor itself while in the world.

Also, if things can have user vars... you can count on the fact that people will use them anywhere they can. It's the only source of custom variables after all.

[Graf Zahl]

That one showed that some tests were done on the wrong pointer.

The problem I am facing is that there is absiutely no way to tell if a state is used by the actor itself or by its owner, before it is actually being used, meaning I can't give any diagnostic messages anymore if this is reverted. So I restricted them to the self pointer's type which is Actor. In ZScript you can get around this limitation by dereferencing invoker, but in DECORATE you cannot. And I assumed that nobody had done that yet because I haven't seen any mod going this far. How stupid of me to make such an assumption.

[Major Cooke]

Well, I think quite a few mods rely upon it. Think for now it should just be reverted?

[Fishytza]

Does reverting mean removing the whole 'invoker' thing? please no

[Graf Zahl]

Reverting means undoing all the error checks for improper variable accesses in weapon and custom inventory states.
It's an all or nothing proposition and we'd be back to crash #1.

[Graf Zahl]

Before I decide anything here I need to know how widespread this was used. Let me make this clear: We MUST find a solution here that makes sure that with ZScript it cannot happen that a anything that starts a weapon or item state which tries to access member variables of the containing class. If all else fails I go back to the beginning and implement different state blocks in ZScript for actors, weapons and items and you'll have to adjust all ZScript code that has already been written. Which may be necessary anyway if HUD overlays from arbitrary actors become a thing. If a state is unambiguously tied to its use case the holes should be plugged in ZScript which only leaves DECORATE where I'd reinstate Leonard's runtime check. Of course even then I can promise you that the item you posted will print a warning that it contains unsafe code.

[Major Cooke]

Are the custom inventories supposed to call invoker. even when in the world, and not picked up?

[Leonard2]

I don't know what exactly broke this and when, I must have missed something on git.

We MUST find a solution here that makes sure that with ZScript it cannot happen that a anything that starts a weapon or item state which tries to access member variables of the containing class.

Wasn't this fixed by your proposed "invoker" solution?

[Graf Zahl]

Yes, but I assumed that nobody would make inventory items that use user variables on themselves. Which has been proven incorrect by this report.
My invoker idea works perfectly, the only drawback is that in DECORATE the items cannot access their own user variables anymore because I need to impose some restrictions for the self parameter.

Right now the only working solution I see is to flag states for the intended use case, and create a runtime error if a bad one is called or a compile time error if a bad one is linked. Of course that will pose a problem with existing DECORATE stuff that's using user variables in such a context - obviously I cannot retroactively flag these mods as would be needed.

[Fishytza]

there is absiutely no way to tell if a state is used by the actor itself or by its owner, before it is actually being used

Personally I wouldn't mind having to use special state blocks if that's what it takes to determine beforehand if "a state is used by the actor itself or by its owner".

Other than that I don't have any ideas.

[Graf Zahl]

Just for discussion, here's a collection of ideas I had:

1. as it is now: Make 'self' a pointer to an 'Actor' object. Drawback: Weapons and CustomInventories cannot use user variables directly because I cannot distinguish between state types.
2. Implement 3 types of states: Regular states, pspstates and itemstates. Drawback: needs runtime checks and adds syntax complications, cannot be retroactively applied to DECORATE.
3. Make the invoker pointer self. Drawback: Syntax of weapon action functions will be completely messed up, not intuitive for modders, not compatible with DECORATE
4. Same as 3 but only for code within braces. Drawback: Incoherent semantics and wouldn't solve anything because such states without braces can be used in a bad context, unless user variables were prohibited in them, which cannot be done.

So, we have 4 solutions, none of which is good. I thought 1 was the best compromise and it still is, as long as there's no weapons or items with custom variables.

[Fishytza]

Sorry for the possibly naive question.

Wouldn't it be possible impose those self paramater restrictions if the state is defined in the "special inventory states block" as opposed to relying if the class is a StateProvider?

EDIT: So, no self parameter restrictions for states in the "States" block even for weapons and custominventory because this is where "Spawn" goes. While "Ready", "Fire", "Select", "Deselect", "Flash", "Pickup", "Use" and anything else I missed goes into a special "itemstates" block.

[Graf Zahl]

Yes, that'd be option 2. But that renders the check ineffective for DECORATE. The beauty about 1 is that it can flag problem cases in DECORATE, too, provided, well... - see this report.
Making this work in DECORATE and making this robust are mutually exclusive.