Use standard pairs/ipairs implementation #137
Conversation
|
Thanks a lot for the contribution! I'll take a closer look at it as soon as possible. I'd like to see some feedback from others who are using the Lua plugin. |
|
@mikke89 I will be able to test this as soon as I get my wifi back :p However, I will say that in our copy of RmlUi we had commented out the RmlLua code which used to override the |
|
@IronicallySerious Great, I'll hold on for your comments then :) |
|
This seems to be working well! @mikke89 .
.
.
<head>
<script>
Demo = Demo or {}
function Demo.toggle(document, enabled)
for k,v in pairs(document:GetElementById("rootex_button").attributes) do
print(k .. ": " .. v)
end
print("Hi")
end
</script>
</head>
<body>
<p id="transition_test"> This is a sample! </p>
<br />
<img
id="rootex_button"
onmousedown="Demo.toggle(document, true)"
onmouseup="Demo.toggle(document, false)"
src="../rootex.png"
width=100
height=100
/>
</body>The output I get is: |
|
@IronicallySerious Thanks for testing! @actboy168 Thanks again for this long needed change. I found a few issues while testing this, which needs to be fixed before I can merge it: This example is taken from the documentation: function OnClick()
local element = g_document:GetElementById('button')
for i,child in ipairs(element.child_nodes) do
address = child.tag_name
if child.id ~= '' then
address = address .. '#' .. child.id
end
for token in string.gmatch(child.class_name, '[%w]+') do
address = address .. '.' .. token
end
print(i .. address)
end
endWith the following RML: <button id="button"><span id="test">Exit</span><p>Hello</p><div test="5">A</div></button>It's missing the first child element. Ie, now it outputs: Previously: This also appears to be missing the first option: function Test(document)
local element = Element.As.ElementFormControlSelect(document:GetElementById('color'))
for i,entry in pairs(element.options) do
print(i .. ': ' .. entry.value)
end
endRML: <select name="colour" id="color">
<option value="233,116,81">Burnt Sienna</option>
<option value="127,255,0">Chartreuse</option>
<option value="21,96,189">Denim</option>
<option value="246,74,138">French Rose</option>
<option value="255,0,255">Fuschia</option>
<option value="218,165,32">Goldenrod</option>
<option selected value="255,255,240">Ivory</option>
</select>Output: This results in an infinite loop: for i,context in ipairs(rmlui.contexts) do
print('Context ' .. i .. ': ' .. context.name)
end |
|
These problems seem to be caused by the difference between the definition of __index and the usage habits of lua. Unlike C++, Lua's array index starts from 1, but now the array index constructed from rmlui starts from 0. So ipairs will go wrong. I think the behavior of __index should be modified, otherwise element[0] represents the first element which is also a confusing thing for Lua users. For infinite loops, this is because the context's __index method always returns the last element for indexes larger than the size of the array, which makes ipairs think that this is an infinite array. |
|
@mikke89 If you think it is worth modifying, I will provide a patch. |
|
I think following the lua practices by RmlLua would be better :) I mean everything in the container is base 1 instead of base 0. |
|
I agree with you guys, it seems appropriate to use 1-indexing for the Lua code. @actboy168 Please feel free to patch this, thanks! |
|
@mikke89 I have updated the patch to solve the above problem. |
|
Thank you! Looks good to me now. |
|
This change is awesome! |
This change removes the replacement of ipairs and pairs in the Lua standard library.
For ipairs, the following two pieces of code are equivalent. So we only need to implement
__indexcorrectly to make ipairs work correctly. In fact,__ipairshas been removed after Lua5.3. So we no longer need to implement__ipairs.In addition, this change also resolves #95.