SmartExchange and grid position check issues

[Cheo]

Hello, I managed to reproduce an issue with item exchange between an inventory and equipment slot in the "7_1 Item Shape Grid With Equipping And Hotbar" demo scene. Here are the steps :

-Follow the instructions to make it so that the inventory grid does not display equipped items.
-Use an ItemViewSlotDropActionSet with only a SmartExchange condition and action.
-Assign a tall item into an equipment slot.
-Make it so that the inventory grid does not contain enough space for that tall item to be dynamically brought back in.
-Assign a smaller item into the equipment slot.

If you follow these steps you should see that the smaller item is equipped but that the big one just disappears and is nowhere to be seen in the Inventory grid. As I looked into ItemViewDropContainerCanSmartExchangeCondition, I noticed that sourceCanAdd wasn't used in the last return line, and that it wasn't set to true if no item were to be added to the source. If these two issues are fixed, we can then prevent an item from being dragged into the equipment slot if the current one can't go back to the grid, but this actually brings another issue as in reality the switch is also prevented even if there is enough space for the bigger item. Here is a video illustrating all of this :

Lastly as I mention at the end of the video my investigation led me to ItemShapeGridData.IsPositionAvailable which seems to return false when it shouldn't. This already took me a good amount of time for today and this is grid code stuff that I haven't touched before, so this is where I stop for now. I was at a playtest event last Saturday and saw a player reproduce the initial issue with a gun disappearing, so this is quite a significant issue for us and hope you can take a look at this, thanks.

 

[Sangemdoko]

Thank you Cheo.
That's a very insightful bug report you made here.

The hard part here is that for SmartExchange to work no matter the scenario whether it's grids, shape grid and/or slots in any combination.

When hitting such limitations I recommend making your own custom dropAction script specifically for your use case (in this case slot to/from shape grid). This will give you more freedom to choose what exactly is supposed to happen when there is no space, or if there is space but only if reorganizing the shape grid, or if there is space only if doing the equip/unequip, etc...

In any case I've made a note about this and will be investigating when I have time, espcially the part about IsPositionAvailable

 

[Cheo]

I looked into IsPositionAvailable and IsSingularPositionAvailable today, and have 2 things to report :

-I experimented with a 1 slot Wooden Sword and the standard 3 slots Iron Sword. When assigning the Wooden Sword to the weapon slot and then trying to drag and drop the Iron Sword into it, it was not possible and part of the reason why seems to be that IsSingularPositionAvailable considered the slot the Wooden Sword would take as occupied by... The Iron Sword ! That's right, the one that was supposed to switch places with it.

-After putting Debug.Logs for every return line in both functions, I got a whole bunch of logs in the console by just dragging the Iron Sword onto the weapon slot where Wooden Sword was, 30 just for that. It seems that these checks are called many more times than necessary, especially for an item that would only take one slot.

I hope that all of this makes sense and is useful, lemme know if you need a video example.

 

[Sangemdoko]

So I've looked into this issue today. And the IsPositionAvailable and IsSingularPositionAvailable work as expected.

The reason we get so many calls on IsSingularPositionAvailable is because it's a recursive function. It checks if the item fits on any position in the grid, one by one.

So the issue was actually the SmartExchangeAction. I think it was too lenient on whether it allowed items to be exchanged or not.
I've add an option to be more rigid when it allows adding and when not:

in ItemViewDropContainerCanSmartExchangeAction.cs I've made those modifications

Line 23:

[Tooltip("Log some useful information to debug smart exchanges.")]
[SerializeField] protected bool m_EnforceTwoWayCheck = true;


Line 81

if (sourceIsNull && destinationIsNull) { return false; }

var twoWayCheck = !m_EnforceTwoWayCheck || ((sourceIsNull || destinationCanGive) && (destinationIsNull ||sourceCanAdd));

return (sourceSlotCanContain && destinationSlotCanContain && sourceCanGive && destinationCanAdd && twoWayCheck);


And I ticked the new option I've added


Now this won't allow items to be added when there is no space.

(It also won't let you equip an item if you have no other space even if the item you try to equip is the exact same size as the one you are trying to unequip. That's a limitation of smart exchange. This would require making a custom drop condition/action, or some heavy refactoring on the itemCollection)

I hope that helps

 

[Cheo]

Hello and thanks for following up on this, unfortunately the issue isn't fixed on my side. Here's another video made in a new test project with the latest version of the asset and your code changes :

To summarize :
-For some reason I can't just drag and drop a weapon into the weapon slot if it's already occupied, I have to first click on the sword and select Move to be able to do that. I was able to do some direct drag and drop in the previous project that used the previous version of the asset.
-The big sword still disappears when switching it with a smaller item and not enough space in the inventory.
-The same thing happens when trying to exchange two identical items with the same size. If I understand the end of your post correctly, this is the limitation you're speaking of and it's a really serious one in my opinion. The whole issue seems to revolve around moved items still acting as blockers for ones we are trying to exchange them with as I had initially pointed out, and I don't think I'm being unfair in expecting this kind of asset to be able to handle this scenario out of the box.
-Lastly I'm attaching to this post the same scene used in my video. If you have different results on your side then by all means please let us know and I'll be ready to share any info you need or try any suggestion. Thanks.

 

[Cheo]

I took another dive today, still unable to fix the issue but have a few new things to add.

-As illustrated by the attached screenshots, IsSingularPositionAvailable tends to be needlessly run on occupied slots after free slots have already been discovered. In this situation, the left column had enough space for the 5 slots sword (slots 0,0 to 0,4 returned true) but the whole series of logs ended with a failure on 1,3 (which happens to be on the item we're trying to move into the weapon slot).

-If you enable the debug option for CanSmartExchange, you'll see that canSourceAdd is always false, regardless of whether the inventory has enough space even outside of the moved inventory item. This explains why your changes lead to the impossibility of drag and drop as we either need destinationIsNull or sourceCanAdd to be true.

-The Move action doesn't seem to have a proper exchange validity check either, and that's why I was able to perform the switch at the risk of the equipped item disappearing.

I hope these bits of information help. @Sangemdoko Could you please make this a high priority task ? This issue has been here for a while now and it's once again not a niche one, I wouldn't have expected something like this from this asset. In case you have different results on your side I'm still willing to check anything, and am open to do a screenshare if it helps. Thanks.

 

[Sangemdoko]

Hey Cheo!
Thank you for the additional details. I've now fixed this issue in my project. But it required to change a few files. So sharing the solution here on the forum is a bit complicated.

We can now move items back and forth and it won't block moving if there is space if the items are exchanged

If you don't mind waiting for the next update, it'll be included. Hopefully we can release it sometime this week

 

[Cheo]

Thanks, we'll wait for the update.

 

[hakul0]

Hey Cheo!
Thank you for the additional details. I've now fixed this issue in my project. But it required to change a few files. So sharing the solution here on the forum is a bit complicated.

We can now move items back and forth and it won't block moving if there is space if the items are exchanged

If you don't mind waiting for the next update, it'll be included. Hopefully we can release it sometime this week

Hi Santiago, how does it look? Do you plan to do the fix sometime soon?