Flaw in setLabels implementation (movieclip.lua)

In the movieclip library, it’s possible to define labels for animation frames so they can be referenced by strings instead of indices, but it appears that the implementation is flawed because the “string” part of the function iterates through all the labels to find the right one; why not just perform a hash lookup?

Original (flawed) definition - version 2.1 of movieclip.lua
[lua]-- use a label
function g:stopAtFrame(label)
if (type(label) == “number”) then
Runtime:removeEventListener( “enterFrame”, self )
animFrames[currentFrame].isVisible = false
currentFrame = label
animFrames[currentFrame].isVisible = true
elseif (type(label) == “string”) then
for k, v in next, animLabels do – BAD: ITERATION!
if (v == label) then – BAD: UNNECESSARY!
Runtime:removeEventListener( “enterFrame”, self )
animFrames[currentFrame].isVisible = false
currentFrame = k
animFrames[currentFrame].isVisible = true
end
end
end
end

– define labels
function g:setLabels(labelTable)
for k, v in next, labelTable do
if (type(k) == “string”) then
animLabels[v] = k
end
end
end[/lua]
Proposed changes:
[lua]-- use a label or frame index
function g:stopAtFrame(label)
if (type(label) == “string”) then
label = animLabels[label]
end

Runtime:removeEventListener( “enterFrame”, self )
animFrames[currentFrame].isVisible = false
currentFrame = label
animFrames[currentFrame].isVisible = true
end

function g:playAtFrame(label)
self.stopAtFrame(label)
self.repeatFunction = nextFrame
Runtime:addEventListener( “enterFrame”, self )
end

– define labels
function g:setLabels(labelTable)
for k, v in next, labelTable do
animLabels[v] = tonumber(k)
end
end[/lua]

This appears to work properly in my tests; here’s some sample code.
[lua]local movieclip = require(“movieclip”)
anim = movieclip.newAnim{“1.png”, “2.png”, “3.png”}
anim.x = 100
anim.y = 100
anim:setLabels{“foo”, “bar”, “baz”}
anim:stopAtFrame(“baz”)

local function update(e)
anim:stopAtFrame(“bar”)
end
timer.performWithDelay(2000, update, 1)[/lua]

Seem reasonable?
[import]uid: 69142 topic_id: 26808 reply_id: 326808[/import]