Monolith DKP

Monolith DKP

687k Downloads

Cancel bid does not cancel all bids but some weird subset of them

f90 opened this issue ยท 8 comments

commented

When under-cutting is allowed and multiple bids are made by one person, and the person then uses !bid cancel, instead of deleting ALL bids from the list of bids shown to the PM, it deletes SOME bids while not deleting others, without any apparent logic.

commented

There should only be 1 bid up for a person at any given time. If they bid higher, it removes their last bid and adds the new one. Canceling a bid just cancels that individuals bid.

commented

Ah makes sense. But then the bug is that I can still see all the other bids made by the same person that are lower than his/her current one, they should be deleted right away!

commented

I can confirm Roe that's true, we also see all bids and its impossible to say which is the newest

commented

That's unusual. When a new bid is received the lower bid should be immediately removed from the bid table. I'll check why it might not be.

commented

@Roeshambo
We have also noticed that not always bids are actually removed when bid >= than the new incoming.
I think this is the faulty line:
Bidding.lua:163

if (mode ~= "Zero Sum" and MonDKP_DB.modes.ZeroSumBidType ~= "Static") and Bids_Submitted[i] and Bids_Submitted[i].player == name and Bids_Submitted[i].bid < cmd then

The first conditional actually requires both to be true, so if you have mode other than ZeroSum but ZeroSumBidType is still Static (probably value not cleared when changing from zerosum static to min bid value, etc.) then this will not execute.
Excerpt from my Variable:

	["modes"] = {
		["SameZoneOnly"] = false,
		["ZeroSumBidType"] = "Static",
                ...
		["mode"] = "Minimum Bid Values",

Other thing is that in lines 134-136 you iterate over

Bids_Submitted
          if Bids_Submitted[i] and Bids_Submitted[i].player == name then
            table.remove(Bids_Submitted, i)
            if MonDKP_DB.modes.BroadcastBids then

AND at the same time you are editing the bids_submitted table. This is actually undefined behavior (not safe) in LUA language. this can be issue for @f90 . This is because you change table size while iterating over it. I would suggest doing a copy of the table, while iterating, but with only values not from the player and then substitute.
e.g. change:

        for i=1, #Bids_Submitted do           -- !bid cancel will cancel their bid
          if Bids_Submitted[i] and Bids_Submitted[i].player == name then
            table.remove(Bids_Submitted, i)

to

Bids_SubmittedNew = {}
        for i=1, #Bids_Submitted do           -- !bid cancel will cancel their bid
          if Bids_Submitted[i] and Bids_Submitted[i].player ~= name then
            table.insert(Bids_SubmittedNew, Bids_Submitted[i])
...
Bids_Submitted = Bids_SubmittedNew

You can also use dual iteration https://forums.coronalabs.com/topic/11057-how-can-i-delete-objects-from-a-table-while-iterating-through-it/

commented

Fixed in #338

commented

Closing this for now assuming pull request #338 fixed it, can only verify once it's on live though

commented

Actually noticed that the faulty code (iterating over table while deleting its entries) is still not changed, @lantisnt you suggested a code change, do you want to implement that as well?