player-health placeholder bug
avescarelo opened this issue · 13 comments
Problem
-
Health display works without rounding, as a result the number is very long.
-
After death, before restarting to the server. The tab will display health at zero.
For example: 0/40 hp
Details
Plugin version: 1.0.1
Minecraft: 1.12.2
SpongeAPI: 7.2.0-94d7fb22f
Sponge: 1.12.2-7.2.4-SNAPSHOT
SpongeForge: 1.12.2-2838-7.2.4-RC4054
Minecraft Forge: 14.23.5.2847
JVM: 1.8.0_252/64-bit (Private Build)
OS: Linux (4.19.0-041900-generic/amd64)
Configuration file(s)
Screenshots (optional)
2\. After death, before restarting to the server. The tab will display health at zero. For example: 0/40 hp
Then this is Sponge issue, because I only uses code, to get the health of the player.
TabList/Sponge/src/Variables.java
Lines 77 to 79 in 7aad4ed
That's totally on them, not us.
Originally posted by @dualspiral in SpongePowered/SpongeForge#3190 (comment)
@pingvikin
Before you take my statement out of context futher, I did not come through to this issue when I closed it. I based it on what you said:
(In short, the request is to add new placeholders and round their values)
That is absolutely nothing to do with us.
There seems to be another issue here that was not mentioned but I'm not about to go say it's Sponge unless a reduced test case can be made that shows this. I, nor those who work alongside me to determine issues, have got time to work through plugin code with our current Sponge API 8 work to determine whether the issue lies in the plugin or within Sponge, and if the latter, how that happens.
If we get a set of steps to reproduce with a simple setup, we'll look at it.
@pingvikin
Before you take my statement out of context futher, I did not come through to this issue when I closed it. I based it on what you said:
(In short, the request is to add new placeholders and round their values)
That is absolutely nothing to do with us.
There seems to be another issue here that was not mentioned but I'm not about to go say it's Sponge unless a reduced test case can be made that shows this. I, nor those who work alongside me to determine issues, have got time to work through plugin code with our current Sponge API 8 work to determine whether the issue lies in the plugin or within Sponge, and if the latter, how that happens.
If we get a set of steps to reproduce with a simple setup, we'll look at it.
Oh, sorry.
How to repeat this?
- download & install Tab plugin https://ore.spongepowered.org/montlikadani/%5BAnimated-Tab%5D---TabList/versions
- Go to the folder of plugin config (my: plugins/config/tablist)
- Open and edit file spongeConfig.conf https://gist.github.com/pingvikin/98c7b06b52ac1aa66ef1397768109bd9
- Restart server
- All
It's plugin issue. Do not cache Player or other Entitiy objects. You are caching it in your Task
Runnable code.
Ah yes. I did think that would probably be it, but I didn’t go through all the code - now I have.
https://github.com/montlikadani/TabList/blob/master/Sponge/src/tablist/groups/TabPlayer.java#L22
Don’t store this. Store the UUID and get the player when you need it.
it's the same thing when you die. So if you die somehow (/kill), and respawns, then your current health returns as 0.0.
It seriously sounds like you're not getting the new player object. If you keep querying the old player object after death, yes, the health will still be 0.
It's plugin issue. Do not cache Player or other Entitiy objects. You are caching it in your
Task
Runnable code.
Bingo. Get player instances by UUID
, don't recycle usages of player/entity objects directly.
Looks like plugin is full of memory leaks.
Replace all your methods accepting Player
to accept UUID
of player, this way you will be 100% safe from memory leaks related to Player object.
e.g.
private void updateTab(final Player p, final List<String> worlds)
->
private void updateTab(final UUID p, final List<String> worlds)
and so on.
Looks like plugin is full of memory leaks.
Replace all your methods acceptingPlayer
to acceptUUID
of player, this way you will be 100% safe from memory leaks related to Player object.
e.g.
private void updateTab(final Player p, final List<String> worlds)
->
private void updateTab(final UUID p, final List<String> worlds)
and so on.
How long do you think this is a memory leak when we only use the Player class for method arguments? A memory leak is when you have a list or a map that contains a lot of data that you need to store. Or create plenty of class instances in a loop.
It's best advice i can give in this plugin's situation.
we only use the Player class for method arguments
- https://github.com/montlikadani/TabList/blob/master/Sponge/src/tablist/groups/TabPlayer.java#L22
- Also you keep references to it on stack in update loop.
I did not look through all code, so there may be more cases. Easiest solution is just replace Player
to it's UUID
everywhere and get it by UUID
when you need Player
.
Also this:
groupsList.parallelStream()
https://github.com/montlikadani/TabList/blob/master/Sponge/src/tablist/groups/TabPlayer.java#L37
This will be extremely slow unless groupsList
has more than 10-15 thousands of objects inside.
There seems to be another issue here that was not mentioned but I'm not about to go say it's Sponge unless a reduced test case can be made that shows this. I, nor those who work alongside me to determine issues, have got time to work through plugin code with our current Sponge API 8 work to determine whether the issue lies in the plugin or within Sponge, and if the latter, how that happens.
If we get a set of steps to reproduce with a simple setup, we'll look at it.
Tested now with
p.get(Keys.HEALTH).get()
it's the same thing when you die. So if you die somehow (/kill), and respawns, then your current health returns as 0.0. This can reproduces in 7.2.4-RC378
version of Sponge and using api-8.