Widget / Gadget Interaction, serious bug

[Argh]

Yup, got the one on the second page, and it appears to be working (no time for detailed tests atm, but Widgets are now operating after including the appropriate files).

[Argh]

Ok, I've finally tracked down the core cause of the buggy behavior: it's this section of the Gadget:

Code: Select all

function gadget:AllowCommand(u, ud, team, cmd, param, opts)
	if cmd == CMD.GUARD then
		if Infantry[ud] then
			if Holders[GetUnitDefID(param[1])] then
				--Spring.Echo("found building")
				GiveOrderToUnit(param[1],CMD_BUILDING_GET,{0},0)
				return true
			else
				return true
			end
		else
			return true
		end
	end
	return true
end

I think what's happening is that it's running CMD_BUILDING_GET for the given UnitID several times simultaneously (which, imo, is a bug, re-issuing a command several times before the first one returns is not a feature).

Is there any practical way to fix this? Maybe stick a state into a table, halt this section if it encounters a match, wipe the table entry when the command completes?

[lurker]

Not simultaneously, lua isn't threaded. It's just calling it sequentially in the same frame, meaning that the unit is dead but still there. Units aren't deleted instantly for various reasons.
You have a couple simple solutions. One is to check if each unit on your list is already dead before you try to replace it. The other is to do it in GameFrame, and make each gadget that wants unsafe mode call AllowUnsafeChanges("USE AT YOUR OWN PERIL") only for itself, and turn it off again after doing unsafe things, so it doesn't catch you by surprise in the future. (Note that you might need to do some tweaks to get access to that from gadgets, more details available upon request.)

[Argh]

One is to check if each unit on your list is already dead before you try to replace it.

I like that one, I'll put that in immediately, see whether it returns a true state immediately after the request to kill it is made.

[Argh]

Tested the kill idea.

Doesn't work, apparently DestroyUnit isn't changing the state immediately. It's safer, but the Guard stuff is obviously calling that command multiple times... and the check isn't preventing it. Maybe dead AND Neutral? <tests>

[Argh]

Hmm, no good, seems it's "killed" and Neutral, but that UnitID isn't being removed before it can repeat again that frame.

Soooo... maybe Spring.ValidUnitID? It's about the only thing left to try... see whether the UnitID becomes invalid after DestroyUnit, and I'm guessing no, it's fine, until the next frame. Probably going to have to run a filter, simply prevent this command from being run multiple times a frame for a given UnitID.

[lurker]

GetUnitIsDead fails?

[Argh]

It returns true over the entire course of the frame, so it's pretty useless, the UnitID is still good.

Finally found a solution, using a table to keep track of who's currently being deleted:

Code: Select all

	if cmd  == CMD_BUILDING_GET then
	if Holders[ud] then
		CallCOBScript(u, "ReturnPieces",0)
		piece = GetUnitScriptPiece(u,piece1)
		fillAmount = 0
		fillNumber = 0

		x,y,z = GetUnitPiecePosDir(u,piece)
		---Spring.Echo(x,y,z)
		fillList = GetUnitsInSphere(x,y,z,16)
		
		if #fillList == nil then fillNumber = 0 else fillNumber = #fillList end
		fillAmount = fillNumber + fillAmount

		--Spring.Echo(fillAmount)
		--if fillAmount == 0 then Spring.Echo("No nearby infantry, move them closer to this building.") end

		amount = tonumber(UnitDefs[ud].customParams.hold_amount)
		unitGroupNum = nil
		unitGroupNum = GetUnitsInSphere(x,y,z,400)
		local numGuys = #unitGroupNum
		--Spring.Echo(numGuys)

		for k,me in ipairs (unitGroupNum) do
			myTeam = GetUnitTeam(me)
			if me ~= u and Infantry[GetUnitDefID(me)] and team == myTeam or team == Gaia then
				--Spring.Echo("Found Infantry")
				if fillAmount < amount then
					if fillAmount == 0 then fillAmount = 1 end
					unitDef = GetUnitDefID(me)
						
					SetUnitNeutral(me,true)
					DestroyUnit(me,false,true)
					if DeleteList[me] == nil then
						table.insert(DeleteList, me, 1)
						you = CreateUnit(UnitDefs[unitDef].customParams.spawn_in_building,  x, y, z,0,team)
						Spring.MoveCtrl.Enable(you)
						Spring.MoveCtrl.SetGravity(you,0)
						Spring.MoveCtrl.SetSlopeStop(you,false)
						Spring.MoveCtrl.SetCollideStop(you,false)
						Spring.MoveCtrl.SetTrackGround(you,false)
						Spring.MoveCtrl.SetPosition(you, x, y, z)
						fillAmount = fillAmount + 1
					end
				else
					Spring.SendMessageToTeam(team,"This building cannot hold any more infantry.")
					return true, true;
				end
			end
		end
	return true, true;
	end
	end

I wipe the list once every 32 frames, seems to (finally) do the trick, and allow Units given the Guard command not to magically replicate themselves.

There needs to be a better way to do this, imo- UnitDelete, or something like, where we just plain get rid of a UnitID immediately, no questions asked.

[lurker]

The better way to do it is to STOP USING UNSAFE MODE

[Argh]

The last time I checked, you can't create / destroy units unless you're in unsafe mode. I can see invoking it only when I need to do that... but what else is forbidden unless it's on? I mean... no offense, but just knowing I'd have to re-check everywhere I use DestroyUnit / CreateUnit is a hassle, I don't really see the point unless it's well worth it in terms of safety.

And it wouldn't have prevented this problem, anyhow. This issue is related to the fact that a UnitID isn't removed immediately until the frame is over. I can still get all of the Orders and other information tied to that ID until the frame ends. Spring knows it's "dead", because it's not really dead, it's just scheduled for deletion.

It would be better, for "transformation" stuff, to just finish the transformation, create a new Unit, and immediately, irrevocably, destroy the parent unit and its id, imo. Then there's nothing left for other scripts, or repeats of same, to even interact with- the UnitID is no longer valid.

[lurker]

You can't create/destroy units outside of GameFrame. Make a list, do it there, far fewer bugs.
If GetUnitIsDead doesn't work then that's a Spring bug. There are good reasons to not delete the data instantly.

[Argh]

You can't create/destroy units outside of GameFrame. Make a list, do it there, far fewer bugs.

If Unsafe isn't allowed, you mean? I don't see the point, tbh, it wouldn't have prevented this behavior from occurring. The real problem here is that:

A. Units can get issued the same commandID many times a frame. I think that's a bug, there are all sorts of evil things that could be done with that (for example, select, then spam lots of commands at any visible enemy Units, which given that I've seen enemy UI show up because of 'selection' stuff is probably possible).

B. There is no way to halt interrogation of a UnitID except through a custom filter like the one I just wrote. And that's orcish engineering at its finest, I'm not saying it was elegant- using a table like that is probably considered abuse of the language We need a "cannot be addressed by Lua any more" method. I don't really care, if at a deeper engine level the UnitID still exists, it just shouldn't be accessible to Lua, imo.

If GetUnitIsDead doesn't work then that's a Spring bug. There are good reasons to not delete the data instantly.

Well, it didn't work, apparently. Saw that it was dead, went ahead and ran anyhow.

The solution I have blocks it, and meh, it's not like it's very slow to do a few table insertions on a table that's being erased on a regular basis. I guess that's where it can stay, for now. I need to write up a music-volume control for the game now, apparently that's a gripe that went under the radar.

[lurker]

Volume control doesn't appear to actually work under 78.2 on windows. 0 is silent, .01 is loud.

If you send commands to enemy units they get ignored; can you explain more?