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

Fix float argument check in minetest.set_timeofday() #10483

Merged
merged 2 commits into from
Oct 13, 2020

Conversation

Zughy
Copy link
Member

@Zughy Zughy commented Oct 12, 2020

  • Goal of the PR
    Avoid an engine assumption crash
  • How does the PR work?
    I've changed the check, now it doesn't make the whole client crash anymore (only the game)
  • Does it resolve any reported issue?
    closes Many Lua C functions are missing type checks #1794 (I've tried all the listed functions, they crash as intended except set_timeofday)

I guess there are other functions that might not work as intended with weird values, but testing them all is a suicide. I suggest to solve them on the go, avoiding keeping an issue open - especially because the listed ones are all solved

To do

This PR is Ready for Review.

How to test

Make it crash with

minetest.register_on_joinplayer(function(player, last_login)
    minetest.set_timeofday(-1)
end)

@HybridDog
Copy link
Contributor

HybridDog commented Oct 12, 2020

I've tested it with the /time nan chatcommand; a hard crash no longer happens.

@paramat paramat assigned paramat and unassigned paramat Oct 12, 2020
@paramat paramat added the Bugfix 🐛 PRs that fix a bug label Oct 12, 2020
@sfan5 sfan5 changed the title Working float check to minetest.set_timeofday() Fix float argument check in minetest.set_timeofday() Oct 12, 2020
@SmallJoker SmallJoker added One approval ✅ ◻️ Trivial The change is a trivial bug fix, documentation or maintenance change, as per the Git Guidelines labels Oct 12, 2020
@paramat
Copy link
Contributor

paramat commented Oct 12, 2020

I did not test but code looks fine 👍

@rubenwardy rubenwardy merged commit 7499ebe into minetest:master Oct 13, 2020
JosiahWI pushed a commit to JosiahWI/minetest that referenced this pull request Oct 17, 2020
HybridDog pushed a commit to HybridDog/minetest that referenced this pull request Dec 17, 2020
@Zughy Zughy deleted the timeofday_crash branch January 2, 2021 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bugfix 🐛 PRs that fix a bug @ Script API Trivial The change is a trivial bug fix, documentation or maintenance change, as per the Git Guidelines >= Two approvals ✅ ✅
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Many Lua C functions are missing type checks
6 participants