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

on_wielditem_change Lua function #1377

Open
MirceaKitsune opened this issue Jun 15, 2014 · 11 comments
Open

on_wielditem_change Lua function #1377

MirceaKitsune opened this issue Jun 15, 2014 · 11 comments
Labels
Feature request Issues that request the addition or enhancement of a feature Low priority @ Script API

Comments

@MirceaKitsune
Copy link
Contributor

Today I discovered stu's fantastic 3D-Armor mod. Which doesn't only add an armor system that reduces damage, but also the long awaited features of showing worn armor and the wielded item on the player model, allowing other players to see what you are wearing or holding!

https://forum.minetest.net/viewtopic.php?f=11&t=4654

Although the armor system works well and efficiently, engine limitations make it difficult to find a good way of handling wielded items. The server needs to periodically check when a player changed the selected item on the hotbar, which is laggy and costly.

Please add a Lua function that gets triggered when the item held by a player changes, and reports the new and possibly old item. It should typically execute when the player selects another hotbar slot which contains another item, or if an item was added / removed to / from the selected hotbar slot.

Visible wielded items are difficult to implement in a quick and cheap way without this. Even if it's for 3D-Armor or any other mod that implements them, an engine function to make it possible is essential in my opinion. Here's an example of what I'm thinking:

minetest.on_wielditem_change(player, old_item, new_item)
mod.change_wielditem_appearance(player:get_player_name(), new_item)
end

@pandaro
Copy link
Contributor

pandaro commented Jun 24, 2014

+1 i agree

@paramat paramat added the Feature request Issues that request the addition or enhancement of a feature label Nov 28, 2017
@paramat paramat closed this as completed Apr 8, 2018
@beyondlimits
Copy link
Contributor

beyondlimits commented Jun 30, 2018

Actually doable in Lua (as a hack of course ;))

local last_wielded = {}
local callbacks = {}

local function register_on_wielditem_change(callback)
	-- callback(player, current, previous)
	assert(type(callback) == 'function')
	table.insert(callbacks, callback)
end

minetest.register_globalstep(function()
	for player, previous in pairs(last_wielded) do
		local current = minetest.get_player_by_name(player):get_wielded_item():get_name()
		if previous ~= current then
			for k, v in pairs(callbacks) do
				v(player, current, previous)
			end
			last_wielded[player] = current
		end
	end
end)

minetest.register_on_joinplayer(function(player)
	last_wielded[player:get_player_name()] = player:get_wielded_item():get_name()
end)

minetest.register_on_leaveplayer(function(player)
	last_wielded[player:get_player_name()] = nil
end)

--------------------

register_on_wielditem_change(function(player, current, previous)
	print(player  .. "'s wielditem has changed.")
end)

EDIT: By the way, 2014 and nothing? Maybe it has been done already.

@ClobberXD
Copy link
Contributor

ClobberXD commented Jul 27, 2018

I've started working on this. At present, I've found the following piece of code that is responsible for handling wielditem change events (i.e. scroll wheel and number keys) client-side:

https://github.com/minetest/minetest/blob/d7d451c647f1fff7983178a9817888f4d0bab695/src/game.cpp#L1960-L2001

Here are my initial propositions:

  • Create and initialize a variable that holds the current wielditem pointer at the top of the function like so
    u16 *old_playeritem = new_playeritem.
  • Call Client::selectPlayerItem(*new_playeritem) at the end of the function (after L2000 would be ideal, I assume) if
    new_playeritem != old_playeritem.
  • The Server::handleCommand_PlayerItem() method that handles the TOSERVER_PLAYERITEM packet calls PlayerSAO::setWieldIndex(u16).
  • Finally, the on_wielditem_change callbacks can be run in setWieldIndex().

EDIT: PlayerSAO::setWieldIndex() is also called in the Server::handleCommand_Interact() method which handles the TOSERVER_INTERACT packet. I don't think it makes sense to run wielditem change callbacks everytime a player interacts. Maybe run the callbacks in Server::handleCommand_PlayerItem() itself?

@paramat
Copy link
Contributor

paramat commented Nov 20, 2018

Requested again so reopening, note this had no core dev support for 4 years.

@paramat paramat reopened this Nov 20, 2018
@ClobberXD
Copy link
Contributor

Note that there's already a PR for this: #7587 :)

@paramat
Copy link
Contributor

paramat commented Nov 29, 2018

So, any core dev support? (i'm neutral).

@ClobberXD
Copy link
Contributor

@paramat I'm starting to think that core devs might not even see this question, at times, in the midst of a hundred other notifications. Asking this question on IRC could help with it getting noticed.

That being said, having one such callback would be very useful for numerous mods. Please take a look at #7587 (comment) for a far-from-complete list of use-cases for this callback.

@ghost
Copy link

ghost commented Nov 29, 2018

Yeah, I'm used to using minetest.after or register_globalstep with player:get_wielded_item():get_name(); and I bet this would be a lot easier/cleaner.

And I agree, perhaps mentions (@ symbol, can you do a group mention?) -- and of course IRC, which one should idle in whenever possible, in case of highlights.

@paramat
Copy link
Contributor

paramat commented Nov 30, 2018

Messages on IRC are often missed by core devs, and often come at a time when they are busy and they then forget. IRC messages don't persist and history is not easily viewable.
Posting in GitHub threads is the proper and reliable way to notify everyone. I also add bumped issues to the 'core dev consideration needed' label and regularly ask on IRC that core devs go through the labeled issues.
Bumped issues get seen by core devs because they are bumped.
The actual reason core devs aren't very responsive is because they are very busy and currently rather absent.
Using @ notifications for all core devs seems overkill, many core devs are already fairly unresponsive to notifications already.

In addition, if anyone wants to draw attention to an issue they can mention it on IRC.

@ghost
Copy link

ghost commented Nov 30, 2018 via email

@paramat
Copy link
Contributor

paramat commented Dec 27, 2018

Because there is a PR i'll leave this open for now. the PR may get core dev support (EDIT: it now has one approval), in which case the request is also supported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature request Issues that request the addition or enhancement of a feature Low priority @ Script API
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants