PaperMC / Velocity

The modern, next-generation Minecraft server proxy.
https://papermc.io/software/velocity
GNU General Public License v3.0
1.77k stars 624 forks source link

Make TabListEntry have nullable fields #1315

Open ghost opened 6 months ago

ghost commented 6 months ago

The way addEntry() works, it only sends the appropriate update packet if the fields have changed.

But there is no way to specify you dont want a field to change, if you do TabListEntry.builder() for example, and dont specify gamemode, it defaults to 0, which you'd think means do nothing, but it sends an update packet to set them to survival (gamemode 0),

if it defaulted to null and did no operation it'd be nice as, in my velocity tab list plugin, spectators can't go through walls, since when the tab list updates it sends gamemode 0, and we cant explicitly specify it because on the proxy we don't know what gamemode they should be.

ghost commented 6 months ago

Every velocity tablist plugin has this exact issue, unless they hack in a packet listener to see which gamemode the backend server is telling client they should be

justlofe commented 3 months ago

+, it makes spectator mode dont working at all. i developed my own plugin which implements tab also with TabListEntry.builder(), but this bug creates really weird problems.

https://github.com/user-attachments/assets/efa53339-5675-4bc6-a91b-d7de6d1f249c

justlofe commented 3 months ago

I'm also tried to present a gamemode through plugin messaging (when player changes gamemode, it updates), but it didn't give a result. It fully broken.

RealBauHD commented 3 months ago

If you set the gamemode to -1 it will not be sent

justlofe commented 3 months ago

If you set the gamemode to -1 it will not be sent

Still broken

RealBauHD commented 3 months ago

Could you give more details like client version, show your code and where it is executed. Then I could make a deeper look

justlofe commented 3 months ago

Client: 1.21 Velocity: latest

It updates every second for all players.

    private void update(Player player) {
        TabList list = player.getTabList();
        player.sendPlayerListHeaderAndFooter(
                headerFooter("header", player),
                headerFooter("footer", player)
        );

        list.clearAll();
        list.addEntries(entries(list));
    }

    private TabListEntry entry(TabList list, Player player) {
        return TabListEntry.builder()
                .listed(true)
                .tabList(list)
                .profile(player.getGameProfile())
                .gameMode(-1)
                .displayName(name(tab(), player))
                .build();
    }

    private TabListEntry[] entries(TabList list) {
        List<Player> players = sort(xMain.I.getServer().getAllPlayers());
        TabListEntry[] entries = new TabListEntry[players.size()];
        for (int i = 0; i < players.size(); i++) {
            entries[i] = entry(list, players.get(i));
        }
        return entries;
    }
RealBauHD commented 3 months ago

Oh yes, because you delete all entries beforehand and no game mode is sent along, it automatically takes game mode 0. So I have two options here that should work.

  1. You don't delete the old entries, you just change them, which would also mean less traffic.

  2. You temporarily save the game modes of the old entries and then send them along.

electronicboy commented 3 months ago

making stuff nullable in your case is not really going to change much, you remove the entry from the list, so there is nothing to upsert, and so we'd enter questionable state in general. you'd basically want to modify the existing entries, etc.

There is probably stuff we can do here, especially around supporting upserts, but, the API is somewhat in a state of "if you want to manage the tab list yourself, you need to manage it yourself", adding new entries creates some headaches around properly sending the expected data to the client. all you'd do not sending the gamemode is likely cause it to be left in the default state, I doubt it would correct any issues you're seeing.

justlofe commented 3 months ago

Oh yes, because you delete all entries beforehand and no game mode is sent along, it automatically takes game mode 0. So I have two options here that should work.

  1. You don't delete the old entries, you just change them, which would also mean less traffic.
  2. You temporarily save the game modes of the old entries and then send them along.

Ok, this worked. Thank you very much!

For someone who also trying to do tab on velocity:

    public TabListener() {
        xMain.I.getServer().getScheduler().buildTask(xMain.I, () ->
                xMain.I.getServer().getAllPlayers().forEach(this::update)
        ).repeat(1, TimeUnit.SECONDS).schedule();
    }

    @Subscribe
    public void onServerConnect(ServerConnectedEvent event) {
        Player player = event.getPlayer();
        update(player);
    }

    private void update(Player player) {
        TabList list = player.getTabList();
        player.sendPlayerListHeaderAndFooter(
                headerFooter("header", player),
                headerFooter("footer", player)
        );

        for(Player temp: xMain.I.getServer().getAllPlayers()) {
            Optional<TabListEntry> opt = list.getEntry(temp.getUniqueId());
            if(opt.isPresent()) {
                TabListEntry entry = opt.get();
                entry.setDisplayName(name(tab(), temp)); // name is method to format names, and tab() is a static configuration
            }
        }
    }

    private @NotNull Component headerFooter(String type, Player player) {
        if(!type.equals("header") && !type.equals("footer")) return Component.text("");

        StringBuilder builder = new StringBuilder();
        List<String> raw = tab().getStringList(type);
        for (int i = 0; i < raw.size(); i++) {
            builder.append(raw.get(i));
            if(i < raw.size() - 1) builder.append("\n");
        }

        return Message.wrap(builder.toString(), player);
    }