made screwdriver repairable by an anvil

[imre84]

No description provided.

[fluxionary]

this is unreliable, as the wielded index can change between server steps.

[imre84]

this is unreliable, as the wielded index can change between server steps.

maybe I can save the wielded index into a local variable and use that in the lambda. Would that be acceptable for you?

[fluxionary]

sorry i haven't responded. the inventory itself can change between server steps, you may still be clobbering an item that you don't intend to. what problem are you trying to solve by not calling anvil_rightclick immediately?

[imre84]

sorry i haven't responded. the inventory itself can change between server steps, you may still be clobbering an item that you don't intend to. what problem are you trying to solve by not calling anvil_rightclick immediately?

hello, if I call it directly, immediately the screwdriver doesn't get transferred to the anvil, it remains in my inventory. I suppose wherever this function gets called from might be writing to the player's inventory (undoing me transferring the screwdriver to the anvil). with this hack the screwdriver ends up on the anvil. I checked, and any other item slots (apart from the wielded one) are not affected and can be modified from this routine without core.after

[imre84]

well, to be precise as per my test right now, the screwdriver gets duplicated: if I rightclick without core.after a screwdriver appears on my anvil, while it still remains in my hand (most probably due to wear/durability management. kind of odd, if I returned false, there's no rotation, therefore there's no wear, therefore there's no point in writing to the player inventory (afaik))

[imre84]

maybe we need to change the api and make sure that whenever the on_rotate returns with an ItemStack (and not with true-false-0-1-2-3) the wielded item gets replaced with that ItemStack by the caller? Or maybe make this tunction return with a pair of "whatever used to be returned until now",optional ItemStack. That would be a solution... What do you think? I might be even able to make that PR. I suppose noone tried to return from an on_rotate with an ItemStack, so both of those solutions would be backwards compatible.

[fluxionary]

i wish the screwdriver API was a bit better, but since MTG is feature frozen, there's probably no chance of that. the way i solve this issue in server integration code is to override the screwdriver's on_place behavior, so that if that is called while pointing at an anvil (or e.g. an item frame), the node's on_rightclick callback ends up being invoked instead (an example).

but let's see if we can get this approach to work.

you are right, of course you can't call anvil_rightclick directly inside of the on_rotate callback immediately, because that on_rotate call is handled inside the screwdriver's on_place callback, which returns an itemstack which overrides the player's wielded item.

the after callback is an interesting approach, but the world can change a lot between server steps. the following things can all happen:

• the player selects a different item in their inventory
• the player uses and breaks the screwdriver
• the screwdriver is modified by another mod (e.g. perhaps it is repaired)
• the player's inventory is modified by another mod entirely in an unpredictable way
• the player disconnects or logs out
• the anvil node is removed or replaced somehow

the solution is to move a lot of the logic inside the after callback, to check that the world is still the way we expect it. if it is, then we can try to place the screwdriver on the anvil.

this is what i've come up with:

	on_rotate = function(pos, node, user, mode, new_param2)
		if mode == screwdriver.ROTATE_FACE then
			return
		end

		if not minetest.is_player(user) then
			return
		end

		local player_name = user:get_player_name()
		local wield_list = user:get_wield_list()
		local wield_index = user:get_wield_index()

		minetest.after(0,function()
			local player = minetest.get_player_by_name(player_name)
			if not player then
				return
			end

			local inv = player:get_inventory()
			local wielded = inv:get_stack(wield_list, wield_index)

			if wielded:get_name() ~= "screwdriver:screwdriver" then
				return
			end

			local node_now = minetest.get_node(pos)
			if node_now.name ~= "anvil:anvil" then
				return
			end

			local z = anvil_rightclick(pos, node_now, player, wielded)
			inv:set_stack(wield_list, wield_index, z)
		end)

		return false
	end,

thoughts?

[imre84]

hello I'll revert back to you with my PR updated once I find the time to inspect and try your code. thank you for your patience.

[imre84]

ok I made one minor change, I renamed z to tool_after_rightclicking

[fluxionary]

looks good to me now, though as i'm not part of the "mintest-mods" org, i'm not sure what weight my approval has.

[imre84]

looks good to me now, though as i'm not part of the "mintest-mods" org, i'm not sure what weight my approval has.

ok, thank you for your help, I guess we'll just need to wait for an official approval then :)

[imre84]

technic:sonic_screwdriver is still able to rotate the anvil in weird ways, but given the fact that for some reason on_rotate isn't called when rotating with technic:sonic_screwdriver, and it's not a regression (normal screwdriver was doing the same before this PR) I'm going to live with that and not extend the scope of this PR

[Niklp09]

@imre84 Merge conflict, rebase needed.

[imre84]

@Niklp09 hello, currently I do not play minetest and I have other projects as well. The PR was made against the head of the master branch at the time though. Maybe I'll revert to minetest in the next two years, in that case maybe I'll update the PR.

[imre84]

@imre84 Merge conflict, rebase needed.

Hello, conflict fixed

[Niklp09]

LGTM, will merge in a few days unless there are objections

[imre84]

should I have received an email when this was merged?