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

Don't check protection of air when placing bones #2964

Merged
merged 1 commit into from Jul 6, 2023

Conversation

OgelGames
Copy link
Contributor

It doesn't make sense to check the protection of air nodes, and it makes bone placement fail more often.

@rollerozxa
Copy link
Member

Won't this lead people to being able to effectively grief areas which are protected? Filling up a protected spawn with bones by repeatedly killing yourself, or the like.

@OgelGames
Copy link
Contributor Author

OgelGames commented Jun 23, 2022

Won't this lead people to being able to effectively grief areas which are protected? Filling up a protected spawn with bones by repeatedly killing yourself, or the like.

That is already possible, this just changes the secondary check to match the logic in may_replace() (which is only called for the initial position):

-- allow replacing air
if node_name == "air" then
return true
end
-- don't replace nodes inside protections
if minetest.is_protected(pos, player:get_player_name()) then
return false
end

@appgurueu
Copy link
Contributor

appgurueu commented Jun 23, 2022

That is already possible, this just changes the secondary check to match the logic in may_replace() (which is only called for the initial position)

Why don't you switch the "find nearby pos" to may_replace() entirely? Ideally there should IMO be a hierarchy:

  1. Try placing bones where the player died;
  2. Try nearby positions, checking may_replace() - prefer air over buildable nodes, unprotected over protected etc.

That said, this change is reasonable and at least partially fixes the issue.

@OgelGames
Copy link
Contributor Author

OgelGames commented Jun 23, 2022

Why don't you switch the "find nearby pos" to may_replace() entirely?

Because that would be more complicated and I just wanted to fix this small mistake without changing too much. I do agree that checking every position with may_replace() would be more thorough.

Copy link
Contributor

@appgurueu appgurueu left a comment

Choose a reason for hiding this comment

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

Probably fixes 99% of cases frequently encountered on servers

@sfan5 sfan5 added this to the 5.8.0 milestone Apr 8, 2023
@sfan5 sfan5 merged commit 110c235 into minetest:master Jul 6, 2023
@OgelGames OgelGames deleted the patch-1 branch July 6, 2023 10:56
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

4 participants