HudKit - refer to HUD elements by strings.

Post Reply
User avatar
rubenwardy
Moderator
Posts: 6381
Joined: Tue Jun 12, 2012 18:11
GitHub: rubenwardy
IRC: rubenwardy
In-game: rubenwardy
Location: United Kingdom
Contact:

HudKit - refer to HUD elements by strings.

by rubenwardy » Post

Code: Select all

local myhudkit = hudkit()
function example(player)
	local some_data = "abcd"
	if not myhudkit:exists(player, "modname:hud_1") then
		myhudkit:add(player, "modname:hud_1", {
			hud_elem_type = "text",
			position = {x = 1, y = 0},
			scale = {x = 100, y = 100},
			text = some_data,
			offset = {x=-100, y = 20}
		})
	else
		myhudkit:change(player, "modname:hud_1", "text", some_data)
	end
end

minetest.register_on_leaveplayer(function(player)
	myhudkit.players[player:get_player_name()] = nil
end)
I created this for capturetheflag, HUDs returning IDs annoys me, I much prefer it like this.

It's more of an experiment, what do you think?
<rubenwardy> Why do HUD elements return IDs, rather than assigning a string?
<rubenwardy> like formspecs
<rubenwardy> as in, why don't you give it a name when creating it, rather than having to remember an ID.
<rubenwardy> You could use serials: num_of_hud++; local name = "mymod:hud_" .. num_of_hud
<rubenwardy> if you don't want to name it.
<rubenwardy> To maintain support, make HUD functions return the string/name, and allow mod devs to specify a string/name (or make one if they don't).
Code Snippet
Last edited by rubenwardy on Thu Apr 02, 2015 13:51, edited 1 time in total.

4aiman
Member
Posts: 1208
Joined: Mon Jul 30, 2012 05:47

Re: HudKit - refer to HUD elements by strings.

by 4aiman » Post

Strings are much slower when it comes to comparison, aren't they?
AFAIU, "logoffs" handling and changing HUDs for existing players will require such comparisons.
And you're already using "if not myhudkit:exists(player, "modname:hud_1")" in your code.
Add a necessity to check/change HUDs on every server step and you'll slow down a whole game a lot.

It's not like I'm against, but IDs are good enough to handle. You've managed to create this snippet after all.

Besides, I think that much work should be done to "stabilize" HUDs feature than making more ways to manage those.

But, once again, I believe, that's not on top priority issues list.

Regards!

User avatar
rubenwardy
Moderator
Posts: 6381
Joined: Tue Jun 12, 2012 18:11
GitHub: rubenwardy
IRC: rubenwardy
In-game: rubenwardy
Location: United Kingdom
Contact:

Re: HudKit - refer to HUD elements by strings.

by rubenwardy » Post

The slowness will not be visible, most of Minetest's server lag comes from EnvLock and the switching between C++ and Lua. You need to optimize in the right places. In Lua, formspecs are referred to by strings, nodes are referred to by strings, items are referred to by strings. The HUD and Formspec system will be rewritten, and I think it should be given a high priority.

Yes, I managed to make this snippet, but this snippet makes it a lot easier to manage HUD elements without having to worry about making a container framework to hold IDs each time. You'll probably do something like this if you want to hold multiple items:

Code: Select all

local player_huds = {
   singleplayer = {
        hud1 = 4789
   }
}
Which is what this snippet does for you.

Also, updating the HUD on each server tick is a bad idea due to the increase in packets to the client it will cause.

4aiman
Member
Posts: 1208
Joined: Mon Jul 30, 2012 05:47

Re: HudKit - refer to HUD elements by strings.

by 4aiman » Post

Sure, you own a server with 50 to 100 players online.

Nah, even in singleplayer I can feel the difference.

It MAY be a bad idea to update huds on_step, but still, there are cases where it's necessary.
Huds update can't be delayed like armor, where armour_groups are set, but visual representation comes a second later.
Think about a hud that shows player's status: poison, lightweight, hunger, lifebar etc.
ANY delay might cost you misleading player into being sure of some inexistent state and wasting items to cure it.
Or, a player with 1-2 HP may not be able to notice he's been poisoned in time w/o immediate HUD notification.

When it comes down to improving usability, there shouldn't be such a thing as "you should not".
Think of apple. iPhone 6 is worse than some phones released 2 years ago (in terms of HW). But iOS it's SO nice to look at and SO smooth in usage, that once you see one - you'll never be able to say, that your super-pooper android looks better.
I haven't got an iPhone 6, but I have plenty of experience with iPad2.

Once again, I'm not against your proposal, but IDs ARE logical for huds, 'cause every second I know how much there are. With strings I would be forced to count those by myself (#player_huds won't work, cause indexes are not ordinal).

User avatar
rubenwardy
Moderator
Posts: 6381
Joined: Tue Jun 12, 2012 18:11
GitHub: rubenwardy
IRC: rubenwardy
In-game: rubenwardy
Location: United Kingdom
Contact:

Re: HudKit - refer to HUD elements by strings.

by rubenwardy » Post

This snippet doesn't use string comparison, it uses hashed indices. Please show real statistics, if Lua is slow at this then there is a real problem with Lua. I can't see how it can be slower than mapgen, EnvLock or the c++/Lua switch.

Why do you need to count them?
Last edited by rubenwardy on Fri Apr 03, 2015 12:42, edited 1 time in total.

4aiman
Member
Posts: 1208
Joined: Mon Jul 30, 2012 05:47

Re: HudKit - refer to HUD elements by strings.

by 4aiman » Post

Ok, I was wrong about strings, my bad. Should've remembered about a certain pdf I'd read.

But that doesn't change things a lot.
Generally, your code adds 2 new local variables, one if clause and indexes tables 4 times.
All that instead of a simple player:hud_X() call. Doesn't seem like a speed-up to me.

Too bad, neither I have time to prepare statistics nor I'm to stubborn to prepare that nevertheless.
"Statistics" is at least a set of 10000 tests. Otherwise that's not statistics.
Bear in mind that it's not the case where we can simply use a for clause.
Statistics should be true to life with all those other mods enabled -- It may well be that this code will affect some other mod which won't be able to do its stuff in time.

Besides, you can't say smth like "it's okay - no one will ever notice anyway" stuff.
Maybe MineTest's envlock IS so bulky (IDK really) that all that code is nothing in comparison to it, but MT isn't the only engine out there. FM has broke down one huge lock into tiny pieces. Why are you so sure that MT won't do it too to become more lag-free?

Don't get me wrong, I'm writing a lot of slow code. But that's only because I don't know how to speed it up.
But in this situation you know that you're adding more code than necessary to handle HUDs.
THAT is what I can't understand and why bothering you.

User avatar
rubenwardy
Moderator
Posts: 6381
Joined: Tue Jun 12, 2012 18:11
GitHub: rubenwardy
IRC: rubenwardy
In-game: rubenwardy
Location: United Kingdom
Contact:

Re: HudKit - refer to HUD elements by strings.

by rubenwardy » Post

This snippet makes it easier to manage HUDs, it's not meant to increase speeds. As programmers, we want easy readable solutions - providing they aren't inoptimal. If we always went for the fastest solution, we'd use assembly.

I have improved the snippet to remove unnecessary checks and statements. I will provide some bench marks, as this is interesting I guess.

User avatar
rubenwardy
Moderator
Posts: 6381
Joined: Tue Jun 12, 2012 18:11
GitHub: rubenwardy
IRC: rubenwardy
In-game: rubenwardy
Location: United Kingdom
Contact:

Re: HudKit - refer to HUD elements by strings.

by rubenwardy » Post

With no overhead from Minetest

I ran hudkit.exists and hudkit.change 1,000,000,000 times - a billion times - in luajit, and it took just 0.39 seconds. This was with Luajit on the command line and with hud_change calls removed. (so we're only testing the things around it).
This gives each exists and change pair of calls a time of 0.00000000039 seconds each, on average. (3.9*10^-10 s)

With 10,000,000 calls, for comparison with below, it took 0.01 seconds. (the finest os.clock can measure is 0.01, so this could be anytime above 5 millisecs, I guess.)

With overhead from Minetest

I ran hudkit.exists and hudkit.change 10,000,000 times - ten million times - in Minetest's lua environment, and it took 4.72 seconds. This was with hud_change calls removed. (so we're only testing the things around it).
This gives each exists and change pair of calls a time of 0.000000472 seconds. (4.72*10^-7 s)

10,000,000: 4.72s
01,000,000: 0.39s
00,100,000: 0.01s (the finest os.clock can measure is 0.01)

Summary

I very much doubt that you'll have more than 1000 checks of these per second, and that would be a fraction of a millisecond.

Code: Select all

-- HudKit, by rubenwardy
-- License: Either WTFPL or CC0, you can choose.

local function hudkit()
	return {
		players = {},

		add = function(self, player, id, def)
			local name = player:get_player_name()
			local elements = self.players[name]

			if not elements then
				self.players[name] = {}
				elements = self.players[name]
			end

			elements[id] = player:hud_add(def)
		end,

		exists = function(self, player, id)
			local elements = self.players[player:get_player_name()]
			return elements and elements[id]
		end,

		change = function(self, player, id, stat, value)
			local elements = self.players[player:get_player_name()]
			if not elements or not elements[id] then
				return false
			end

			return true
		end,

		remove = function(self, player, id)
			local elements = self.players[player:get_player_name()]
			if not elements or not elements[id] then
				return false
			end

			elements[id] = nil
			return true
		end
	}
end

local myhudkit = hudkit()

player = {
	get_player_name = function(self)
		return "player1"
	end,

	hud_add = function(self, def)
		return 1
	end,

	hud_change = function(self, one, two, three)

	end,

	hud_remove = function(self, id)

	end
}

-- Please note, passing a nil player value will cause HudKit
--  to crash (it does not check for that).
local x = os.clock()
myhudkit:add(player, "modname:hud_1", {
	hud_elem_type = "text",
	position = {x = 1, y = 0},
	scale = {x = 100, y = 100},
	text = some_data,
	offset = {x=-100, y = 20}
})

local some_data = "abcd"
for i=1, 10000000 do
	if myhudkit:exists(player, "modname:hud_1") then
		myhudkit:change(player, "modname:hud_1", "text", some_data)
	else
		print("error")
	end
end

print(string.format("elapsed time: %.2f\n", os.clock() - x))
4aiman wrote:Bear in mind that it's not the case where we can simply use a for clause.
Statistics should be true to life with all those other mods enabled -- It may well be that this code will affect some other mod which won't be able to do its stuff in time.
That's bollocks. Sorry, but it is.

User avatar
prestidigitator
Member
Posts: 640
Joined: Thu Feb 21, 2013 23:54

Re: HudKit - refer to HUD elements by strings.

by prestidigitator » Post

The only place you're likely to have to worry about performance in this case is where HUDs have to be updated very frequently. If you're going to do some serious benchmarking, I'd suggest also throwing a closure-based solution into the mix. Something like:

Code: Select all

function hud_update_function_factory(player, hud_id)
   return function(...) player:hud_change(hud_id, ...); end;
end
IMO small performance optimizations like this aren't likely to be worth the time and effort, but it's a decent candidate that avoids some extra table look-ups in the fast path (replacing them with upvalue dereferences) if someone's going to go the distance.

EDIT: FYI, composed before seeing rubenwardy's last post reporting some performance data.

4aiman
Member
Posts: 1208
Joined: Mon Jul 30, 2012 05:47

Re: HudKit - refer to HUD elements by strings.

by 4aiman » Post

Ok, statistics has shown it.
I officially agree that this upgraded variant won't kill performance even a bit on a... what HW precisely?
Good if that was some old netbook. I'm not being sarcastic, this is interesting indeed due to the reasons I gave earlier.

I really care to hear about HW, OS, MT version and, of course, the subgame this was tested with.



But
That's bollocks. Sorry, but it is.
If it was, you wouldn't have spent x11+ time (way over 4 seconds instead of 0.39) for x100 less tests ;)

You may say that was the other stuff that had eaten so much time, but it all adds up.
I'm almost sure that any other modder thinks that his/her mod won't take so much time as those of others.

For example, MOBF runs just fine when it's alone. But as soon as more mods were added - it starts to lag intensively. I can't rely on it's mechanisms. Especially in a multi-threaded client.
So it's not only about the speed but reliability.

Your upgraded snippet IS reliable enough.
(It's totally not like you had to have my approval anyway).



The idea with testing inside a closure closure is a good one too.
(Please, don't "fyi" me. The acronyms I use are not addressed to anyone -e.g. there are no "you" or "your". There's nothing wrong with "fyi", but I simply dislike that one.)


But overall I'm impressed with the results. Those are much better than I thought.
It would be great to test the initial version to compare with the current one.
I've always been told to avoid indexing of "for" clauses where it's possible.
How much would those add?

Regards!

Post Reply

Who is online

Users browsing this forum: No registered users and 2 guests