Skip to content
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.

Sign up for GitHub

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

Jump to bottom

wesnoth-s print is incompatible with normal lua print #2236

Closed
vgaming opened this issue Nov 23, 2017 · 6 comments
Closed

wesnoth-s print is incompatible with normal lua print #2236

vgaming opened this issue Nov 23, 2017 · 6 comments
Assignees
@GregoryLundberg
Labels
Bug Issues involving unexpected behavior. Lua API Issues with the Lua engine and API.

Comments

@vgaming
Copy link
Member

vgaming commented Nov 23, 2017 •
edited

Steps to reproduce:

  • execute this in wesnoth: print("test", true)
  • execute this in "OS" lua: print("test", true)

Expected:
similar or identical output

Actually:

  • First line prints test
  • Second line prints test true
@vgaming
Copy link
Member Author

vgaming commented Nov 23, 2017

On IRC chat, JyrkiVesterinen pointed this to be the possible buggy implementation of print: https://github.com/wesnoth/wesnoth/blob/master/src/scripting/lua_kernel_base.cpp#L94-L119

@GregoryLundberg
Copy link
Contributor

Not buggy but it can be improved.

Wesnoth depends upon loa_tostring obtaining a string value.

The Lua version calls into the Lua runtime to tostring(..) to do the conversion, then uses lua_tostring to obtain the result.

Give me a few hours ...

@GregoryLundberg GregoryLundberg self-assigned this Nov 23, 2017
@GregoryLundberg GregoryLundberg added Bug Issues involving unexpected behavior. Lua API Issues with the Lua engine and API. labels Nov 23, 2017
@kallaballa
Copy link
Contributor

oh.. i didn't see that you already assigned the issue to you. in the meanwhile i fixed the bug: kallaballa@49d1055

Should i create a PR?

GregoryLundberg added a commit that referenced this issue Nov 23, 2017
@GregoryLundberg
Fix wesnoth.print
0eb25f4 
Pass each parameter through Lua tostring() to convert to strings. This also allows meta-method tostring on userdata.

This aligns the operation of wesnoth print() to be closer to what a Lua programmer would expect from the normal Lua print() which is not available.

Closes Issue #2236
@GregoryLundberg
Copy link
Contributor

If you compare, I think you'll see you missed a lot of cases.

@kallaballa
Copy link
Contributor

You are right, though i see only one case: user data

@GregoryLundberg
Copy link
Contributor

boolean, table, userdata, function, thread ...

GregoryLundberg added a commit that referenced this issue Nov 30, 2017
@GregoryLundberg
Fix wesnoth.print
a37177f 
Pass each parameter through Lua tostring() to convert to strings. This also allows meta-method tostring on userdata.

This aligns the operation of wesnoth print() to be closer to what a Lua programmer would expect from the normal Lua print() which is not available.

Closes Issue #2236
GregoryLundberg added a commit that referenced this issue Nov 30, 2017
@GregoryLundberg
Fix wesnoth.print
2337ef9 
Pass each parameter through Lua tostring() to convert to strings. This also allows meta-method tostring on userdata.

This aligns the operation of wesnoth print() to be closer to what a Lua programmer would expect from the normal Lua print() which is not available.

Closes Issue #2236
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@GregoryLundberg GregoryLundberg

Labels
Bug Issues involving unexpected behavior. Lua API Issues with the Lua engine and API.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@kallaballa @GregoryLundberg @vgaming