Strange for loop problem

[tdc5013]

Hi Guys, I'm having a bit of trouble with some code. At the beginning of my game (a card game) players are allowed to switch out certain cards from one 'reserve' and their hand. The reserve cards are kept until the end. I have a function where the AI opponent looks to reserve only the most powerful cards. Here's the code:

Code: Select all

	
	if( GLOBAL_VARS.GAME_STATE == GLOBAL_VARS.CARD_CHANGE ) then
		table.sort(GLOBAL_VARS.Player2Store.hand, function(a,b) return a.number>b.number end)
		table.sort(GLOBAL_VARS.Player2Store.TopResvCards, function(a,b) return a.number>b.number end)
		for i = #GLOBAL_VARS.Player2Store.TopResvCards, 1, -1 do
			for j = #GLOBAL_VARS.Player2Store.hand, 1, -1 do
				if((GLOBAL_VARS.Player2Store.hand[i].Attribute and GLOBAL_VARS.Player2Store.TopResvCards[j].Attribute == nil) or
					(GLOBAL_VARS.Player2Store.hand[i].number > GLOBAL_VARS.Player2Store.TopResvCards[j].number) ) then
					
					local temp = {}
					local temp2 = {}
					
					table.insert(temp, GLOBAL_VARS.Player2Store.hand[i])
					table.insert(temp2, GLOBAL_VARS.Player2Store.TopResvCards[j])
					
					table.remove(GLOBAL_VARS.Player2Store.hand, i)
					table.remove(GLOBAL_VARS.Player2Store.TopResvCards, j)
					
					table.insert(GLOBAL_VARS.Player2Store.hand, i, temp2[1])
					table.insert(GLOBAL_VARS.Player2Store.TopResvCards, j, temp[1])
				end
			end
		end
	end

So basically, what should be happening is that in a double backwards for loop the rank (number) and whether or not the cards have attributes (some do, some don't). It will choose the highest rank or an attribute card each time. However, when the code is actually run I see that some card swaps are being done in an infinite loop. Also, it's not just the one's you might expect. For instance, if two attribute cards are being switched out all the time then that's understandable; that's something I hadn't fully factored in when coding this. But I've previously also seen it repeatedly swapping a Jack and an Ace (high), which it shouldn't be doing. Does anyone have any clue why this is happening?

Edit: Upon further inspection it appears to be cycling through a lot of other cards, not just swapping 2 around every time; more like juggling a load of cards between the two tables.

[Hexenhammer]

I am not sure I understood what exactly your code is supposed to do, but let me suggest a different coding style. No offense, but I think your style leads to hard to read code. Here is the same thing written in a different style

Code: Select all

local State = require "State"
local Player = require "Player"

local hand = Player.Two.hand
local topReserveCards = Player.Two.topReserveCards

if not State.Is(State.CardChange) then return end

local higherRank = function(a, b) return a.number > b.number end

table.sort(hand, higherRank)
table.sort(topReserveCards, higherRank)

for i = #topReserveCards, 1, -1 do
  for j = #hand, 1, -1, do
    local hCard = hand[i]
    local rCard = topReserveCards[j]

    if hCard.attribute and rCard.attribute == nil or
       hCard.number > rCard.number then

       hand[i], topRerserveCards[j] = rCard, hCard
    end

  end
end

Note that one of the cool features of Lua is that it can swap without temps. Also "and" precedes "or" so you don't need parentheses there.

I think your problem is that your algorithm is wrong, the code looked fine (if hard to read). Well, except that it seems strange to me that you use the length of the hand cards table as the limit for iterating over the reserve cards table (and vice versa)..

[tdc5013]

Hey, thanks for the reply.

No offense taken, I realize it looks a little sloppy. I had a false memory of having problems when equating tables to local variables that I see now was, well, false...

Also "and" precedes "or" so you don't need parentheses there.

I didn't realize it worked that way, thanks for the info.

Code: Select all

local higherRank = function(a, b) return a.number > b.number end

Again, good idea. I pretty much ripped that straight off the lua website.

I think your problem is that your algorithm is wrong

What part of it would be wrong? What I'm try to do is have the AI find the strongest cards out of two tables of equal length. I wanted all cards to be compared so I used a double for loop. In the if statement I'm basically saying "if either the current card in hand has an attribute and the reserve card doesn't, or the hand card is of higher rank than the reserve card, then swap those cards". As far as I can see it should go through once and then just stop as it should get to a point where almost all cards in the hand are weaker than their counterparts in reserve.

[Hexenhammer]

What I'm try to do is have the AI find the strongest cards out of two tables of equal length

Ok, that's an easy problem. I ran the loop with some test data:

Code: Select all

local hand = {
  { attribute = true, number = 3},
  { attribute = true, number = 4},
  { attribute = nil,  number = 5},
  { attribute = true, number = 9}
}


local topReserveCards = {
  { attribute = true, number = 1},
  { attribute = true, number = 2},
  { attribute = nil, number = 7},
  { attribute = nil, number = 6}
}



function love.load()

  local higherRank = function(a, b) return a.number > b.number end

  table.sort(hand, higherRank)
  table.sort(topReserveCards, higherRank)

  for i = #topReserveCards, 1, -1 do
    for j = #hand, 1, -1 do
      local hCard = hand[i]
      local rCard = topReserveCards[j]

      if hCard.attribute and rCard.attribute == nil or
        hCard.number > rCard.number then

        hand[i], topReserveCards[j] = rCard, hCard
      end
    end
  end

end


function love.draw()

  for i = 1, #hand do
    love.graphics.print(hand[i].number, 10 * i, 30 )
  end

  for i = 1, #topReserveCards do
    love.graphics.print(topReserveCards[i].number, 10 * i, 60 )
  end


end

It worked.. kinda. I mean I did not get an infinite loop and I don't see how one could be triggered either. You should upload your entire source code otherwise I am at the end of my rope here.

I said "kinda" because I assume attribute cards are actually meant to be stronger than non-attribute cards of higher rank. If this is the intention the algorithm does not work correctly. You might want to try this alternative method, maybe it will solve your strange infinite loop problem too:

Code: Select all

local hand = {
  { attribute = true, number = 3},
  { attribute = true, number = 4},
  { attribute = nil,  number = 5},
  { attribute = true, number = 9}
}


local topReserveCards = {
  { attribute = true, number = 1},
  { attribute = true, number = 2},
  { attribute = nil, number = 7},
  { attribute = nil, number = 6}
}



function love.load()

  -- combine both decks
  all = {}
  for _, card in pairs(hand) do all[#all+1] = card end
  for _, card in pairs(topReserveCards) do all[#all+1] = card end

  -- sort combined deck
  local stronger = function(a, b)
      -- attribute cards are always stronger
      -- if both cards are attribute cards sort by rank
      if a.attribute and b.attribute then
        return a.number > b.number
      end

      if a.attribute then return true end
      return false
  end

  table.sort(all, stronger)

  -- split combined deck into two decks again
  for i = 1, #hand do hand[i] = all[i] end
  for i = 1, #topReserveCards do topReserveCards[i] = all[#hand+i] end

end


function love.draw()

  for i = 1, #hand do
    love.graphics.print(hand[i].number, 10 * i, 30 )
  end

  for i = 1, #topReserveCards do
    love.graphics.print(topReserveCards[i].number, 10 * i, 60 )
  end

end

[tdc5013]

Ok, will do. Oh and rest assured I cleaned the code up a little in the meantime. And ignore the semicolons, Lua isn't my only language so it's a useful habit.

EDIT: Sorry, I didn't see the second half of your post. I'll try your suggestions and get back to you, thanks!

[Hexenhammer]

Yes, try my alternative method because I can't find the cause of the infinite loop in your uploaded source.

[tdc5013]

Hey again,

Problem solved you'll be happy to hear. The function was being called repeatedly after the first run through, I put a quick bit of boolean code in there and it seems to have solved the problem. So thanks for your help, but if I could ask a few questions about your code just to better my understanding of it. This part of the local sorting function

Code: Select all

if a.Attribute then return true end
return false;

What exactly is this doing? Why is it returning true?

Also I'd like to try and work in my original idea for picking high cards over low ones, or will it already do this? If there were a situation where there is only one attribute card, would it pick that card and then whatever the highest ranked cards are after that point? If not would something like this as an addition statement in the function be a solution:

Code: Select all

if a.Attribute == nil and b.Attribute == nil then
	return a.number > b.number
end

Thanks again, you've been a big help.

[Hexenhammer]

Code: Select all

if a.Attribute then return true end
return false;

What exactly is this doing? Why is it returning true?

As you said it's in the sorting function. If a sorting function(a,b) returns true that means "put a before b" (while returning false means the opposite).
Here is a step by step explanation how the function works.

Code: Select all

      if a.attribute and b.attribute then
        return a.number > b.number
      end

This condition is only true if both cards are attribute cards. At least as far as I could see you mark non-attribute cards by setting their .attribute field to nil. nil is always false in Lua.
IF both cards are attribute cards we compare the rank and return the result of that. "return a.number > b.number" will return true only if a has a higher rank than b i.e. it practically means "put higher ranking cards before lower ranking ones".

Code: Select all

    if a.attribute then return true end

Remember that at this point we already know that at least one of the compared cards is not an attribute card. Because if both were attribute cards the function would have already returned (see above).
So we only need to check whether (a) is an attribute card. If (a) is an attribute card that means that (b) is not an attribute card. We return true at this point because the function is meant to put attribute cards before non-attribute ones. And remember "return true" = "put a before b".

Code: Select all

return false

This line of code is only reached if neither of the above conditionals is true. We return false here i.e. "put b before a". Because if both conditionals evaluated to false that means:
- One card is an attribute card, the other isn't
- (a) is not the attribute card
That means: (b) must be an attribute card.. and we want to put attribute cards before non-attribute cards thus we return false i.e. "put b before a".

Also I'd like to try and work in my original idea for picking high cards over low ones, or will it already do this?

See above. Yes, it puts high cards in front of low ones. However the "attribute card before non-attribute card" rule is more important i.e. you get the order: attribute cards (sorted by rank, highest first) followed by non-attribute cards (again sorted by rank, highest first)

If there were a situation where there is only one attribute card, would it pick that card and then whatever the highest ranked cards are after that point?

Yes, that's what it does.

[Hexenhammer]

Oops, wait! It does not handle the case where a non-attribute card is compared against another non-attribute card correctly. Here is the fixed version

Code: Select all

  local stronger = function(a, b)

      if a.attribute and b.attribute or
        not a.attribute and not b.attribute then

        return a.number > b.number
      end

      if a.attribute then return true end

      return false

  end

This should - finally - do everything you want.

[tdc5013]

Thank you very much, once more. It's people like you whole keep these forums alive.