fix: Use concurrent queue and dictionary to avoid race conditions.

[shanecelis]

With Unity2019.1.0a13 the example scene had failures with WorksQueue.Dequeue() initially. Presumably this is because the WorksQueue.Count test and Dequeue() happen at different times. To fix this, I switched WorksQueue to a ConcurrentQueue and use TryDequeue() which dequeues atomically if there is an item available and returns true; otherwise, it returns false.

After that was fixed, a new error popped up when the trees ran out. This appeared to be caused by a race condition between the test for HasKey and Get. To fix this, I switched the dictionaries in ReGoapState to ConcurrentDictionary classes. And I removed Get() in favor of TryGetValue() which avoids that race condition.

The example scene works. All unit tests pass.

I hope these bugs mean that Unity is using more threads than it was before.

[wqaetly]

First of all, thank you for your contribution to ReGoap, but when I use your library, I have a little problem. Take this unit test as an example Please pay attention to the cost of each Action Then we do this unit test again We found that the cost of each Action is inherited from the previous unit test, so that in multiple plannings to achieve certain complex goals, it will produce incorrect results because of the incorrect value.

[wqaetly]

Okay, forgive my offense, I tried to do a test in the original warehouse, the same result。。。

[shanecelis]

I'm seeing the same behavior which you've found in Unity2019.3.10f1. It'd be nice to make a failing test out of this. However, if I roll back to the prior commit 506600c, I get the same behavior where the costs increase. I'd advise making a failing test, then running git bisect to find the failing commit. It doesn't seem like it's related to my changes.

If I were to guess, I'd guess some static variables that are being kept around that should be cleared.

[wqaetly]

In fact, it is the problem of cachedStates and cachedNodes, it will cause the next test to get the cost of the previous test

[shanecelis]

Ha, I understand your comment now. Cheers.

[wqaetly]

Excellent!

[wqaetly]

All we need to do is add a sentence of g = 0 to Recycle () of ReGoapNode.cs Then no matter how many times we test, their cost is correct

[wqaetly]

Speaking of which, in fact, I always have a question, I would like to ask you here Still this example, the "ChopTree" action costs 2，"WorksWood" action costs 5，"MineOre" action costs 10，"SmeltOre" action costs 10，So the result we wanted was ChopTree->WorksWood->MineOre->SmeltOre，but the result given by ReGoap is not what we want。 I have seen the A * algorithm in the ReGoap library and understand how he got this result, but what I really want to ask is how to set the cost if we want the planning result to be exactly as we thought Value, if you can, please tell me how to do it, or use the example above， I'm so grateful to you

[jswigart]

If you want them to prioritize them to gather an expend 1 resource at a time I would probably increase the cost of gather actions if they already have items of another type. You can't really enforce an order of operations without basically representing it in the form of cost modifications, otherwise to A* your plan is equally as good as the plan it came up with cost wise, and the result you get out is largely dependent on the order the actions were evaluated in.

[wqaetly]

If you want them to prioritize them to gather an expend 1 resource at a time I would probably increase the cost of gather actions if they already have items of another type. You can't really enforce an order of operations without basically representing it in the form of cost modifications, otherwise to A* your plan is equally as good as the plan it came up with cost wise, and the result you get out is largely dependent on the order the actions were evaluated in.

thank you for your reply Can I understand you this way? We should not rely on the value of Cost to achieve the planning order we want, but need to increase the conditions, as I did in the picture below

[jswigart]

Yes, in principle it should be possible to address it that way. If you have multiple actions, one that is higher cost if carrying something already, and the other that is low cost if carrying nothing, that should allow you to bias toward not carrying stuff around and instead towards the actions which make use of that carried resource right away.

[jswigart]

Another possibility could be to generalize it to a "hasRoomInInventory" which you might set to 1 slot, effectively providing the same benefit without needing to do it for each resource type, but it would also give you the flexibility to give an AI multiple inventory slots that he could carry multiple things at a time. If coupled with movement based action evaluations it can generate a most optimal plan, though most people don't integrate movement evaluation into their action planning systems.

[wqaetly]

Another possibility could be to generalize it to a "hasRoomInInventory" which you might set to 1 slot, effectively providing the same benefit without needing to do it for each resource type, but it would also give you the flexibility to give an AI multiple inventory slots that he could carry multiple things at a time. If coupled with movement based action evaluations it can generate a most optimal plan, though most people don't integrate movement evaluation into their action planning systems.

emm, sorry, I didn't understand your hasRoomInInventory statement。。。 Can you give an example？ Still want this order that ChopTree->WorksWood->MineOre->SmeltOre

[jswigart]

What I mean is that you have a condition for whether you "hasWood" or "hasRawWood", etc but no conditions that act as a limitation on whether something can be acquired, like a condition based inventory.

Picking up something(anything) could set inventorySpace to false, and actions that refine that resource or otherwise drop it off can turn it back to true. That would act as a more generalized mechanism for controlling the order.

Its probably possible to use an integer base inventory slots condition as well, but a bit more complicated.

[wqaetly]

What I mean is that you have a condition for whether you "hasWood" or "hasRawWood", etc but no conditions that act as a limitation on whether something can be acquired, like a condition based inventory.

Picking up something(anything) could set inventorySpace to false, and actions that refine that resource or otherwise drop it off can turn it back to true. That would act as a more generalized mechanism for controlling the order.

Its probably possible to use an integer base inventory slots condition as well, but a bit more complicated.

Okay, I understand now, thanks

[luxkun]

Accepted and also fixed the issue with the g, thank you both! @shanecelis and @wqaetly