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

Fix set_bone_position regressions #14141

Merged
merged 2 commits into from
Dec 21, 2023
Merged

Fix set_bone_position regressions #14141

merged 2 commits into from
Dec 21, 2023

Conversation

appgurueu
Copy link
Contributor

@appgurueu appgurueu commented Dec 20, 2023 •
edited

Without this, omitting the parameters (but not explicitly passing them as nil) causes a crash. This is a regression caused by #12388; readParam treats nil and none the same.

Reported by @grorp as causing a crash when using Mineclone.

Not tested yet, but there is no reason why this shouldn't work.

@sfan5 sfan5 added Trivial The change is a trivial bug fix, documentation or maintenance change, as per the Git Guidelines Bugfix 🐛 PRs that fix a bug @ Script API labels Dec 20, 2023
@appgurueu appgurueu changed the title Fix set_bone_position regression (error on passing none) Fix set_bone_position regressions Dec 20, 2023
@appgurueu
Copy link
Contributor Author

appgurueu commented Dec 20, 2023 •
edited

Piggybacking with another pretty trivial fix, this time for warning spam. Question: Is there any reason why we don't use script_log_unique for all deprecation warnings? When is it useful to repeat a deprecation warning?

grorp
grorp approved these changes Dec 21, 2023
View reviewed changes
Copy link
Member

@grorp grorp left a comment •
edited

Choose a reason for hiding this comment

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

Works, prevents errors and deprecation warning spam.

Thank you for the quick response.

@sfan5 sfan5 merged commit cad8e89 into minetest:master Dec 21, 2023
13 checks passed
@grorp grorp mentioned this pull request Dec 26, 2023
Merged
@appgurueu appgurueu deleted the fix-noneornil branch March 31, 2024 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@grorp grorp grorp approved these changes

Assignees
No one assigned
Labels
Bugfix 🐛 PRs that fix a bug One approval ✅ ◻️ @ Script API Trivial The change is a trivial bug fix, documentation or maintenance change, as per the Git Guidelines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@appgurueu @grorp @sfan5