Mon 19 Apr 2010 08:12:02 PM UTC, original submission:
Seen in trunk r9670. If a trigger calls KillUnitAt twice with the same arguments, then the second calls finds the same units that the first call already killed, and tries to kill them again. This causes an assertion failure in UnitLost:
Although this bug is easiest to reproduce by calling KillUnitAt twice, I think it could also occur if some units in the rectangle were dying for another reason.
CclKillUnitAt should be changed to check whether the units are still alive, and skip dead ones without counting them. That is, each call with the same arguments should kill a few more units in the rectangle.
The assertion failed because of what I think is a bug in UnitLost:
After the assignment temp->PlayerSlot = unit->PlayerSlot, both units point to the same PlayerSlot, which of course can only point back to one of them (to the live unit). Because the dying unit is being removed from the player's list of units, I think it would be good to assign unit->PlayerSlot = NULL here, even if the readers of CUnit::PlayerSlot do not yet properly support NULL. Crashes due to null pointers would be easier to debug than assertion failures due to incorrect pointers, at least.
When a unit dies, it must be removed from the player's list of units, so that the AI won't try to recruit it to a force. However, the CUnit::Player pointer must not be reset to NULL, because CUnit::Draw needs it (and resurrection spells would need it too). Then, I think CUnit::PlayerSlot == NULL should be okay as a flag indicating the unit is no longer in the player's list.
|