Skip to content

Check harvester protection for each node it attempts to harvest#59

Merged
joe7575 merged 2 commits intojoe7575:masterfrom
oversword:bls-186
Dec 15, 2020
Merged

Check harvester protection for each node it attempts to harvest#59
joe7575 merged 2 commits intojoe7575:masterfrom
oversword:bls-186

Conversation

@oversword
Copy link
Contributor

Fixes BLS issue 186 BlockySurvival/issue-tracker#186

Performs protection check for each node it attempts to harvest, and exits as soon as it sees a protected node.

Considering there is a protection check outside of the for loop, I have a feeling this wasn't done for efficiency reasons. It would be nice if we could ask if a volume is protected, not just a node, but currently this code is required for security reasons.

One odd feature of this change is that nodes above a protected area will be harvested, but nodes below that protected area will not. I don't think that's such an issue, but we could:

  • Loop through the nodes checking for any protection before removing anything, skipping the entire thing if anything is protected
  • Skip this iteration of harvesting instead of exiting the function, so that harvest would resume below the protected area

@joe7575
Copy link
Owner

joe7575 commented Dec 14, 2020

But you could do the check a few lines later:

	for y_pos = start_y_pos,stop_y_pos,-1 do
		pos.y = y_pos
		local node = minetest.get_node_or_nil(pos)
		if node and node.name ~= "air" then
			local order = tubelib_addons1.FarmingNodes[node.name] or tubelib_addons1.Flowers[node.name]
			if order and not minetest.is_protected(pos, this.owner) then
                                ....

This would be not that time consuming and would allow to harvest nodes below that protected area.

@oversword
Copy link
Contributor Author

Thanks @joe7575 I'll give that a go in a minute

@oversword
Copy link
Contributor Author

oversword commented Dec 14, 2020

@joe7575 "would allow to harvest nodes below that protected area" I don't think that's true actually, the else on that condition is

else 	
	return true	-- hit the ground
end

However, I think if I put this on the next line it would skip, like:

	for y_pos = start_y_pos,stop_y_pos,-1 do
		pos.y = y_pos
		local node = minetest.get_node_or_nil(pos)
		if node and node.name ~= "air" then
			local order = tubelib_addons1.FarmingNodes[node.name] or tubelib_addons1.Flowers[node.name]
			if order then
				if not minetest.is_protected(pos, this.owner) and not remove_or_replace_node(this, pos, inv, node, order) then
					return false
				end
			else 	
				return true	-- hit the ground
			end

I'll give it a test, but as long as lua works like other languages the condition should exit before running remove_or_replace_node

@oversword
Copy link
Contributor Author

@joe7575 That's working now, take another look

@joe7575 joe7575 merged commit 898ed2e into joe7575:master Dec 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants