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

Deserialization: Restore backwards compat #12519

Merged
merged 3 commits into from
Jul 14, 2022

Conversation

appgurueu
Copy link
Contributor

@appgurueu appgurueu commented Jul 8, 2022

#11427 deliberately stopped not throwing an error when a non-string is passed to minetest.deserialize. Mods however seem to rely on this (unfortunately it's hard to [sem]grep for this since this depends heavily on program logic). The first case of this causing a crash was with petz, where it lead to an issue in petz code being discovered & fixed, which prompted me to write this patch. The second case was with technic_plus, a rather well-written mod which relied on passing nil returning nil, which prompted me to open this PR. Both crashes have since been fixed, but widespread adoption of 5.6.0 might cause more crashes to pop up.

This PR now changes the hard error into:

  1. A deprecation warning if nil is passed, and returns nil plus an error message as before
  2. A more useful error if any other type is passed.

builtin/common/serialize.lua Outdated Show resolved Hide resolved
builtin/common/serialize.lua Outdated Show resolved Hide resolved
@sfan5 sfan5 added @ Script API Bugfix 🐛 PRs that fix a bug labels Jul 8, 2022
@appgurueu appgurueu force-pushed the restore-serialize-compat branch 3 times, most recently from 4b21232 to 4c3354d Compare July 8, 2022 12:05
builtin/common/serialize.lua Outdated Show resolved Hide resolved
Copy link
Member

@sfan5 sfan5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

Copy link
Member

@SmallJoker SmallJoker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Acceptable.

@sfan5 sfan5 added this to the 5.6.0 milestone Jul 14, 2022
@SmallJoker SmallJoker merged commit ac4eb74 into minetest:master Jul 14, 2022
@appgurueu appgurueu deleted the restore-serialize-compat branch March 31, 2024 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants