NEZNAMY / TAB

"That" TAB plugin.
Apache License 2.0
903 stars 245 forks source link

running LayoutManager.sendLayout freezes placeholders #1347

Closed U5B closed 3 months ago

U5B commented 3 months ago

Server version

Paper 1.19.4

TAB version

4.1.6

Plugin list

2024-08-05_12-18

Describe the bug

LayoutManager.sendLayout freezes placeholders

Expected behavior

If LayoutManager.sendLayout is sent twice, it shouldn't freeze placeholders

Steps to reproduce

  1. Setup a placeholder that refreshes every second.
  2. Run sendLayout once to see the placeholders refresh properly.
  3. Run sendLayout again to see the placeholders freeze.

Additional info

Header and Footer refreshes placeholders like normal Layout name is changing between sends

Checklist

U5B commented 3 months ago

No errors in server log And /tab parse me %placeholder_name% shows the placeholder updating as well

NEZNAMY commented 3 months ago

Send a screenshot / video and example code that reproduces the issue.

U5B commented 3 months ago

Code example

public void playerLoadEvent(PlayerLoadEvent event) {
    UUID uuid = event.getPlayer().getUniqueId();
    test1(event.getPlayer());
    Bukkit.getScheduler().runTaskLater(Plugin.getInstance(), () -> {
        test2(TabAPI.getInstance().getPlayer(uuid));
    }, 100);
    // Bukkit.getScheduler().runTaskAsynchronously(Plugin.getInstance(), () -> refresh());
}

public void test1(TabPlayer viewer) {
    Layout layout = TabAPI.getInstance().getLayoutManager().createNewLayout("layout_a");
    layout.addFixedSlot(1, "%monumenta_effect_1%");
    layout.addFixedSlot(2, "This is layout A");
    TabAPI.getInstance().getLayoutManager().sendLayout(viewer, layout);
}

public void test2(TabPlayer viewer) {
    Layout layout = TabAPI.getInstance().getLayoutManager().createNewLayout("layout_b");
    layout.addFixedSlot(1, "%monumenta_effect_1%");
    layout.addFixedSlot(2, "This is layout B");
    TabAPI.getInstance().getLayoutManager().sendLayout(viewer, layout);
}

https://github.com/user-attachments/assets/f647e8ef-2534-4a74-a3ec-8bb2e9e8538d

Sorry for the delay, wanted to minimize the amount of code

U5B commented 3 months ago

A workaround that I can do is run sendLayout constantly but that is quite silly if there were no changes others then the placeholders. Also sendLayout constantly makes the tablist flicker

U5B commented 3 months ago

Removing p.layoutData.view.getPattern() != pattern from https://github.com/NEZNAMY/TAB/blob/032af707dd24057129698a295303caff0a7d79f0/shared/src/main/java/me/neznamy/tab/shared/features/layout/FixedSlot.java#L32 seems to have "fixed" it. However I don't think this is a valid fix. Maybe this needs to be a name check specifically?

EDIT: !p.layoutData.view.getPattern().getName().equals(pattern.getName()) doesn't seem to work either

U5B commented 3 months ago

Another idea would to use PlaceholderManager to register placeholders and then just send a layout of placeholders. However, I wouldn't be able to adjust ping via placeholder then.

U5B commented 3 months ago

This doesn't seem like it have fixed it, but it might be specific to my setup

NEZNAMY commented 3 months ago

Have you tried the code you sent? I did and it was fixed. Have you tried latest build from github actions?

U5B commented 3 months ago

Have you tried the code you sent? I did and it was fixed. Have you tried latest build from github actions?

I tried the latest build from actions. And that code didn't work. However, I did have values in a default layout so it might be because of that. Try making a loop where it swaps between test1 and test2 layout.

U5B commented 3 months ago

Hmm, having a unique Layout between refreshes (using .createLayout(UUID.randomUUID)) fixed it. So I think it was an issue with my setup.

NEZNAMY commented 3 months ago

Did you try to create layouts with duplicated names?

U5B commented 3 months ago

Did you try to create layouts with duplicated names?

Yes. My original code swapped between two names test1 and test2 in a loop.

NEZNAMY commented 3 months ago

That behavior is not supported. Makes sense now.

U5B commented 3 months ago

Yeah I am using TAB in an awkward way. I am using Layout to provide my own set of players since RedisBungee was unreliable in my setup. So every time a player joins/leaves, I resend the Layout. Each Layout is unique to the player's preferences.

Thank you for fixing the issue!