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

Expand multi-server support #1435

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

Rubenicos
Copy link

@Rubenicos Rubenicos commented Feb 17, 2025

Redis support -> Proxy support

First of all, any redis-related feature name was renamed to "proxy feature".

Why?

  • I was looking on TAB code to find a solution for multi-server support without using TAB on proxies.
  • I find out a pretty well-made feature to transfer messages across proxies wasted on RedisBungee messaging.
  • I don't want to mantain this feature with future TAB API updates like to share any plugin edit.

How it works?

proxy-support:
  enabled: true
  # Supported types: PLUGIN, REDIS, RABBITMQ
  type: PLUGIN
  plugin:
    # Compatible plugins: RedisBungee
    # If enabled and compatible plugin is found, hook is enabled to work with proxied players
    name: RedisBungee
  redis:
    url: 'redis://:password@localhost:6379/0'
  rabbitmq:
    exchange: 'plugin'
    url: 'amqp://guest:guest@localhost:5672/%2F'

# On backend servers you should set this option with the ID of the server as defined in your proxy config.
server-name: 'server'

By default, the functionality is the same as RedisBungee support configuration.
Adding compatibility for Redis and RabbitMQ using delivery4j library.

On proxy servers, it works with global player list.
On backend servers, it make global player list work.

For people that still using TAB v4

I made the same changes on this branch: https://github.com/MineLAT/TAB/tree/v4
You should compile it by yourself.

Note

I also add RGB support on pre 1.16 servers using TAB v4 + ViaVersion, tell me if you want that implementation on TAB v5 to make a pull request later.

player.getBelowNameNumber(),
null, // Unused by this objective slot
player.getBelowNameFancy()
);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, was the functionality of applying value on join missing the whole time?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, unlike nametag and yellow number, this one doesn't seems to be at the end of the class

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The proxy message class is separated in this one, it's not nested. I started refactoring, but didn't finish it.

@NEZNAMY
Copy link
Owner

NEZNAMY commented Feb 18, 2025

Overall interesting. I wouldn't call the potential wasted, as RedisBungee has been requested and used by more servers than this functionality, which has never been requested.

Viaversion hook sounds interesting. I don't know if there are many <1.16 servers that allow 1.16+ and want to show RGB colors to them. But I guess having it shouldn't hurt. I am more interested in NumberFormat support though. You only need to implement null (shouldn't be used by the plugin, but keeping it nullable because it is) and FixedFormat. The remaining two are redundant.

@Rubenicos
Copy link
Author

Rubenicos commented Feb 18, 2025

The fact this functionality has never been requested is because most networks don't keep separated the same server modes, and for the ones that do it have already become a standard that players list is separated between hosts.
Maybe by making a poll or something you can find that there are networks that will see this implementation useful, I also can bring other server owners to see what they think about this.

@NEZNAMY
Copy link
Owner

NEZNAMY commented Feb 22, 2025

Can you test global playerlist and check out if the new belowname code is actually needed? Then, it might be ready for merge.

relocate("org.json", "me.neznamy.tab.libs.org.json")
relocate("org.slf4j", "me.neznamy.tab.libs.org.slf4j")
relocate("com.rabbitmq", "me.neznamy.tab.libs.com.rabbitmq")
relocate("com.saicone.delivery4j", "me.neznamy.tab.libs.com.saicone.delivery4j")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't realize you are trying to tenfold the jar size. Are these really necessary? Google gson is available on every 1.8+ server, what would you even use slf4j for?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redis library depends on com.google.code.gson:gson, org.apache.commons:commons-pool2, org.json:json and org.slf4j:slf4j-api.
While RabbitMQ depends on org.slf4j:slf4j-api.

Slf4j API requires Slf4j NOP if there is no implementation.

I removed the shading of gson and slf4j, apparently most platforms have those libraries.

@Rubenicos
Copy link
Author

Can you test global playerlist and check out if the new belowname code is actually needed? Then, it might be ready for merge.

Technically is not needed since you cannot see a player entity that is not in the same backend server, but I don't know if it's needed for platforms like MultiPaper.

Btw, I will do some testing later, most of the changes made on this pull are not tested yet with TAB v5.

@NEZNAMY
Copy link
Owner

NEZNAMY commented Feb 22, 2025

Go ahead with the tests. Check the belowname too if the addition is really needed. What is the current jar size?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants