fa icon indicating copy to clipboard operation
fa copied to clipboard

Some features of Lua in FA

Open KionX opened this issue 3 years ago • 1 comments

If you add the following lines to config.lua: metacleanup(false) metacleanup(dummy) , hidden errors will be detected. And if you also uncomment __index, then there will be an indecent lot of errors. Inappropriate interaction with the metatable of type.

Example: local a=false(or func) a.x=123 print(a.x) -> 123 local b=false(same type as a) print(b.x) -> 123

Another feature of FA: print(nil>0) -> false print(nil<0) -> true print(nil==0) -> false print(nil~=0) -> true print(false>nil) -> true print(false<0) -> true In case of type inequality, their indexes are compared.

KionX avatar Feb 14 '22 17:02 KionX

Also as feature Faforever/lua-lang#7, preallocating memory for tables

4z0t avatar Mar 27 '22 18:03 4z0t

metacleanup(false) huh, looks like I ran across this as well :) metacleanup(dummy) I'm not sure what this would do though And if you also uncomment __index, then there will be an indecent lot of errors. At least for nil, allowing field access is so very useful in chaining and vastly cleans up code. Disabling the rest of the types also keeps the codebase neat somewhat by removing the need for table checks, since the important bit isn't that it's a table, but that it has the attribute. Because it returns nil anyway, it ends up with the correct behavior regardless.

Hdt80bro avatar Nov 10 '22 06:11 Hdt80bro

I actually see this as a problem, not a solution. It may hide some bugs like this. I also don't know how it affects performance.

4z0t avatar Nov 10 '22 10:11 4z0t

If we decided to make nil access a bug (and introducing bugs isn't recommended - more on why I think it's a bug introduction [which are bad] instead of an error introduction [which are useful] in a bit) then the equivalent code for

local d = a.b.c.d
if d then
    -- do something with d
end

would be have to be changed to

if a then
    local b = a.b
    if b then
        local c = b.c
        if c then
            local d = c.d
            if d then
                -- do something with d
            end
        end
    end
end

As you can imagine, this unmaintainable code will only lead to more problems. Since the result is exactly the same when any fields are nil, the behavior is identical. Thus, I don't see any benefit in making nil access raise errors, and see several reasons it would introduce bugs. The only kinds of bugs I can this hiding are instances where the behavior needed to be different in the first place, and thus already have to do something different - there might be some usage for nil besides table chaining that doesn't behave identically that I'm not thinking of though, so I'm interested to see what you have in mind.

The other thing to consider is that it's unlikely that we would catch all instances of this pattern in our codebase, so we would probably introduce more bugs if we tried this. Regardless, mod compatibility means we already can't, so it's all hypothetical at this point :)

Performance-wise, it should be on par with or better than manually unrolling the nil-coalescing table access (based only on theory though). In the case that one of the early fields is nil, the above unrolled code removes the later table accesses by failing fast. However, it also does just as many checks for nil as it does table accesses, so it should (again, tentatively) be slower when more fields are present. And introduces more local variables. However, even that being so, there are already many awful instances of table accesses being abused in our codebase that decrease performance and readability, so the performance of a simple pattern like this is not important.

Hdt80bro avatar Nov 11 '22 20:11 Hdt80bro