Skip to content

compatibility with lua 5.1 #10

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

Closed
wants to merge 1 commit into from
Closed

compatibility with lua 5.1 #10

wants to merge 1 commit into from

Conversation

spiiin
Copy link

@spiiin spiiin commented Apr 22, 2018

I used your library for scripting in fceux emulator, which use lua 5.1 and found little bug with bool serialization. Hope this fix helps others.

…s tostring(val)..tostring(stack), but in lua>5.1 it returns just tostring(val).
@rxi
Copy link
Owner

rxi commented Apr 22, 2018

Assuming I understand what you meant correctly, lua 5.1.5 doesn't seem to exhibit this problem — would you be able to provide an example that demonstrates the problem, and the exact version number of lua you are using?

In regard to the commit message: you should try to keep the first line as a very brief summary, followed by an empty line and an optional detailed description.

@spiiin
Copy link
Author

spiiin commented Apr 22, 2018

print(_VERSION)
json = require("json")
local a = json.encode({"test", true})
print(a)

-- Lua 5.1
-- ["test",true {[{'test', true}]=true}]

https://github.com/TASVideos/fceux/tree/master/src/lua

@rxi
Copy link
Owner

rxi commented Apr 22, 2018

Is there any way you can find out the x.x.x version of lua? I want to determine whether this is behaviour is exhibited in a normal, unmodified version of lua, and if it is, which versions are effected. If you're getting this behaviour from the standalone lua interpreter you can run lua -v to get the x.x.x version.

Would you be able to tell me what the following prints:

print(tostring({123}))

Thanks

@spiiin
Copy link
Author

spiiin commented Apr 22, 2018

I beleive, it's not modified Lua, I provide you link to sources, you can check yourself. I haven't standalone interpreter for this version.

print(tostring({123}))
--{123}

But read your code, in your case you call:

print(tostring(true, {123}))
--true {123}

difference here.

@rxi
Copy link
Owner

rxi commented Apr 22, 2018

The result you pasted is not standard tostring() behaviour — if you look at the source in the repo you linked you can see the tostring() function only handles one argument and does not print tables in the form from your example.

The tostring() function you are calling seems to have been changed and is not the standard lua version. For comparison the lua 5.1.5 interpreter gives the following:

Lua 5.1.5  Copyright (C) 1994-2012 Lua.org, PUC-Rio
> print(tostring({123}))
table: 0x9fca568
> print(tostring(true, {123}))
true
> 

@spiiin
Copy link
Author

spiiin commented Apr 22, 2018

I contacted with developers of fceux, they used 5.1.4 version.
I checked standalone binary of lua 5.1.4 , it print your result.

But fceux built from that sources with visual studio 2010 and 2015 printed my version, I don't understand why.

@rxi
Copy link
Owner

rxi commented Apr 22, 2018

If you're using other 3rd-party modules they may be overriding the tostring function. If you want to find the cause I'd suggest searching for all instances of tostring in the code base.

@rxi rxi closed this Apr 22, 2018
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.

2 participants