Animated-TabList

Animated-TabList

65.2k Downloads

player-health placeholder bug

avescarelo opened this issue · 13 comments

commented

Problem

  1. Health display works without rounding, as a result the number is very long.

  2. 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)

clap-clap

Screenshots (optional)

image

commented
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.

if (str.contains("%player-health%")) {
str = str.replace("%player-health%", String.valueOf(p.getHealthData().health().get()));
}

commented

That's totally on them, not us.

Originally posted by @dualspiral in SpongePowered/SpongeForge#3190 (comment)

commented

@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.

commented

@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?

  1. download & install Tab plugin https://ore.spongepowered.org/montlikadani/%5BAnimated-Tab%5D---TabList/versions
  2. Go to the folder of plugin config (my: plugins/config/tablist)
  3. Open and edit file spongeConfig.conf https://gist.github.com/pingvikin/98c7b06b52ac1aa66ef1397768109bd9
  4. Restart server
  5. All
commented

It's plugin issue. Do not cache Player or other Entitiy objects. You are caching it in your Task Runnable code.

commented

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.

commented

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.

commented

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.

commented

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.

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.

commented

It's best advice i can give in this plugin's situation.

we only use the Player class for method arguments

  1. https://github.com/montlikadani/TabList/blob/master/Sponge/src/tablist/groups/TabPlayer.java#L22
  2. 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.

grafik

commented

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.

commented

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.

commented

That is what needs to go on our tracker - please file that issue on there.