Subtle bug/unintended-behavior in lime-tile.lua in Tile:create()

Lime
#1

The relevant code segment in lime-tile.lua in Tile:create() is the following:
[lua]…
for i=1, #properties, 1 do

– Read in the Config file data if it has one, otherwise it is a normal property
if properties[i].name == “configFile” then
readInConfigFile(properties[i].value, self)
else
self:addProperty(properties[i])
end

end
…[/lua]

If I understand it well, then the Tiled-set properties are read-in, and made into Lime-properties for the tile.

The issue is with the sequence, where the “configFile” property is treated like any other, which means that those properties read before the config one is encountered will potentially be overwritten by the config-file props, while those that are read after the config file is dealt with, will overwrite those set by the config file props…

You probably want to first read and deal with the configFile before you go over the other Tiled props, other wise you have a behavior where the set property value depends on whether it is encountered before or after the configFile-property…

A possible solutions is to loop twice - first to deal with any configFile, then to deal with the other props:

[lua]…
for i=1, #properties, 1 do
– Read in the Config file data if it has one, otherwise it is a normal property
if properties[i].name == “configFile” then
readInConfigFile(properties[i].value, self)
break
end
end

for i=1, #properties, 1 do
– Read in the Config file data if it has one, otherwise it is a normal property
if properties[i].name ~= “configFile” then
self:addProperty(properties[i])
end
end
…[/lua]

-Frank.
-Frank.

[import]uid: 8093 topic_id: 6896 reply_id: 306896[/import]

#2

Ough… that same code-pattern is used for all the new/create functions for tile/tieset/object,etc.

So if I’m right about this subtle bug, then it requires changes in multiple files.

-FS.
[import]uid: 8093 topic_id: 6896 reply_id: 24102[/import]

#3

I’ve made the following change to readInConfigFile() in lime-utils.lua:

[lua] …
if configData then

– Set all the lovely new properties on the object
– First deal with the configFiles before the “normal” props
if(configData[“configFiles”])then
value = configData[“configFiles”]
for i=0, #value, 1 do
readInConfigFile(value[i], object)
end
end
for key, value in pairs(configData) do
if key ~= “configFiles” then
object:setProperty(key, value)
end
end

if(lime.isDebugModeEnabled()) then
print("Lime-Lychee: Loaded Config File - " … path)
end
end
…[/lua]

(I’ve seen some weird behavior with props defined in config files… but this didn’t solve it… but I believe it fixes the before mentioned inconsistent setting of properties…)

-Frank.
[import]uid: 8093 topic_id: 6896 reply_id: 24106[/import]

#4

Hey, thanks once again for fixing my sillyness :slight_smile: [import]uid: 5833 topic_id: 6896 reply_id: 24112[/import]

#5

I believe that this issue with not reading-in the config files first before assigning the properties, has been addressed in lime-utils.lua, but not yet in lime-tile.lua (Tile:create) and neither in the other modules that parse the xml-attribute data…

Is that an oversight maybe?

Regards, Frank.
[import]uid: 8093 topic_id: 6896 reply_id: 25369[/import]

#6

The changes added into 2.9 were last minute changes just to try to stop some of the coercion however I have most likely (as you have now found) missed a few cases. In version 3.0 that is coming out on Friday I have switched it all over to JSON stuff to help remove the need for any coercion. [import]uid: 5833 topic_id: 6896 reply_id: 25432[/import]

#7

Sounds good - love big version number changes :wink: [import]uid: 8093 topic_id: 6896 reply_id: 25442[/import]

#8

Me too, makes me feel like Lime is progressing really fast :slight_smile: [import]uid: 5833 topic_id: 6896 reply_id: 25452[/import]