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

HUD/hudbars: Do not send useless HUDCHANGE packets #122

Merged
erlehmann merged 1 commits from fix-hudchange-spam into master 2021-07-22 03:21:23 +02:00
Conversation 7 Commits 1 Files Changed 1 +2
erlehmann commented 2021-07-17 08:07:12 +02:00
Owner
Copy Link

Problem
TRACKING ISSUE: #111

Minetest servers running Mineclonia send HUDCHANGE packets to each client to hide or show HUD bars several times a second, even when the HUD bar visibility did not change.

Solution

HUD bar state of a client is already cached on the server.

With this PR, when the visibility of a HUD bar does not change, no HUDCHANGE packet is sent.

Details

The patch was taken from the hudbars mod by Wuzzy:
https://repo.or.cz/minetest_hudbars.git/commitdiff/a44de6230bcde79b78749bc6024a5ee8614bc6c0

Testing Steps
Verify Bug
  1. Check out Mineclonia commit c1cf50ae15.
  2. In Bourne shell, run Minetest with timeout 600 minetest --info >log1.txt 2>&1 >/dev/null.
  3. Create a new world with mapgen v7 and the seed packetspam.
  4. Enter the world in survival mode with damage turned on.
  5. In the chat/terminal, execute the following commands:
    • /grantme give
    • /giveme mcl_armor:helmet_iron
    • /giveme mcl_armor:chestplate_iron
    • /giveme mcl_armor:leggings_iron
    • /giveme mcl_armor:boots_iron
  6. Verify that you have the following items in your inventory:
    • 1 iron helmet
    • 1 iron chestplate
    • 1 iron leggings
    • 1 iron boots
  7. Verify that the armor bar is not visible.
  8. Verify that the breath bar is not visible.
  9. Open the inventory screen.
  10. Put on the pieces of armor one by one by moving them from the inventory to the appropriate armor slot.
  11. Verify that the armor bar shows more armor points the more armor you put on.
  12. Verify that the armor bar is visible as soon as you put on the first piece of armor.
  13. Take off the pieces of armor one by one by moving them from the appropriate armor slot to the inventory.
  14. Verify that the armor bar shows less armor points the more armor you take off.
  15. Verify that the armor bar is not visible as soon as you take off the last piece of armor.
  16. Dive into the water.
  17. Swim down, wait about 2 seconds, then swim to the surface again.
  18. Go on the land.
  19. Verify that the breath bar is visible a short time after the player character is fully below the surface of the water.
  20. Verify that as long as the player character is fully below the surface of the water, the breath bar shows a decreasing amount of breath bubbles.
  21. Verify that the breath bar is invisible a short time after it has been recharged outside the water.
  22. Wait until Minetest exits (about 10 minutes after it was started).
  23. In Bourne shell, execute ./tools/analyze-packet-spam <log1.txt.
  24. Verify that the line ending with PACKET_COUNT_TOCLIENT_HUDCHANGE in the output of the previous step starts with a number over 50.
  25. Include the number of the previous step in your testing protocol.
Verify Patch
  1. Checkout Mineclonia commit 280aed484c.
  2. In Bourne shell, run Minetest with timeout 600 minetest --info >log2.txt 2>&1 >/dev/null.
  3. Create a new world with mapgen v7 and the seed packetspam.
  4. Enter the world in survival mode with damage turned on.
  5. In the chat/terminal, execute the following commands:
    • /grantme give
    • /giveme mcl_armor:helmet_iron
    • /giveme mcl_armor:chestplate_iron
    • /giveme mcl_armor:leggings_iron
    • /giveme mcl_armor:boots_iron
  6. Verify that you have the following items in your inventory:
    • 1 iron helmet
    • 1 iron chestplate
    • 1 iron leggings
    • 1 iron boots
  7. Verify that the armor bar is not visible.
  8. Verify that the breath bar is not visible.
  9. Open the inventory screen.
  10. Put on the pieces of armor one by one by moving them from the inventory to the appropriate armor slot.
  11. Verify that the armor bar shows more armor points the more armor you put on.
  12. Verify that the armor bar is visible as soon as you put on the first piece of armor.
  13. Take off the pieces of armor one by one by moving them from the appropriate armor slot to the inventory.
  14. Verify that the armor bar shows less armor points the more armor you take off.
  15. Verify that the armor bar is not visible as soon as you take off the last piece of armor.
  16. Dive into the water.
  17. Swim down, wait about 2 seconds, then swim to the surface again.
  18. Go on the land.
  19. Verify that the breath bar is visible a short time after the player character is fully below the surface of the water.
  20. Verify that as long as the player character is fully below the surface of the water, the breath bar shows a decreasing amount of breath bubbles.
  21. Verify that the breath bar is invisible a short time after it has been recharged outside the water.
  22. Wait until Minetest exits (about 10 minutes after it was started).
  23. In Bourne shell, execute ./tools/analyze-packet-spam <log2.txt.
  24. Verify that the line ending with PACKET_COUNT_TOCLIENT_HUDCHANGE in the output of the previous step starts with a number less than 10.
  25. Include the number of the previous step in the testing protocol.
To do
  • Fill out issue template
  • Write testing steps
  • Verify testing
Problem TRACKING ISSUE: https://git.minetest.land/Mineclonia/Mineclonia/issues/111 Minetest servers running Mineclonia send HUDCHANGE packets to each client to hide or show HUD bars several times a second, even when the HUD bar visibility did not change. ##### Solution HUD bar state of a client is already cached on the server. With this PR, when the visibility of a HUD bar does not change, no HUDCHANGE packet is sent. ##### Details The patch was taken from the hudbars mod by Wuzzy: https://repo.or.cz/minetest_hudbars.git/commitdiff/a44de6230bcde79b78749bc6024a5ee8614bc6c0 ##### Testing Steps ###### Verify Bug 1. Check out Mineclonia commit c1cf50ae1568cecca2f64c158acb9fe5721d8248. 2. In Bourne shell, run Minetest with `timeout 600 minetest --info >log1.txt 2>&1 >/dev/null`. 3. Create a new world with mapgen v7 and the seed `packetspam`. 3. Enter the world in survival mode with damage turned on. 4. In the chat/terminal, execute the following commands: - /grantme give - /giveme mcl_armor:helmet_iron - /giveme mcl_armor:chestplate_iron - /giveme mcl_armor:leggings_iron - /giveme mcl_armor:boots_iron 5. Verify that you have the following items in your inventory: - 1 iron helmet - 1 iron chestplate - 1 iron leggings - 1 iron boots 6. Verify that the armor bar is not visible. 6. Verify that the breath bar is not visible. 7. Open the inventory screen. 7. Put on the pieces of armor one by one by moving them from the inventory to the appropriate armor slot. 9. Verify that the armor bar shows more armor points the more armor you put on. 8. Verify that the armor bar is visible as soon as you put on the first piece of armor. 10. Take off the pieces of armor one by one by moving them from the appropriate armor slot to the inventory. 12. Verify that the armor bar shows less armor points the more armor you take off. 11. Verify that the armor bar is not visible as soon as you take off the last piece of armor. 13. Dive into the water. 13. Swim down, wait about 2 seconds, then swim to the surface again. 13. Go on the land. 14. Verify that the breath bar is visible a short time after the player character is fully below the surface of the water. 15. Verify that as long as the player character is fully below the surface of the water, the breath bar shows a decreasing amount of breath bubbles. 16. Verify that the breath bar is invisible a short time after it has been recharged outside the water. 17. Wait until Minetest exits (about 10 minutes after it was started). 18. In Bourne shell, execute `./tools/analyze-packet-spam <log1.txt`. 19. Verify that the line ending with `PACKET_COUNT_TOCLIENT_HUDCHANGE` in the output of the previous step starts with a number over 50. 20. Include the number of the previous step in your testing protocol. ###### Verify Patch 1. Checkout Mineclonia commit 280aed484ce7adb3fc52df1bca79c8770b4e8719. 2. In Bourne shell, run Minetest with `timeout 600 minetest --info >log2.txt 2>&1 >/dev/null`. 3. Create a new world with mapgen v7 and the seed `packetspam`. 3. Enter the world in survival mode with damage turned on. 4. In the chat/terminal, execute the following commands: - /grantme give - /giveme mcl_armor:helmet_iron - /giveme mcl_armor:chestplate_iron - /giveme mcl_armor:leggings_iron - /giveme mcl_armor:boots_iron 5. Verify that you have the following items in your inventory: - 1 iron helmet - 1 iron chestplate - 1 iron leggings - 1 iron boots 6. Verify that the armor bar is not visible. 6. Verify that the breath bar is not visible. 7. Open the inventory screen. 7. Put on the pieces of armor one by one by moving them from the inventory to the appropriate armor slot. 9. Verify that the armor bar shows more armor points the more armor you put on. 8. Verify that the armor bar is visible as soon as you put on the first piece of armor. 10. Take off the pieces of armor one by one by moving them from the appropriate armor slot to the inventory. 12. Verify that the armor bar shows less armor points the more armor you take off. 11. Verify that the armor bar is not visible as soon as you take off the last piece of armor. 13. Dive into the water. 13. Swim down, wait about 2 seconds, then swim to the surface again. 13. Go on the land. 14. Verify that the breath bar is visible a short time after the player character is fully below the surface of the water. 15. Verify that as long as the player character is fully below the surface of the water, the breath bar shows a decreasing amount of breath bubbles. 16. Verify that the breath bar is invisible a short time after it has been recharged outside the water. 17. Wait until Minetest exits (about 10 minutes after it was started). 18. In Bourne shell, execute `./tools/analyze-packet-spam <log2.txt`. 19. Verify that the line ending with `PACKET_COUNT_TOCLIENT_HUDCHANGE` in the output of the previous step starts with a number less than 10. 20. Include the number of the previous step in the testing protocol. ##### To do - [x] Fill out issue template - [x] Write testing steps - [ ] Verify testing
erlehmann added a new dependency 2021-07-17 10:24:07 +02:00
#123 Add script to show packets count from client logs
erlehmann changed title from WIP: HUD/hudbars: Do not send useless HUDCHANGE packets to HUD/hudbars: Do not send useless HUDCHANGE packets 2021-07-17 10:24:51 +02:00
erlehmann changed title from HUD/hudbars: Do not send useless HUDCHANGE packets to WIP: HUD/hudbars: Do not send useless HUDCHANGE packets 2021-07-17 10:25:12 +02:00
erlehmann commented 2021-07-17 10:25:42 +02:00
Author
Owner
Copy Link

This PR will be rebased once #123 is merged.

This PR will be rebased once https://git.minetest.land/Mineclonia/Mineclonia/pulls/123 is merged.
erlehmann force-pushed fix-hudchange-spam from 51d7d8e133 to 280aed484c 2021-07-21 10:29:20 +02:00 Compare
erlehmann commented 2021-07-21 10:31:13 +02:00
Author
Owner
Copy Link

Rebased.

Rebased.
erlehmann changed title from WIP: HUD/hudbars: Do not send useless HUDCHANGE packets to HUD/hudbars: Do not send useless HUDCHANGE packets 2021-07-21 18:15:26 +02:00
cora commented 2021-07-21 22:25:56 +02:00
Member
Copy Link

I've tested this - a number of times but 21 keeps failing

"Verify that the breath bar is invisible a short time after it has been recharged outside the water."

it is invisible as soon as it's reachared (I don't even see the last bubble full), there is no delay neither in c1cf50ae15 nor in 280aed484c.

I'm not sure why this is even part of the testing (a supposed delay after recharge). I'm also not sure why its necessary to repeat all the preparation steps from verify bug in the verify fix part (creating world, giving yourself armor) - with copy and paste being so bad in minetest I think it would be good to make testers do as little /give's as possible (unless there is some reason for it i don't see).

I've tested this - a number of times but 21 keeps failing "Verify that the breath bar is invisible a short time after it has been recharged outside the water." it is invisible as soon as it's reachared (I don't even see the last bubble full), there is no delay neither in c1cf50ae15 nor in 280aed484c. I'm not sure why this is even part of the testing (a supposed delay after recharge). I'm also not sure why its necessary to repeat all the preparation steps from verify bug in the verify fix part (creating world, giving yourself armor) - with copy and paste being so bad in minetest I think it would be good to make testers do as little /give's as possible (unless there is some reason for it i don't see).
cora approved these changes 2021-07-21 22:31:27 +02:00
cora left a comment
Member

verify bug

  • Check out Mineclonia commit c1cf50ae15.
  • In Bourne shell, run Minetest with timeout 600 minetest --info >log1.txt 2>&1 >/dev/null.
  • Create a new world with mapgen v7 and the seed packetspam.
  • Enter the world in survival mode with damage turned on.
  • In the chat/terminal, execute the following commands:
  • /grantme give
  • /giveme mcl_armor:helmet_iron
  • /giveme mcl_armor:chestplate_iron
  • /giveme mcl_armor:leggings_iron
  • /giveme mcl_armor:boots_iron
  • Verify that you have the following items in your inventory:
  • 1 iron helmet
  • 1 iron chestplate
  • 1 iron leggings
  • 1 iron boots
  • Verify that the armor bar is not visible.
  • Verify that the breath bar is not visible.
  • Open the inventory screen.
  • Put on the pieces of armor one by one by moving them from the inventory to the appropriate armor slot.
  • Verify that the armor bar shows more armor points the more armor you put on.
  • Verify that the armor bar is visible as soon as you put on the first piece of armor.
  • Take off the pieces of armor one by one by moving them from the appropriate armor slot to the inventory.
  • Verify that the armor bar shows less armor points the more armor you take off.
  • Verify that the armor bar is not visible as soon as you take off the last piece of armor.
  • Dive into the water.
  • Swim down, wait about 2 seconds, then swim to the surface again.
  • Go on the land.
  • Verify that the breath bar is visible a short time after the player character is fully below the surface of the water.
  • Verify that as long as the player character is fully below the surface of the water, the breath bar shows a decreasing amount of breath bubbles.
  • [] Verify that the breath bar is invisible a short time after it has been recharged outside the water.
    This failed - it is invisible immediately upon recharge (you dont even see the last bubble full)
  • Wait until Minetest exits (about 10 minutes after it was started).
  • In Bourne shell, execute ./tools/analyze-packet-spam <log1.txt.
  • Verify that the line ending with PACKET_COUNT_TOCLIENT_HUDCHANGE in the output of the previous step starts with a number over 50.
  • Include the number of the previous step in your testing protocol : 54.9

Verify fix

  • Checkout Mineclonia commit 280aed484c.

  • In Bourne shell, run Minetest with timeout 600 minetest --info >log2.txt 2>&1 >/dev/null.

  • Create a new world with mapgen v7 and the seed packetspam.

  • Enter the world in survival mode with damage turned on.

  • In the chat/terminal, execute the following commands:

  • /grantme give

  • /giveme mcl_armor:helmet_iron

  • /giveme mcl_armor:chestplate_iron

  • /giveme mcl_armor:leggings_iron

  • /giveme mcl_armor:boots_iron

  • Verify that you have the following items in your inventory:

  • 1 iron helmet

  • 1 iron chestplate

  • 1 iron leggings

  • 1 iron boots

  • Verify that the armor bar is not visible.

  • Verify that the breath bar is not visible.

  • Open the inventory screen.

  • Put on the pieces of armor one by one by moving them from the inventory to the appropriate armor slot.

  • Verify that the armor bar shows more armor points the more armor you put on.

  • Verify that the armor bar is visible as soon as you put on the first piece of armor.

  • Take off the pieces of armor one by one by moving them from the appropriate armor slot to the inventory.

  • Verify that the armor bar shows less armor points the more armor you take off.

  • Verify that the armor bar is not visible as soon as you take off the last piece of armor.

  • Dive into the water.

  • Swim down, wait about 2 seconds, then swim to the surface again.

  • Go on the land.

  • Verify that the breath bar is visible a short time after the player character is fully below the surface of the water.

  • Verify that as long as the player character is fully below the surface of the water, the breath bar shows a decreasing amount of breath bubbles.

  • [] Verify that the breath bar is invisible a short time after it has been recharged outside the water.
    (same as in verify bug, no delay)

  • Wait until Minetest exits (about 10 minutes after it was started).

  • In Bourne shell, execute ./tools/analyze-packet-spam <log2.txt.

  • Verify that the line ending with PACKET_COUNT_TOCLIENT_HUDCHANGE in the output of the previous step starts with a number less than 10.

  • Include the number of the previous step in the testing protocol : .1

I think this is a perfect example of being overly specific - the delay is not what we are testing here and indeed neither the code suggests it should have anything to do with it nor does the behavior noticeably change between with and without the fix.

I will still approve it but i think this is less than ideal.

### verify bug - [x] Check out Mineclonia commit c1cf50ae15. - [x] In Bourne shell, run Minetest with timeout 600 minetest --info >log1.txt 2>&1 >/dev/null. - [x] Create a new world with mapgen v7 and the seed packetspam. - [x] Enter the world in survival mode with damage turned on. - [x] In the chat/terminal, execute the following commands: - [x] /grantme give - [x] /giveme mcl_armor:helmet_iron - [x] /giveme mcl_armor:chestplate_iron - [x] /giveme mcl_armor:leggings_iron - [x] /giveme mcl_armor:boots_iron - [x] Verify that you have the following items in your inventory: - [x] 1 iron helmet - [x] 1 iron chestplate - [x] 1 iron leggings - [x] 1 iron boots - [x] Verify that the armor bar is not visible. - [x] Verify that the breath bar is not visible. - [x] Open the inventory screen. - [x] Put on the pieces of armor one by one by moving them from the inventory to the appropriate armor slot. - [x] Verify that the armor bar shows more armor points the more armor you put on. - [x] Verify that the armor bar is visible as soon as you put on the first piece of armor. - [x] Take off the pieces of armor one by one by moving them from the appropriate armor slot to the inventory. - [x] Verify that the armor bar shows less armor points the more armor you take off. - [x] Verify that the armor bar is not visible as soon as you take off the last piece of armor. - [x] Dive into the water. - [x] Swim down, wait about 2 seconds, then swim to the surface again. - [x] Go on the land. - [x] Verify that the breath bar is visible a short time after the player character is fully below the surface of the water. - [x] Verify that as long as the player character is fully below the surface of the water, the breath bar shows a decreasing amount of breath bubbles. - [] Verify that the breath bar is invisible a short time after it has been recharged outside the water. This failed - it is invisible immediately upon recharge (you dont even see the last bubble full) - [x] Wait until Minetest exits (about 10 minutes after it was started). - [x] In Bourne shell, execute ./tools/analyze-packet-spam <log1.txt. - [x] Verify that the line ending with PACKET_COUNT_TOCLIENT_HUDCHANGE in the output of the previous step starts with a number over 50. - [x] Include the number of the previous step in your testing protocol : 54.9 ### Verify fix - [x] Checkout Mineclonia commit 280aed484c. - [x] In Bourne shell, run Minetest with timeout 600 minetest --info >log2.txt 2>&1 >/dev/null. - [x] Create a new world with mapgen v7 and the seed packetspam. - [x] Enter the world in survival mode with damage turned on. - [x] In the chat/terminal, execute the following commands: - [x] /grantme give - [x] /giveme mcl_armor:helmet_iron - [x] /giveme mcl_armor:chestplate_iron - [x] /giveme mcl_armor:leggings_iron - [x] /giveme mcl_armor:boots_iron - [x] Verify that you have the following items in your inventory: - [x] 1 iron helmet - [x] 1 iron chestplate - [x] 1 iron leggings - [x] 1 iron boots - [x] Verify that the armor bar is not visible. - [x] Verify that the breath bar is not visible. - [x] Open the inventory screen. - [x] Put on the pieces of armor one by one by moving them from the inventory to the appropriate armor slot. - [x] Verify that the armor bar shows more armor points the more armor you put on. - [x] Verify that the armor bar is visible as soon as you put on the first piece of armor. - [x] Take off the pieces of armor one by one by moving them from the appropriate armor slot to the inventory. - [x] Verify that the armor bar shows less armor points the more armor you take off. - [x] Verify that the armor bar is not visible as soon as you take off the last piece of armor. - [x] Dive into the water. - [x] Swim down, wait about 2 seconds, then swim to the surface again. - [x] Go on the land. - [x] Verify that the breath bar is visible a short time after the player character is fully below the surface of the water. - [x] Verify that as long as the player character is fully below the surface of the water, the breath bar shows a decreasing amount of breath bubbles. - [] Verify that the breath bar is invisible a short time after it has been recharged outside the water. (same as in verify bug, no delay) - [x] Wait until Minetest exits (about 10 minutes after it was started). - [x] In Bourne shell, execute ./tools/analyze-packet-spam <log2.txt. - [x] Verify that the line ending with PACKET_COUNT_TOCLIENT_HUDCHANGE in the output of the previous step starts with a number less than 10. - [x] Include the number of the previous step in the testing protocol : .1 I think this is a perfect example of being overly specific - the delay is not what we are testing here and indeed neither the code suggests it should have anything to do with it nor does the behavior noticeably change between with and without the fix. I will still approve it but i think this is less than ideal.
cora commented 2021-07-21 22:58:51 +02:00
Member
Copy Link

There is no delay on clamity or mcl5 either btw. I think this is a mistake.

There is no delay on clamity or mcl5 either btw. I think this is a mistake.
erlehmann commented 2021-07-22 03:18:44 +02:00
Author
Owner
Copy Link

@cora oh, you are right, there is no last bubble. I was overly specific when writing it and the delay is not what we should test here for. Sorry for that. :/

@cora oh, you are right, there is no last bubble. I was overly specific when writing it and the delay is not what we should test here for. Sorry for that. :/
erlehmann commented 2021-07-22 03:20:42 +02:00
Author
Owner
Copy Link

@cora you are also right that I should have not made testers (i.e. you) go through the motions twice. I apologize, please forgive me.

@cora you are also right that I should have not made testers (i.e. you) go through the motions twice. I apologize, please forgive me.
erlehmann merged commit eddbfb4b5c into master 2021-07-22 03:21:23 +02:00
erlehmann deleted branch fix-hudchange-spam 2021-07-22 03:22:30 +02:00
erlehmann commented 2021-07-22 03:32:38 +02:00
Author
Owner
Copy Link

I'm also not sure why its necessary to repeat all the preparation steps from verify bug in the verify fix part (creating world, giving yourself armor)

I was really lazy and copied the whole instructions, then changed some minor details like the commit.

Please forgive me.

> I'm also not sure why its necessary to repeat all the preparation steps from verify bug in the verify fix part (creating world, giving yourself armor) I was really lazy and copied the whole instructions, then changed some minor details like the commit. Please forgive me.
This repo is archived. You cannot comment on pull requests.
Reviewers
No reviewers
cora
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
2 Participants
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Depends on
#123 Add script to show packets count from client logs
Mineclonia/Mineclonia
Reference: Mineclonia/Mineclonia#122
No description provided.
Delete Branch "fix-hudchange-spam"

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?