bugBos Wars - Bugs: bug #29608, KillUnitAt kills already dead...

 
 

bug #29608: KillUnitAt kills already dead units, causing an assertion failure

Submitted by:  Kalle Olavi Niemitalo <kon>
Submitted on:  Mon 19 Apr 2010 08:12:02 PM UTC  
 
Severity: 4 - ImportantItem Group: Bug
Status: FixedPrivacy: Public
Assigned to: Jimmy Salmon <jsalmon3>Open/Closed: Closed

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

Thu 22 Apr 2010 03:15:59 AM UTC, comment #2:

The cache needs to have dead units so buildings under fog of war remain visible until you've seen the tile.

Jimmy Salmon <jsalmon3>In charge of this item.
Wed 21 Apr 2010 11:50:51 PM UTC, comment #1:

We should consider not adding dead units/corpses to the unit cache. This was done in Stratagus so you could raise units from the dead but Bos doesn't need this.

Jimmy Salmon <jsalmon3>In charge of this item.
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.

Kalle Olavi Niemitalo <kon>
Project Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by kon (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 5 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Thu 22 Apr 2010 03:15:59 AM UTCjsalmon3StatusConfirmed=>Fixed
      Open/ClosedOpen=>Closed
    Wed 21 Apr 2010 11:50:51 PM UTCjsalmon3Severity3 - Normal=>4 - Important
      StatusNone=>Confirmed
      Assigned toNone=>jsalmon3

    Back to the top


    Powered by Savane 3.1-cleanup1