Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust implementation of table functions #98

Merged
merged 11 commits into from
Nov 16, 2024

Conversation

4z0t
Copy link
Member

@4z0t 4z0t commented Nov 10, 2024

@4z0t 4z0t assigned 4z0t and unassigned 4z0t Nov 10, 2024
@4z0t 4z0t requested review from Garanas and Hdt80bro November 10, 2024 10:18
@Garanas
Copy link
Member

Garanas commented Nov 11, 2024

I've done a few AI vs AI games and they appear to run fine. I'll do a few manual tests next.

@4z0t
Copy link
Member Author

4z0t commented Nov 14, 2024

Made it compatible with Python patcher


const luaL_reg RegTableFuncsDesc[] = {{"getsize2", &lua_tablesize},
{"empty2", &lua_tableempty},
{"getn2", (lua_CFunction)0x00927C20},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need these 2 alternatives?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can remove them.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is Lua resets them in current dev branch:

--- table.empty(t) returns true iff t has no keys/values.
---@param t table
---@return boolean
function table.empty(t)
    if type(t) ~= 'table' then return true end
    return next(t) == nil
end

--- Returns actual size of a table, including string keys
---@param t table
---@return number
function table.getsize(t)
    if type(t) ~= 'table' then return 0 end
    local size = 0
    for k, v in t do
        size = size + 1
    end
    return size
end

-- replace with assembly implementations
table.empty = table.empty2 or table.empty
table.getsize = table.getsize2 or table.getsize

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then we remove that logic. As long as the assembly versions are functionally equivalent then that should be no issue.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See also: FAForever/fa#6537

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@4z0t
Copy link
Member Author

4z0t commented Nov 16, 2024

If we want to have deepcopy here is how it was done in my dev branch https://github.com/4z0t/FA-Binary-Patches/blob/develop/section/Lua/LuaObject.cxx#L87-L121

@Garanas Garanas merged commit 85beeb2 into FAForever:master Nov 16, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants