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

WIP: Fix ipairs() for enum attrs #1973

Draft
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

lethosor
Copy link
Member

@lethosor lethosor commented Sep 29, 2021

Fixes #1860

(Note: I had put this together but evidently never finished it - need to investigate what is missing)

Edit: looks like I didn't make the test pass - that sounds familiar to me now.

@lethosor lethosor self-assigned this Sep 29, 2021
@myk002 myk002 added this to To Do in 0.47.05-r5 Mar 10, 2022
@myk002 myk002 removed this from To Do in 0.47.05-r5 May 1, 2022
@myk002
Copy link
Member

myk002 commented May 13, 2022

this does address a legitimate issue. do you think you will be able to get this done within the release cycle?

@lethosor
Copy link
Member Author

lethosor commented Jun 4, 2022

Looking at this again. The Lua layer here is a bit complicated, so no promises.

@lethosor
Copy link
Member Author

lethosor commented Jun 4, 2022

I looked into this some. Turns out this is broken because I'm calling an existing ipairs() implementation that uses rawget():

lua_rawgeti(L, lua_upvalueindex(1), i);

but the attrs table is not actually populated, and instead uses the __index method to return values, which rawget() ignores:

dfhack/library/LuaWrapper.cpp

Lines 1534 to 1536 in f290b1c

lua_pushcclosure(state, meta_enum_attr_index, 3);
freeze_table(state, false, (eid->getFullName()+".attrs").c_str());

so a solution would likely involve a custom ipairs() implementation or changing attrs so that it is populated for all valid enum items.

By the way, the reason for this implementation is probably because indexing attrs with an invalid index returns a table of the default attribute values. So I am leaning towards the former as a solution (a custom ipairs() instead of reusing wtype_ipairs()).

@myk002 myk002 added this to In progress in 0.47.05-r6 via automation Jun 5, 2022
@myk002 myk002 removed this from In progress in 0.47.05-r6 Jun 13, 2022
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.

ipairs() called on enum attr tables never ends
2 participants