-
-
Notifications
You must be signed in to change notification settings - Fork 63
Add multi-server support #197
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My only concern here is the safety of automated database migration. First, I would prefer there to be a way to turn this on (config toggle). If this is the case, it will performt he DB migration as needed. Above the toggle, in full caps it can be made clear that "if you don't have a backup: expect to lose all your data. This toggle will migrate your DB IRP tables at your own risk."
Otherwise, I also commented a way to fulfill the teleportation without instant messaging.
Overall, If DB safety, full teleportation, & the minor cleanup is resolved, I would happily merge this. Also, merge into dev please, not main. This will need testing :p
|
|
||
| if (!Objects.equals(ConfigData.getServerName(), location[0])) { | ||
| ByteArrayOutputStream b = new ByteArrayOutputStream(); | ||
| DataOutputStream out = new DataOutputStream(b); | ||
|
|
||
| out.writeUTF("Connect"); | ||
| out.writeUTF(location[0]); | ||
|
|
||
| staff.sendPluginMessage(InventoryRollback.getInstance(), "BungeeCord", b.toByteArray()); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Util class here would be nice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, no need for redis to teleport to the correct location. Setting an expiring connection teleport 'intent' (or whatever you want to call it), would be sufficient: user click TP -> intent entry is recorded into DB table (await) -> Bungee msg -> user async pre-login -> lookup (with SQL time validity check condition) -> hold intent in cache -> user joins (PlayerSpawnLocationEvent iirc) -> execute location set (effectively teleport) if location valid for the server & permissions for TP still exist.
User disconnection or login failure -> cache invalidation
| String update = "INSERT INTO " + backupTable.getTableName() + " " + | ||
| "(uuid, timestamp, xp, health, hunger, saturation, location_world, location_x, location_y, location_z, version, death_reason, main_inventory, armour, ender_chest)" + " " + | ||
| "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"; | ||
| "(uuid, timestamp, xp, health, hunger, saturation, location_server, location_world, location_x, location_y, location_z, version, death_reason, main_inventory, armour, ender_chest)" + " " + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
User-optional DB migration for previous installs (as mentioned in original PR comment)
Sorry, my bad haha. Currently I don't really have the time to work on fully building the migration or any of the parts myself. I might be able to revisit it later tho. But feel free to intergrate your own version if you prefer, since I'm also not that experienced in Java, so there might be places to improve on :) |
Hi,
I had a very specific usecase for this plugin;
We are running multiple servers connected to the same Velocity network, and everything is shared between those servers. Inventory, health, items, and a single Skyblock instance is hosting islands across those servers to reduce and spread load.
We wanted to use Irp on all servers, connected to the same database (so it all feels like the same server). However, all worlds have simular names, so you couldn't tell apart which server a death happened in, so if you tried to visit it the location would not be the same.
With this commit I added part of support to additionally log a servername (configurable in the config). Servernames are shown with the irp's, so you can visually see where it happened. If you try to teleport to one in another server, you will be sent to that server instead. I couldn't figure out a way to TP to the death after you switched servers without the need of some sort of redis / proxy plugin setup, so I figured this was good enough for me.
Feel free to use this, however its not migrating any existing backups yet. We could start with a clean database, so I didn't specifically needed that, and figured if you are not gonne add this (which is understandable) it would be a waste of time.
Everything should work as it is now if you are not using a proxy. It'll just always send to the right location.
This also is a solution to Issue #51