LorenzoVulcan wrote:ElectricSolstice wrote:Hmm, think I fixed a bug with the quick patch I made. Hopefully that's all there is to it. Will send another pull request. Hard to debug though since it so slowly occurs if it is there.
This is a random error that occures very rarely,that's why i didn't understand why those values got losts.Anyway if you can fix i'll really accept the push and write your name in the credits :)
The thing is, for me, before any of my patches, the error actually occured fairly enough. After my first patch, rarely a nil lua entity would try be used as a key to the table I made, which is something my code introduced after fixing what I think the problem was. After checking that the lua entity isn't nil, it should fix the bug in my patch, which now I can't seem to find any bug in the pick up code as I play the game.
Here's what I think the problem was with the code. It's basically a concurrency problem. Now, let's say you have an item that you want to pick up on the ground, you click on it and it runs through the code till it gets to minetest.after. Well, the minetest.after is likely a seperate thread rather than sleeping two seconds to run. This means, the code continues to run and goes to the point after minetest.after after creating the thread for it. Well, even after the item entity on the ground was clicked on, that item is still a valid item that wouldn't be nil yet because the 2 seconds for the thread to collect the item hasn't ran yet. Click on that item again or maybe another in the range to be collected, and you could end up creating another thread from the minetest.after on the same item on the ground. Well, the first minetest.after would pick up the item and then make it nil once it calls item:remove. Then, when the second minetest.after thread was called on the item, it most likely would be nil by then because the previous call removed it. If the two threads ran at the same time, then you could possibly be using the same valid item twice, accounting for the unusual increase in getting more items than I should.
Here's why I think my patch fixes the problem that I think it is. My patch adds a table that the first time the item entity is hit, it gets added to that table. When the item is in the table, it's not allowed to spawn a new minetest.after thread, thus allowing the minetest.after code to be called only once on that item. The table uses item's lua entity as the key, so as long as that item is not nil, that should be the only item in the game with that key. As a result, minetest.after is only allowed to be called once on each item.