Mineclonia
/
Mineclonia
Archived
12
17
Fork 13
Code Issues 105 Pull Requests 19 Projects Releases Wiki Activity

The server sends a ridiculous amount of network traffic to burning players #137

New Issue
Closed
opened 2021-08-05 13:27:40 +02:00 by anon5 · 23 comments
anon5 commented 2021-08-05 13:27:40 +02:00
Copy Link
No description provided.
cora added the
confirmed
 label 2021-08-05 15:38:40 +02:00
cora commented 2021-08-05 15:41:43 +02:00
Member
Copy Link

How to reproduce / test:

  1. start minetest --info >log.txt 2>&1
  2. in another terminal do tail -f log.txt |grep TOCLIENT_HUD
  3. enter a world and get yourself burning
  4. observe > 1k TOCLIENT_HUDCHANGE every time you catch fire

this is analogous to the other already fixed hudspam issues #111 and #90

EDIT: adapted packet count to later results, added grep to the tail line to make it less spammy

How to reproduce / test: 1. start `minetest --info >log.txt 2>&1` 2. in another terminal do `tail -f log.txt |grep TOCLIENT_HUD` 3. enter a world and get yourself burning 4. observe > 1k TOCLIENT_HUDCHANGE every time you catch fire this is analogous to the other already fixed hudspam issues #111 and #90 EDIT: adapted packet count to later results, added grep to the tail line to make it less spammy
cora commented 2021-08-05 15:45:27 +02:00
Member
Copy Link

TOCLIENT_HUD_SET_FLAGS might be an issue here as well:

2021-08-05 15:44:08: INFO[Main]: cmd 75 (TOCLIENT_HUDCHANGE) count 2559
2021-08-05 15:44:08: INFO[Main]: cmd 76 (TOCLIENT_HUD_SET_FLAGS) count 441

EDIT: HUD_SET_FLAGS is (was) unrelated - minimap privs ( #146 )

TOCLIENT_HUD_SET_FLAGS might be an issue here as well: ``` 2021-08-05 15:44:08: INFO[Main]: cmd 75 (TOCLIENT_HUDCHANGE) count 2559 2021-08-05 15:44:08: INFO[Main]: cmd 76 (TOCLIENT_HUD_SET_FLAGS) count 441 ``` EDIT: HUD_SET_FLAGS is (was) unrelated - minimap privs ( https://git.minetest.land/Mineclonia/Mineclonia/pulls/146 )
erlehmann commented 2021-08-05 15:50:25 +02:00
Owner
Copy Link

In mods/ENTITIES/mcl_burning/init.lua, there exists a globalstep:

minetest.register_globalstep(function(dtime)
	for _, player in ipairs(minetest.get_connected_players()) do
		mcl_burning.tick(player, dtime)
	end
end)
In `mods/ENTITIES/mcl_burning/init.lua`, there exists a globalstep: ``` minetest.register_globalstep(function(dtime) for _, player in ipairs(minetest.get_connected_players()) do mcl_burning.tick(player, dtime) end end) ```
erlehmann commented 2021-08-05 15:52:12 +02:00
Owner
Copy Link

In mods/ENTITIES/mcl_burning/api.lua:

function mcl_burning.tick(obj, dtime)
	local burn_time = mcl_burning.get(obj, "float", "burn_time") - dtime

	if burn_time <= 0 then
		mcl_burning.extinguish(obj)
	else
		mcl_burning.set(obj, "float", "burn_time", burn_time)

		local damage_timer = mcl_burning.get(obj, "float", "damage_timer") + dtime

		if damage_timer >= 1 then
			damage_timer = 0
			mcl_burning.damage(obj)
		end

		mcl_burning.set(obj, "float", "damage_timer", damage_timer)
	end

	mcl_burning.catch_fire_tick(obj, dtime)
end
In `mods/ENTITIES/mcl_burning/api.lua`: ``` function mcl_burning.tick(obj, dtime) local burn_time = mcl_burning.get(obj, "float", "burn_time") - dtime if burn_time <= 0 then mcl_burning.extinguish(obj) else mcl_burning.set(obj, "float", "burn_time", burn_time) local damage_timer = mcl_burning.get(obj, "float", "damage_timer") + dtime if damage_timer >= 1 then damage_timer = 0 mcl_burning.damage(obj) end mcl_burning.set(obj, "float", "damage_timer", damage_timer) end mcl_burning.catch_fire_tick(obj, dtime) end ```
erlehmann commented 2021-08-05 15:54:35 +02:00
Owner
Copy Link

This issue may cause about 100 packets/s:

; ./tools/analyze-packet-spam
2021-08-05 15:44:08: INFO[Main]: cmd 75 (TOCLIENT_HUDCHANGE) count 2559
2021-08-05 15:44:08: INFO[Main]: cmd 76 (TOCLIENT_HUD_SET_FLAGS) count 441
14.7	PACKET_COUNT_TOCLIENT_HUD_SET_FLAGS
85.3	PACKET_COUNT_TOCLIENT_HUDCHANGE
This issue may cause about 100 packets/s: ``` ; ./tools/analyze-packet-spam 2021-08-05 15:44:08: INFO[Main]: cmd 75 (TOCLIENT_HUDCHANGE) count 2559 2021-08-05 15:44:08: INFO[Main]: cmd 76 (TOCLIENT_HUD_SET_FLAGS) count 441 14.7 PACKET_COUNT_TOCLIENT_HUD_SET_FLAGS 85.3 PACKET_COUNT_TOCLIENT_HUDCHANGE ```
erlehmann commented 2021-08-05 15:55:18 +02:00
Owner
Copy Link

I wonder if fixing this also fixes burning in general causing performance problems.

I wonder if fixing this also fixes burning in general causing performance problems.
erlehmann commented 2021-08-05 16:00:18 +02:00
Owner
Copy Link

In mods/ENTITIES/mcl_burning/api.lua, mcl_burning.catch_fire_tick() is called unconditionally from mcl_burning.tick():

function mcl_burning.catch_fire_tick(obj, dtime)
	if mcl_burning.is_affected_by_rain(obj) or #mcl_burning.get_touching_nodes(obj, "group:puts_out_fire") > 0 then
		mcl_burning.extinguish(obj)
	else
		local set_on_fire_value = mcl_burning.get_highest_group_value(obj, "set_on_fire")

		if set_on_fire_value > 0 then
			mcl_burning.set_on_fire(obj, set_on_fire_value)
		end
	end
end
In `mods/ENTITIES/mcl_burning/api.lua`, `mcl_burning.catch_fire_tick()` is called unconditionally from `mcl_burning.tick()`: ``` function mcl_burning.catch_fire_tick(obj, dtime) if mcl_burning.is_affected_by_rain(obj) or #mcl_burning.get_touching_nodes(obj, "group:puts_out_fire") > 0 then mcl_burning.extinguish(obj) else local set_on_fire_value = mcl_burning.get_highest_group_value(obj, "set_on_fire") if set_on_fire_value > 0 then mcl_burning.set_on_fire(obj, set_on_fire_value) end end end ```
erlehmann commented 2021-08-05 16:07:17 +02:00
Owner
Copy Link

In mods/ENTITIES/mcl_burning/api.lua:

function mcl_burning.set_on_fire(obj, burn_time, reason)
	local luaentity = obj:get_luaentity()
	if luaentity and luaentity.fire_resistant then
		return
	end

	local old_burn_time = mcl_burning.get(obj, "float", "burn_time")
	local max_fire_prot_lvl = 0

	if obj:is_player() then
		if minetest.is_creative_enabled(obj:get_player_name()) then
			burn_time = burn_time / 100
		end

		local inv = obj:get_inventory()

		for i = 2, 5 do
			local stack = inv:get_stack("armor", i)

			local fire_prot_lvl = mcl_enchanting.get_enchantment(stack, "fire_protection")
			max_fire_prot_lvl = math.max(max_fire_prot_lvl, fire_prot_lvl)
		end
	end

	if max_fire_prot_lvl > 0 then
		burn_time = burn_time - math.floor(burn_time * max_fire_prot_lvl * 0.15)
	end

	if old_burn_time <= burn_time then
		local sound_id = mcl_burning.get(obj, "int", "sound_id")
		if sound_id == 0 then
			sound_id = minetest.sound_play("fire_fire", {
				object = obj,
				gain = 0.18,
				max_hear_distance = 16,
				loop = true,
			}) + 1
		end

		local hud_id
		if obj:is_player() then
			hud_id = mcl_burning.get(obj, "int", "hud_id")
			if hud_id == 0 then
				hud_id = obj:hud_add({
					hud_elem_type = "image",
					position = {x = 0.5, y = 0.5},
					scale = {x = -100, y = -100},
					text = "fire_basic_flame.png",
					z_index = 1000,
				}) + 1
			end
		end
		mcl_burning.set(obj, "float", "burn_time", burn_time)
		mcl_burning.set(obj, "string", "reason", reason)
		mcl_burning.set(obj, "int", "hud_id", hud_id)
		mcl_burning.set(obj, "int", "sound_id", sound_id)

		local fire_entity = minetest.add_entity(obj:get_pos(), "mcl_burning:fire")
		local minp, maxp = mcl_burning.get_collisionbox(obj)
		local obj_size = obj:get_properties().visual_size

		local vertical_grow_factor = 1.2
		local horizontal_grow_factor = 1.1
		local grow_vector = vector.new(horizontal_grow_factor, vertical_grow_factor, horizontal_grow_factor)

		local size = vector.subtract(maxp, minp)
		size = vector.multiply(size, grow_vector)
		size = vector.divide(size, obj_size)
		local offset = vector.new(0, size.y * 10 / 2, 0)

		fire_entity:set_properties({visual_size = size})
		fire_entity:set_attach(obj, "", offset, {x = 0, y = 0, z = 0})
		mcl_burning.update_animation_frame(obj, fire_entity, 0)
	end
end
In `mods/ENTITIES/mcl_burning/api.lua`: ``` function mcl_burning.set_on_fire(obj, burn_time, reason) local luaentity = obj:get_luaentity() if luaentity and luaentity.fire_resistant then return end local old_burn_time = mcl_burning.get(obj, "float", "burn_time") local max_fire_prot_lvl = 0 if obj:is_player() then if minetest.is_creative_enabled(obj:get_player_name()) then burn_time = burn_time / 100 end local inv = obj:get_inventory() for i = 2, 5 do local stack = inv:get_stack("armor", i) local fire_prot_lvl = mcl_enchanting.get_enchantment(stack, "fire_protection") max_fire_prot_lvl = math.max(max_fire_prot_lvl, fire_prot_lvl) end end if max_fire_prot_lvl > 0 then burn_time = burn_time - math.floor(burn_time * max_fire_prot_lvl * 0.15) end if old_burn_time <= burn_time then local sound_id = mcl_burning.get(obj, "int", "sound_id") if sound_id == 0 then sound_id = minetest.sound_play("fire_fire", { object = obj, gain = 0.18, max_hear_distance = 16, loop = true, }) + 1 end local hud_id if obj:is_player() then hud_id = mcl_burning.get(obj, "int", "hud_id") if hud_id == 0 then hud_id = obj:hud_add({ hud_elem_type = "image", position = {x = 0.5, y = 0.5}, scale = {x = -100, y = -100}, text = "fire_basic_flame.png", z_index = 1000, }) + 1 end end mcl_burning.set(obj, "float", "burn_time", burn_time) mcl_burning.set(obj, "string", "reason", reason) mcl_burning.set(obj, "int", "hud_id", hud_id) mcl_burning.set(obj, "int", "sound_id", sound_id) local fire_entity = minetest.add_entity(obj:get_pos(), "mcl_burning:fire") local minp, maxp = mcl_burning.get_collisionbox(obj) local obj_size = obj:get_properties().visual_size local vertical_grow_factor = 1.2 local horizontal_grow_factor = 1.1 local grow_vector = vector.new(horizontal_grow_factor, vertical_grow_factor, horizontal_grow_factor) local size = vector.subtract(maxp, minp) size = vector.multiply(size, grow_vector) size = vector.divide(size, obj_size) local offset = vector.new(0, size.y * 10 / 2, 0) fire_entity:set_properties({visual_size = size}) fire_entity:set_attach(obj, "", offset, {x = 0, y = 0, z = 0}) mcl_burning.update_animation_frame(obj, fire_entity, 0) end end ```
erlehmann commented 2021-08-05 16:08:36 +02:00
Owner
Copy Link

Could obj:hud_add() in mcl_burning.set_on_fire() be the smoking (hahaha) gun?

Could `obj:hud_add()` in `mcl_burning.set_on_fire()` be the smoking (hahaha) gun?
erlehmann commented 2021-08-05 16:44:19 +02:00
Owner
Copy Link

Turns out fire_basic_flame.png is not even an animation. Why is it added so often?

Turns out `fire_basic_flame.png` is not even an animation. Why is it added so often?
erlehmann commented 2021-08-05 16:46:49 +02:00
Owner
Copy Link
; cat mods/ENTITIES/mcl_burning/mod.conf 
name = mcl_burning
description = Burning Objects for MineClone2
author = Fleckenstein

@EliasFleckenstein03 as the author of the mcl_burning mod, would you help us investigate the issue?

``` ; cat mods/ENTITIES/mcl_burning/mod.conf name = mcl_burning description = Burning Objects for MineClone2 author = Fleckenstein ``` @EliasFleckenstein03 as the author of the mcl_burning mod, would you help us investigate the issue?
cora commented 2021-09-13 00:21:32 +02:00
Member
Copy Link

Turns out fire_basic_flame.png is not even an animation. Why is it added so often?

Maybe because fire_basic_flame.png isn't the issue here. mcl_burning_hud_flame_animated.png is where the party is at ^^.

Upon further investigation i would say setting the update frequency to lower (a lot lower, rn it's every 0.015 seconds from what i understand [which is even faster than globalstep frequency is in the first place right ? ] ).

I would set it to something like 0.2 for at least for multiplayer if we can differentiate that - i tried it and i think it looks ok.

the "limiting" logic is at

  • mods/ENTITIES/mcl_burning/api.lua:288

the offending function that calls the hud_change and is called too often is at (update_animation_frame)

  • mods/ENTITIES/mcl_burning/api.lua:246

However in my opinion this whole thing shouldnt be an animation if it needs to send every f. frame over the net. Something like animated png is off the table right ?

> Turns out `fire_basic_flame.png` is not even an animation. Why is it added so often? Maybe because fire_basic_flame.png isn't the issue here. mcl_burning_hud_flame_animated.png is where the party is at ^^. Upon further investigation i would say setting the update frequency to lower (a lot lower, rn it's every 0.015 seconds from what i understand [which is even faster than globalstep frequency is in the first place right ? ] ). I would set it to something like 0.2 for at least for multiplayer if we can differentiate that - i tried it and i think it looks ok. the "limiting" logic is at * mods/ENTITIES/mcl_burning/api.lua:288 the offending function that calls the hud_change and is called too often is at (update_animation_frame) * mods/ENTITIES/mcl_burning/api.lua:246 However in my opinion this whole thing shouldnt be an animation if it needs to send every f. frame over the net. Something like animated png is off the table right ?
cora commented 2021-09-13 00:28:30 +02:00
Member
Copy Link

However in my opinion this whole thing shouldnt be an animation if it needs to send every f. frame over the net. Something like animated png is off the table right ?

APNG does not seem to be supported by the engine :/

> However in my opinion this whole thing shouldnt be an animation if it needs to send every f. frame over the net. Something like animated png is off the table right ? APNG does not seem to be supported by the engine :/
cora commented 2021-09-13 12:54:56 +02:00
Member
Copy Link

Moving gif doesnt seem to work either. I personally would be in favor to have it not be animated at all to be honest - it's not like this is super fun too look at - the idea is to block the players view a bit right ? let's just use one of those animation frames imo.

Moving gif doesnt seem to work either. I personally would be in favor to have it not be animated at all to be honest - it's not like this is super fun too look at - the idea is to block the players view a bit right ? let's just use one of those animation frames imo.
cora commented 2021-09-13 12:59:24 +02:00
Member
Copy Link

I would be honestly interested though, what is the point of this "limiting"?

mods/ENTITIES/mcl_burning/api.lua:288

local animation_timer = self.animation_timer + dtime
if animation_timer >= 0.015 then
    animation_timer = 0
    local animation_frame = self.animation_frame + 1
    if animation_frame > mcl_burning.animation_frames - 1 then
        animation_frame = 0
    end
    mcl_burning.update_animation_frame(parent, obj, animation_frame)
    self.animation_frame = animation_frame
end
self.animation_timer = animation_timer

a wait of 0.015 translates to 66 2/3 times a second right ? Isnt that significantly faster than globalstep even is ? Where does this number come from? @EliasFleckenstein03

I would be honestly interested though, what is the point of this "limiting"? mods/ENTITIES/mcl_burning/api.lua:288 ``` local animation_timer = self.animation_timer + dtime if animation_timer >= 0.015 then animation_timer = 0 local animation_frame = self.animation_frame + 1 if animation_frame > mcl_burning.animation_frames - 1 then animation_frame = 0 end mcl_burning.update_animation_frame(parent, obj, animation_frame) self.animation_frame = animation_frame end self.animation_timer = animation_timer ``` a wait of 0.015 translates to 66 2/3 times a second right ? Isnt that significantly faster than globalstep even is ? Where does this number come from? @EliasFleckenstein03
cora added the
packet spam
 label 2021-09-13 14:56:20 +02:00
cora commented 2021-09-19 13:42:05 +02:00
Member
Copy Link

Note this mcl2 commit which relies on having the animation done client side (i.e. you need a csm).

27e4bd6d09

MineClone2/MineClone2#1864

Note this mcl2 commit which relies on having the animation done client side (i.e. you need a csm). https://git.minetest.land/MineClone2/MineClone2/commit/27e4bd6d09b8a1ea36e9b0120b5aa7b90d16cc5a https://git.minetest.land/MineClone2/MineClone2/issues/1864
cora commented 2021-09-20 18:24:20 +02:00
Member
Copy Link

Note this mcl2 commit which relies on having the animation done client side (i.e. you need a csm).

since they don't even seem to bother to fix this mess i propose just throttling it severely. To like 0.2 (wait) or sth.

> Note this mcl2 commit which relies on having the animation done client side (i.e. you need a csm). > since they don't even seem to bother to fix this mess i propose just throttling it severely. To like 0.2 (wait) or sth.
cora commented 2021-09-21 20:48:53 +02:00
Member
Copy Link

oof. I'm just finished making it a setting but it seems you can be on fire multiple times !?

try throttling the fps to like 1 or 2 with this commit:
be8b35df43

you will still be able to trigger the packet spam by catching fire multiple times :/

oof. I'm just finished making it a setting but it seems you can be on fire multiple times !? try throttling the fps to like 1 or 2 with this commit: https://git.minetest.land/Mineclonia/Mineclonia/commit/be8b35df438842c2081123ea31662bbd68aeb4b0 you will still be able to trigger the packet spam by catching fire multiple times :/
LizzyFleckenstein03 commented 2021-10-24 17:40:33 +02:00
Contributor
Copy Link

People will need to adapt to having to use client side code.

And Minetest will have to fix their rendering code.

I will do adjustments to my implementation however.

People will need to adapt to having to use client side code. And Minetest will have to fix their rendering code. I will do adjustments to my implementation however.
cora commented 2021-10-24 20:34:16 +02:00
Member
Copy Link

well i dont want to break backwards compatibility for this hence the setting to jus throttle it.

Another big problem is that you can get the animation multiple times. This commit fixes that:

bf8577f860

well i dont want to break backwards compatibility for this hence the setting to jus throttle it. Another big problem is that you can get the animation *multiple* times. This commit fixes that: https://git.minetest.land/Mineclonia/Mineclonia/commit/bf8577f86058af478d037920ec36809a60aa989d
LizzyFleckenstein03 commented 2021-10-24 20:40:20 +02:00
Contributor
Copy Link
for _, other in ipairs(minetest.get_objects_inside_radius(obj:get_pos(), 0)) do
			local luaentity = obj:get_luaentity()
			if luaentity and luaentity.name == "mcl_burning:fire" and not luaentity.doing_step and not luaentity.removed then
				do_remove = true
				break
			end
		end

this code prevented it.

and your fix does not update other burning related values, like the reason.

```lua for _, other in ipairs(minetest.get_objects_inside_radius(obj:get_pos(), 0)) do local luaentity = obj:get_luaentity() if luaentity and luaentity.name == "mcl_burning:fire" and not luaentity.doing_step and not luaentity.removed then do_remove = true break end end ``` this code prevented it. and your fix does not update other burning related values, like the reason.
cora commented 2021-10-24 22:53:36 +02:00
Member
Copy Link

and your fix does not update other burning related values, like the reason.

o thats a good point, looks like that can be fixed easily though. but to be fair anything is better than having that traffic generator run mulitple times ^^ - yes this has been an actual problem.

> and your fix does not update other burning related values, like the reason. o thats a good point, looks like that can be fixed easily though. but to be fair anything is better than having that traffic generator run mulitple times ^^ - yes this has been an actual problem.
erlehmann commented 2021-11-13 02:37:46 +01:00
Owner
Copy Link

Fixed by merge of this PR: #149

Fixed by merge of this PR: https://git.minetest.land/Mineclonia/Mineclonia/pulls/149
This repo is archived. You cannot comment on issues.
No Branch/Tag Specified
Branches Tags
master
fix_copyright_music
cora-production
extend-nether-upwards
refactor-stairs-api-to-allow-modnames
add-podzol-to-large-spruce-trees
uncarved-pumpkin-4
limit-redstone-distance
add-mcl2-maps
fix-nether-portal-stack-overflow-2
fix-nether-portal-stack-overflow
fix-malformed-clientready
fix-daylight-detector
optimize-png-textures
add-ruined-portal
mcl-game
fix-daylightsensor-chmodsayshello
fix-etable-lag
fix-nether-portal-particles
add-selftests-setting
fix-dimension-hopping
erlehmann-production
fix-find-nodes-in-area
fix-village-freeze
rewrite-weather
fix-weather-particles
easier-grass-path-creation
fix-lavacasting-4
fix-lavacasting-2
fix-lavacasting
minecart-with-mobs-2
uncarved-pumpkin
banner-mask-fix-3
fake_master
banner-mask-fix-2
banner-mask-fix
mineclone2-backport-portals-2
mineclone2-backport-portals
mineclone2-pr-1315-portals
0.71.0
0.70.0
Labels
Clear labels   
blocker

blocks release until resolved

  
bug

   
code quality

   
confirmed

   
critical

crashes, catastrophic bugs

  
discussion

   
high priority

   
incompatibility

incompatible with mcl2

  
incomplete feature

   
invalid

   
low priority

   
missing feauture

   
needs testing

   
packet spam

   
performance

   
project

meta-issues

  
regression

   
translations

   
unconfirmed
  
in review

   
ready for review
No Label
blocker
bug
code quality
confirmed
critical
discussion
high priority
incompatibility
incomplete feature
invalid
low priority
missing feauture
needs testing
packet spam
performance
project
regression
translations
unconfirmed
in review
ready for review
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
4 Participants
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: Mineclonia/Mineclonia#137
No description provided.
Delete Branch "%!s(<nil>)"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?