 |
As we attempt to wrap up 1.08, is this fix required, or can it wait? |
 |
It is not fixed, and since its a crash, it should probably be fixed. A cursory look didn't show me an easy way to fix it, tho. Once the cursor runs over, it somehow needs to be set back to a valid item. |
|
|
And I forgot to mention that the crash can occur because the player drops an item from the inventory to the hands or ground, too. |
|
|
I picked up two flashmines, then dropped one after the other on the ground, with no crash. Guess I have to find out what's going on. |
|
|
And how do I "make a custom category"? |
|
|
This note is for angua, because she filed the issue. I'm unable to reproduce a crash, and want to know if I'm creating a reasonable test case. I have a flashmine (Mine1) and a mine (Mine2), both of which belong to the Tools category. I have two buttons. Button 1 calls a script that does this: $player1.replaceInvItem($Mine1, $null_entity); $player1.replaceInvItem($Mine2, $null_entity); Button 2 calls a script that does this: $player1.replaceInvItem($Mine2, $null_entity); $player1.replaceInvItem($Mine1, $null_entity); In the game, I pick up Mine1, then Mine2. In the inventory cursor list, Mine2 becomes the first entry, and Mine1 is the second entry. Regardless of which button I press, I can't get a crash. I assume Button 2 is the one that creates the problem you describe, since Mine2--the second item from the end--is removed first, followed by Mine1. Have I set this up correctly? (There is a display problem where the screen isn't being updated properly depending on what's being displayed, but that's a separate issue.) |
 |
Can't reproduce the crash either, looks like this is not an issue any more. |
|
|
May I close it,then? Thanks. |
|
|
I am still not convinced that this bug is really fixed. Consider what this code does: void CInventory::RemoveItem(const CInventoryItemPtr& item) { if (item == NULL) return; // Update the cursors first for (int i = 0; i < m_Cursor.Num(); i++) { if (m_Cursor[i]->GetCurrentItem() == item) { // Advance the cursor, this should be enough m_Cursor[i]->GetNextItem(); } } // Now remove the item, the cursors are updated. item->Category()->RemoveItem(item); } If you have three items in a category, 0, 1, and 2, and the cursor is at item 1 (the second-to-last-one), then m_Cursor[i]->GetNextItem(); will fetch the ptr to the last item (2) and set the item index for the cursor to "2". However, when you then remove the second-to-last-item, the ptr to the item inside the cursor is still valid (points to 2), but the itemIndex is invalid, it is still "2", but there are only two items now (0 and 1). The code item->Category()->RemoveItem(item); does not seem to update the cursor anymore. E.g. the code seems to be backwards, it first updates the cursor, then removes the item, leaving and invalid cursor. That it no more crashes is probably pure luck. |
 |
Wouldn't it be better to select the previous item? This 'could' lead to a case where it tries to select an invalid index, ie, -1, but, that shouldn't be hard to account for. |
 |
Possibly relevant: when I made the this-message-will-self-destruct-after-you-read-it note in ITB I started out just dropping it in the map with "inv_map_start" "1", and removing it with $player1.replaceInvItem($inv_item_to_remove, $null_entity) caused the item cursor to change to empty. However, after I made a corresponding shop item and added it to the starting inventory via the shop, the inventory cursor would stick around until I cycled to another item. I ended up adding $player1.getPrevInvItem(); to the script. |
 |
The GetCurrentItem()-method family do a validity check of the index and return NULL-ptr for invalid indices. The callers do not consistently check for nullptr and I didn't want to mess with the whole codebase, so I have ensured the index is always valid. As a fallback, the TDM_DUMMY_ITEM is selected in error cases, which always exists. This is the same as clearing the inventory ingame. Rev. 7957 |
- Anonymous
