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.

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

6ca214fefc9e1eb707f006631d39c1e4dbafe1b5 changes handling of nil in MetaDataRef:set_string (breaking e.g. Digtron) #14391

Closed
j-r opened this issue Feb 20, 2024 · 1 comment · Fixed by #14396
Labels
Bug Issues that were confirmed to be a bug Regression Something that used to work no longer does. @ Script API

Comments

@j-r
Copy link

j-r commented Feb 20, 2024

Minetest version

6ca214f

Operating system and version

Debian Oldstable (no upgrade path available)

Summary

The commit effectively replaces tolstring with checklstring.

The documentation doesn't say anything about nil. If I understand the code correctly nil is handled as an empty string which is documented to have the effect of removing an entry. So nil isn't really needed for anything, but since assigning nil is the LUA way of removing a mapping, it probably should be kept functional.

Steps to reproduce

Get a MetaDataRef in LUA and call set_string("something", nil) on it

@j-r j-r added the Unconfirmed bug Bug report that has not been confirmed to exist/be reproducible label Feb 20, 2024
@j-r
Copy link
Author

j-r commented Feb 20, 2024

Forgot to add: partially reverting only the changes to src/script/lua_api/l_metadata.cpp fixes the issue.

AFAICS there are no other instances of replacing tolstring.

@sfan5 sfan5 added Bug Issues that were confirmed to be a bug Regression Something that used to work no longer does. and removed Unconfirmed bug Bug report that has not been confirmed to exist/be reproducible labels Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issues that were confirmed to be a bug Regression Something that used to work no longer does. @ Script API
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants