Suggestion: use json as the format for all property values - also as defined in Tiled

Lime
#1

Right now, the prop-values in Tiled are somewhat loosely defined - they are all essentially strings, but some of them are sometimes automatically converted to numbers or booleans, and some strings could be lists… some of the code relies on Lua’s auto-conversion, which does what you intend most of the time, but snags you when you don’t expect it. This ambiguity leads to subtle errors and patch-code that sometimes converts “true” into true, but not always, and you can only do it when you “know” that the user meant true and not “true”. (I’ve been bitten by this a few times already)

The config files for Lime, however, use json throughout, and there is no ambiguity - nice and clean.

My suggestion is to start using json also for the Tiled prop-values.
This would allow you to specify the difference between true and “true”, 42 and “42”, null as nil, lists as [1,2,3], etc.,etc.

It would also make the format specification between Tiled props and config files consistent.

The only issue is that it would break much of what is already there if you would simply make the switch…

Unless… you would define a special Tiled property, like “*JSON-FORMAT*”, which would indicate to the Lime parser that all property values defined for this Tiled-bugger are defined in json-format.
(Lime would have to look for this special prop first before it starts parsing and interpreting the props-values of the XML…). Kind of a hack to provide backwards compatibility.

Comments?

-FrankS.

[import]uid: 8093 topic_id: 6937 reply_id: 306937[/import]

#2

Also a small step towards less-ambiguity would be to specify some strict rules that you will adhere to while parsing Tiled-prop values. Some suggestions would be:

* all property values are converted to Lua-strings unless:

* values are quoted with ", in which case the string in between the quotes will be used for the prop value as a Lua-string (so first level quotes are removed by default and value-type is Lua-string - enforce backslash escapes if you need a " - if it starts with a " then give it to the json parser/decoder)

* value of true or false will be automatically converted to equivalent Lua booleans (use “true” or “false” if you need the string-values)

* values of numbers, like 42, will be automatically converted to Lua numbers if tonumber(value)~=nil (use “42” if you need the string value).

* value of null or nil or empty string will be converted to Lua-nil (use “” explicitly for a Lua empty string value)

* value is sandwiched between either {} or [], in which case the value will be parsed/interpreted/decoded as a json-value

I don’t think much will break - and the code that breaks may be improved by fixing anyway :wink:

-FS.

[import]uid: 8093 topic_id: 6937 reply_id: 24294[/import]

#3

This all seems like a very good idea, I will start thinking about getting it implemented. Not sure if it will get into 2.9 (as I’m going to try to make a release every friday) so for now I will tentatively aim for 3.0

Thanks again, this should certainly help clear up a lot of things to do with properties. [import]uid: 5833 topic_id: 6937 reply_id: 24392[/import]

#4

As maybe a welcome side-effect, the Lime performance may increase substantially because you do not have to rely on Lua’s auto-conversion for number arithmetic.

Right now most of Lime’s numbers read-in from the Tiled-tmx files seem to be strings, which means that they are auto-converted to numbers before any operation that requires them to be numbers. This feels like an expensive process as it requires string parsing and conversion. On big maps, while you have to iterate over many tiles and calculate with many “numbers”, that may make a big difference in performance.

-FS.
[import]uid: 8093 topic_id: 6937 reply_id: 24457[/import]

#5

Please take a look at the looslyJsonDecode() in the snippet, which essentially does what I described in a previous message: it tries to json-decode any string, but if it fails, i.e. string is not json-encoded, it returns that string itself.

You may be able to use it in transition to a strict adherence to json format.

Anywhere you do a self:setProperty(key, value) or self:setProperty(attributes.name, attributes.value), where the value is from the xml-file, you could substitute that by self:setProperty(key, looslyJsonDecode(value)) and self:setProperty(attributes.name, looslyJsonDecode(attributes.value))… and maybe in a few other places - and then some code will probably break - this is the kind of micro code-surgery you will have to do yourself as it will affect the whole Lime-body :wink:

[lua]— Returns the json decoded value for all proper json encoded string s, but in case of error returns the adds “'s to the original string and return the json decoded result with the error message.
function looslyJsonDecode(s,strict)
if(strict)then
return Json.Decode(s)
else
local r,v,e
r,v = pcall(Json.Decode,s)
if®then
return v
else
print(“WARNING - looslyJsonDecode:”,v)
return Json.Decode(”""…s…"""),v
end
end
end

local t,e
j = ““jaja””; t,e = looslyJsonDecode(j);print(“JSON:”, j, type(t),t,e)
j = “true”; t,e = looslyJsonDecode(j);print(“JSON:”, j, type(t),t,e)
j = “false”; t,e = looslyJsonDecode(j);print(“JSON:”, j, type(t),t,e)
j = “null”; t,e = looslyJsonDecode(j);print(“JSON:”, j, type(t),t,e)
j = “123”; t,e = looslyJsonDecode(j);print(“JSON:”, j, type(t),t,e)
j = “[1,2,3]”; t,e = looslyJsonDecode(j);print(“JSON:”, j, type(t),t,e)
j = “jaja”; t,e = looslyJsonDecode(j);print(“JSON:”, j, type(t),t,e)
j = “ja’ja”; t,e = looslyJsonDecode(j);print(“JSON:”, j, type(t),t,e)
j = “”; t,e = looslyJsonDecode(j);print(“JSON:”, j, type(t),t,e)[/lua] [import]uid: 8093 topic_id: 6937 reply_id: 24463[/import]

#6

This looks great, and would certainly be a good solution to the ambiguities and as you say, a performance boost to boot. I will try to bump this up and get it into 2.9 [import]uid: 5833 topic_id: 6937 reply_id: 24479[/import]

#7

One more suggestions.

I noticed that your splitString() function does not deal with some corner cases…

When you look at “http://lua-users.org/wiki/SplitJoin”, where you probably borrowed your splitString code from, there are a great many implementations listed and it’s confusing to understand which one is the best to choose.

The one I chose is the “Function: Split a string with a pattern, Take Three” implementation. I have include the code at the end.
Running both this "split and the “splitString” thru the unit tests shown on that page, you’ll get:

split vs splitString  
split "a:b", ":"  
["a", "b"]  
splitString "a:b", ":"  
["a", "b"]  
split "abc", ":"  
["abc"]  
splitString "abc", ":"  
["abc"]  
split "", ":"  
[""]  
splitString "", ":"  
[]  
split "a:b:", ":"  
["a", "b", ""]  
splitString "a:b:", ":"  
["a", "b"]  
split "::", ":"  
["", "", ""]  
splitString "::", ":"  
[]  
split "abc", ""  
["abc"]  
splitString "abc", ""  
Runtime error  
 ...franks/MySqwormy/CollectibleItemsIPad/lime-utils.lua:156: malformed pattern (missing ']')

(the results are json encoded for easy pretty-print)

What you can see is that splitString doesn’t seem to handle all unit test cases well…and even crashes for the last case.

You may want to substitute you splitString by that take-3 implementation - they are plug-compatible (you can ignore the 3rd parameter for your usage) - it could save you some headaches in the future…

It really starts to annoy me badly that the Lua gatekeepers do not put any efforts to maintain and ship “their” Lua with a well-tested, high-performance standard library of functions that would cater for the basic programming needs - no batteries included with Lua. It is such a waste of time having to look for solid implementations of functions that ship standard with any other language (java, python, ruby, C/C++, …). It shows poor leadership to leave the Lua user community hanging like that…

Sorry about that last part… but I believe we can all spend our time much better than worrying about basic use cases for a simple split string function :frowning:

-Frank.

[lua]— Split a string in parts given a delimiter
– from http://lua-users.org/wiki/SplitJoin
– added bad case “” for sep
split = function(str, delim, maxNb)
– Eliminate bad cases…
if(delim == “” or string.find(str, delim) == nil) then
return { str }
end
if maxNb == nil or maxNb < 1 then
maxNb = 0 – No limit
end
local result = {}
local pat = “(.-)” … delim … “()”
local nb = 0
local lastPos
for part, pos in string.gfind(str, pat) do
nb = nb + 1
result[nb] = part
lastPos = pos
if nb == maxNb then break end
end
– Handle the last field
if nb ~= maxNb then
result[nb + 1] = string.sub(str, lastPos)
end
return result
end[/lua]
[import]uid: 8093 topic_id: 6937 reply_id: 24599[/import]

#8

Damn Frank! I tuned in to this thread and got a wealth of information out of it from you.

+1 to JSON!!

And thanks for the code snippets too!! [import]uid: 11636 topic_id: 6937 reply_id: 24809[/import]

#9

Thanks, glad you appreciate the suggestions and snippets.

The great thing about this Lime toolkit is that Graham Ranson keeps it all open source AND that he is exceptionally responsive, which allows us all to help him to make it a fantastic tool for Corona game development - note that this kind of a toolkit is very complex and requires serious focus to get it right. And then the price is right too - to pay 20-40 bucks for that much functionality and for a maintainer who is so dedicated… priceless :wink:

–

Unfortunately, I felt like I had to make one more change to the split function that I mentioned.

The reason was that although it didn’t choke on split(“abcd”, “”), like the other splitString() function, it didn’t give the “correct” result either, which should be [“a”,“b”,“c”,“d”] and an easy way to convert a string into an array of chars (it was mentioned on that webpage also as a desirable feature…).

Anyway, the attached seems to do it all and is a welcome addition to my little toolbox.

Regards, Frank.
[lua]— Split a string str in maximum maxNb parts given a delimiter delim.
– from http://lua-users.org/wiki/SplitJoin -
– added case “” for delim to split the str in array of chars
–@param str - string to split
–@param delim - string - delimiter
–@param maxNb - integer - maximum number of split parts (optional).
–@return - table-array of string-parts that were found during the split
function split(str, delim, maxNb)
local result = {}
if maxNb == nil or maxNb < 1 then
maxNb = 0 – No limit
end
if(delim == “”)then
local nb = 0
for c in str:gmatch"." do
nb = nb+1
result[nb] = c
if(nb==maxNb)then return result end
end
return result
end
if(string.find(str, delim) == nil) then
– eliminate bad case
return { str }
end
local pat = “(.-)” … delim … “()”
local nb = 0
local lastPos
for part, pos in string.gfind(str, pat) do
nb = nb + 1
result[nb] = part
lastPos = pos
if nb == maxNb then break end
end
– Handle the last field
if nb ~= maxNb then
result[nb + 1] = string.sub(str, lastPos)
end
return result
end[/lua]

[import]uid: 8093 topic_id: 6937 reply_id: 24825[/import]

#10

Thanks for the kind words Frank, I really hope that we can get it as awesome as possible and with help like this I’m sure we can.

I will update the split function with your changes right away! [import]uid: 5833 topic_id: 6937 reply_id: 24895[/import]

#11

Frank thanks much for all your contributions and for keeping Graham on his toes. I see he finally has found his match! :slight_smile: This should keep him out of mischief and focused on getting Lime out the door! Thanks a bunch to both of you!!! [import]uid: 11904 topic_id: 6937 reply_id: 24926[/import]

#12

Who has time for mischief when you’re always working? :slight_smile: [import]uid: 5833 topic_id: 6937 reply_id: 25217[/import]

#13

A quick update to this, I wanted to get the change into 2.9 but thought that as it could end up breaking things it would probably not put it in the morning of the release so instead I waited and have put it in now. I am happy to report that after the arguably short testing everything seems to all be working fine and now I have a week for testing until the next release so by then any issues that do come up should be sorted. [import]uid: 5833 topic_id: 6937 reply_id: 25220[/import]