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?

 

[Sangemdoko]

Hey hakuI0
This is fixed on the dev project. Unfortunatly the official update will have to wait for a week or two as we need to sync up with the UCC update as some shared code need to be updated in both project at once.

If you need the code asap feel free to DM me and I'll share the files.

Sorry for the inconvinience

 

[Cheo]

Well this is awkward, thanks for sending the scripts in DM, but I'm sorry to report that the Item Shape Grid With Equipping And Hotbar demo scene is still a mess ! Here's a video as usual :

To summarize :
-Two swords of the same size can't be exchanged through a simple drag and drop, even with a lot of space around.
-The exchange is still possible by using the Move action.
-Even when using the Move action, the weapon slot is highlighted in red when hovering it, even if the exchange is going to succeed.
-Lastly, we can still get an item disappearing by using the Move action when there is not enough room in the grid for the equipped item.

Once again, this issue is putting us and our project in a tough spot and we really need it to be fixed. I'm still willing to check and try anything, and the screenshare suggestion still stands. Thanks.

 

[Sangemdoko]

Hey Cheo. I see in your video you are not using the correct drop action.
You should be using the ItemViewShapeDropAction

The two way restriction should be turned off if you are using the ItemViewShapeDropAction.

As for the Move action. I'm surprised it acts differently from dragging and dropping. It should be doing the exact same. In theory both approach uses the same logic, just a different input handling.

I will send you the scene again in a DM in case you made any changes there. If you continue having issues then we can compare our setups as it seems to work fine on my end

 

[Cheo]

Thanks for the quick reply, I took a look and indeed using the ItemViewShapeDrop condition and action and having the two way check disabled allows for dragging and dropping items in the weapon slot again. However we still have the item disappearing bug, and I recorded it in the scene you sent me :

So we are on the same page, on your side are you able to perform drag and drops as expected, with all free slots including the moved item's being taken into account, and no item disappearing ?

 

[Sangemdoko]

I figured out why the item was disapearing on "Move". it was not the move or the drop action. It was the Unequip action on click from the Equipment panel.
Removing that action prevent the item from disapearing when moving.
In addition I've added an additional condition on the SmartExchange drop condition for the issue you showed in the video. I'll send you the scene and the script again in DMs

 

[Cheo]

Thanks, we're making progress but still aren't done. Here's a new video :

-Successfully exchanged objects can be positioned elsewhere than where they were visualized.
-The slot from which an object is dragged has an impact on the exchange evaluation, even though the equipped item may be visualized in the same slots. In this video the great sword fails to be exchanged at the bottom of the grid if the iron sword is in the middle and grabbed from the bottom.
-When hovering an item over the equipped great sword, its visualizer can spread outside of the grid as it's always positioned where the source item is (the bottom of the video is cropped, you can see it more clearly on the attached screenshot). While we could make do with this limitation, it would be great if the item could be visualized and placed elsewhere if there is enough space. I'm fairly confident the asset already has the capacity for that.

As always, let me know if there's any discrepancy with the results on your side and if I can try anything, thanks.

 

[Cheo]

@Sangemdoko Hello, any news on this ? I have to say it again, our project's first Steam release is being on put on hold mainly because of this issue, we really need this to be fixed once and for all, and well it's not just about us, this should be fixed for the sake of the asset itself and its many other users.

 

[Sangemdoko]

Hey Cheo, Sorry I missed your earlier post with the video.

I looked into it and you are correct.
Fixing the not being allowed to exchange the items if it does not fit at the exact source index even though there is space in the grid
And fixing the adding item in the proper position is easy enough.
I can send you the files if you want.

The main difficulty is the visualization. Right now it's using the same DropPreview system as all ItemContainers and using
DropHoverIconPreviewItemView & ItemShapeDropPreviewItemView to visualize the drop. But all I can do is switch the icon to preview on the same anchor point as the current item. The difficulty here is that the itemview does not have the data to know exactly where the item will be added if it is exchanged. So there is no way to visualize it where it will go exactly.

Additionally visualizing an ItemShapeView outside of it's position can potentially cause other issues that are hard to replicate

Doing so would require some pretty in depth changes which we are not ready to do at this time as we need to take into account a lot of different use cases.

With what you and some other community members have brought up we are looking into a new proper solution, but that will take time and cause breaking changes. Some of these changes go deep into the core UIS package so it won't be for anytime soon. We will be taking our time to ensure the quality and stability is on par with the rest of our solutions. I hope you understand

If you wish to attempt it yourself for you specific use case then you may want to look into making a custom DropHoverIconPreviewItemView & ItemShapeDropPreviewItemView and potentiall ItemViewDropHandler & ItemViewDropContainerSmartExchangeAction.

I'll send you the files with the code fixes I mentioned above, I hope that at least helps with your release.

Best of luck

 

[Cheo]

Thanks for the quick reply, I tested the scripts you sent me and confirm that the drag and drop exchange now works as expected both in the demo scene and our project, thanks ! The visualizer issues are for now a secondary issue, that's alright.

There is unfortunately one last similar issue I have to ask you help with, which are the Equip/Unequip actions. More specifically, the MoveToCollectionItemAction and CharacterEquipUnequipItemAction ones. The latter one in particular is relevant to our project as we use the UCC-UIS integration. After implementing your fixes the drag and drop exchange, I realized that it was still possible to get an item disappearing by selecting the Equip action on a grid item while the equipped item doesn't have enough space to be put in the grid. It is worth mentioning however that the exchange was prevented when selecting Unequip on the equipped item.

From what I've seen, it seems that no inventory check similar to the smart exchange one is performed in any of these actions, nor even in CharacterInventoryBridge.MoveItemToEquippable. I tried modifying this function with ChatGPT's help but only ended up with items being cloned in the grid. I am aware that I already gave you a lot of work with all this, but would nonetheless request an inventory space check option for these actions. Just like with the drag and drop, I don't think that this is an edge case and you would expect such actions to be prevented in your average RPG if the inventory didn't have enough space to realistically perform an exchange.

Lastly I should mention that the item disappearing when using Equip issue is reproducible in the demo scene. Once again, if I need to make a new video, share more details or try something out, please let me know. Thanks.