NoQ uploaded file warzone2100.gdmp-K5W1J5 (34.1 KiB)

The crash dump, one of the most interesting ones, that leads straight to the function mentioned above.

NoQ uploaded file warzone2100.log (40.7 KiB)

Another crash with similar properties.

NoQ uploaded file warzone2100.gdmp-N7fKXA (29.1 KiB)

Another crash with similar properties.

vexed commented

While this is a bug, the real problem is, there are no checks being done script side, or source side, and the game assumes everything is OK.

If you look at how a human would normally do this, they would issue a DORDER_EMBARK order, then it does the checks there.

The old script code just checked for capacity, and if on mission or not, and also safety checks to make sure it is a transport, and a non null droid.

The new script code... don't do squat, and just assumes everything is fine.

I would say, for a quick fix, do some script side checking...

NoQ changed _comment1 which not transferred by tractive

NoQ changed _comment0 which not transferred by tractive

NoQ commented

It sounds as if using js newGroup() during the game (rather than during the early initialisation) may lead to group id collisions. Transporter group id to js tank group id, in particular.

In fact i had quite a few problems with group id collisions while working on NullBot, that disappeared only when i made sure groups are only allocated on startup-time; otherwise it sometimes lead to "trying to add a tank to group belonging to the other player" js exception, and i never managed to find a bug on the script side that may lead to such a trouble.

But this particular Scav AI still allocates groups on run-time, and using it together with NullBot sometimes leads to group collision exceptions on both sides (sometimes in NullBot and sometimes in Scav AI, even though NullBot is free from it on its own).

So probably something similar leads to newGroup() returning an id already belonging to a chopper, or vice versa, an id already taken by a chopper is already in use by one of the js groups.

Note: this paradigm also avoids using the Cyp's Orange Fruit argument.

NoQ changed _comment0 which not transferred by tractive

NoQ commented

grpCreate() call without arguments (defaulting to -1) actually does lead to creating duplicate group ids.

NoQ commented

The crash does not happen until there actually exists a group that was not found from js at grpFind() at some point (and thus re-created), but the group not found isn't usually equal to the transporter's group.

NoQ changed _comment0 which not transferred by tractive

NoQ commented

The faulty transporter is being assigned to one of the groups already created by JS at eventStartLevel(). JS doesn't ever free or release this group (in fact it has no function to do that).

NoQ commented

I think i found it. When a last droid is deleted for a certain group, this group is removed - see void DROID_GROUP::remove(DROID *psDroid), at the end.

But JS still remembers its id in its variable, even if it's empty. But the rest of the game doesn't know about it. So probably some group gets empty, then when transporter is produced the game says hey let's assign this group id to it, then JS says hey i've had a group somewhere with this id and i want to add my tank to it again, then we get this tank deleted when the transporter dies, then we crash.

NoQ commented

Is it ok if we just never delete groups? Or maybe add a flag into the group saying "don't delete me".

NoQ changed type from bug to patch (an actual patch, not a request for one)

NoQ changed _comment0 which not transferred by tractive

NoQ commented

I propose the following fix.
Add this line to the middle of js_newGroup() function body in qtscriptfuncs.cpp:

newGrp->refCount++; // make sure the group is not deleted when it runs out of units

It will make sure JS groups are never deleted, any other groups are unaffected.

vexed commented

Technically, no, it isn't ok to never delete groups, on shutdown.

As for the patch.. erm... reference counting is supposed to be used to help track down issues, so this is a kludge of sorts.

I'll let per fix it :)

NoQ commented

on shutdown
Well, by "never delete groups" i meant "never delete groups simply because refCount==0" (that is, never allow removing a droid lead to deleting the group as well".

reference counting is supposed to be used to help track down issues
Hmm. The comment in group.h says
Number of objects in the group. Group is deleted if refCount<=0. Count number of droids+NULL pointers.
I wonder what are those NULL pointers in this context.
I'll let per fix it :)
ok ...

NoQ commented

Here is how groups are created in wzscript (scriptvals.cpp):

// create a group structure for a ST_GROUP variable
bool scrvNewGroup(INTERP_VAL *psVal)
{
	DROID_GROUP		*psGroup;

	psGroup = grpCreate();

	// increment the refcount so the group doesn't get automatically freed when empty
	psGroup->add(NULL);

	psVal->v.oval = psGroup;

	return true;
}

The "psGroup->add(NULL);" call is completely equivalent to incrementing refCount. I don't see a reason not to do the same in js.

Per changed status from new to closed

Per changed resolution from `` to fixed