Check for tables for default in AuthorOptions

[adamhl8]

Fixes #998

[emptyrivers]

That will work, but it should be recursive to be fully correct.

[InfusOnWoW]

This doesn't work because the logic is the wrong way around. You want to check if the tables are different.

Also this will not work if you are comparing two indentical values, as in that case compareValues is called with two values, and calling pairs on that will result in a error.

[adamhl8]

ef5be30

I believe that should do it. It's recursive and fixes the logic for checking if the values match.

[InfusOnWoW]

The loop needs to continue if the values are equal. It should only return false if the values are not equal.
Also your function also checks for equality for non-table values, so the original check "option.default ~= data.config[option.key]" should not be necessary.

We might have a table comparing equal function already somewhere.

[adamhl8]

Gotcha, sorry. Latest commit should (finally) be correct.

[emptyrivers]

 if option.key ~= nil and option.default ~= nil then
-  if tablesAreEqual(option.default, data.config[option.key]) then
-  elseif option.default ~= data.config[option.key] then
+ if not tablesAreEqual(option.default, data.config[option.key]) then
    return false
  end
end

[emptyrivers]

That's still not quite correct, but your if branch was too complex.

[adamhl8]

Oh, yeah that's more simple.

Why is that not correct? With that, it still returns true/false as it should, right (looking at the example inputs from my latest comment)?

[emptyrivers]

Well, for one thing, you can simply check that the option class is not noninteractive (see the top of the file), instead of checking that the key and default aren't nil.

[InfusOnWoW]

Right it's indeed correct. But that's not how that check should be written. A if, whose only purpose is to act as a additional check for the elseif branch is unnecessarily complex.

[adamhl8]

That's why it's only exiting the loop on finding something unequal, but continues when the values are the same.
For tables, the same must happen. If the tables are equal, continue with the next option. If they aren't equal, we found a option that isn't on the default vaue and should exit that function.

With that latest commit, it does only exit the loop when it finds something unequal.

For example:

Say the function is passed 5, 5.
tablesAreEqual returns true, loop continues.

Say it's passed 5, 7.
tablesAreEqual returns false, elseif option.default ~= data.config[option.key] is checked (5 ~= 7), function returns false, as it should.

Pass {1, 2}, {1, 2}.
Same thing, tablesAreEqual returns true, loop continues.

Pass {1, 2}, {1, 3}.
tablesAreEqual returns false, elseif option.default ~= data.config[option.key] is checked (tables are unique), function returns false, as it should.

Apologies if I'm being difficult, but I'm fairly sure that's correct.

Edit:

Main thing is that if tablesAreEqual returns true, then the elseif is NOT checked, so the loop only exits if the two options are different.

[adamhl8]

Alright, that's my last try. If that doesn't work I'll give up and let you guys do it :P

[emptyrivers]

I pushed a fix to your whitespace (you should install editorconfig so that it's taken care of for you)

[InfusOnWoW]

Thanks, merged now.