Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tab list api rough around the edges. #1455

Open
Rossterd opened this issue Nov 4, 2024 · 1 comment
Open

Tab list api rough around the edges. #1455

Rossterd opened this issue Nov 4, 2024 · 1 comment
Labels
type: bug Something isn't working

Comments

@Rossterd
Copy link

Rossterd commented Nov 4, 2024

Expected Behavior

The current tab list API allows the following to occur:

// Two players
Player player1 = p1.get();
Player player2 = p2.get();

// Game profile to send tablist entry for
GameProfile tabListPlayer = GameProfile.forOfflinePlayer("Johny");

// A TabListEntry which will internally reference Player1's TabList
TabListEntry tabListEntry = TabListEntry.builder()
    .profile(tabListPlayer)
    .displayName(Component.text("DisplayName").color(NamedTextColor.GOLD))
    .latency(1000)
    .gameMode(1)
    .tabList(player1.getTabList())
    .build();


// Add this entry both the player's TabLists. 
player1.getTabList().addEntry(tabListEntry);
player2.getTabList().addEntry(tabListEntry);

// If we try to change a value of the previously added TabListEntry for Player2,
// it changes the entry for Player1 because the entry still references that TabList.
Optional<TabListEntry> entry = player2.getTabList().getEntry(tabListPlayer.getId());
entry.ifPresent(listEntry -> listEntry.setDisplayName(Component.text("Changed DisplayName").color(NamedTextColor.DARK_PURPLE)));

Someone editing a tab list entry in player2's tab list would expect player2's tab list to change.

Actual Behavior

player1's tab list changes.

Steps to Reproduce

See code above.

Plugin List

Velocity Version

Velocity 3.4.0-SNAPSHOT (git-08a42b37-b449)

Additional Information

A simple solution is throwing an exception on the second AddEntry because player.getTabList() != tabListEntry.getTabList().

Another option is modifying the internal TabList when an TabListEntryis added to a TabList. But then why bother specifying a TabList when creating a TabListEntry? I think this area of the API is a little clunky in this respect and could use some work.

@Rossterd Rossterd added the type: bug Something isn't working label Nov 4, 2024
@electronicboy
Copy link
Member

The broken behavior here is that it doesn't support updating both tablists;

I've wanted to consider replacing this API with something that plugins can replace the impl of for their own needs (and all upserts, etc, would run through that). I'm not really sure what the answer here is, in the meantime, I'd assume that we should just try to make it track a set of tab lists, rather than just 1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants