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

Multiplayer Support. #153

Open
wants to merge 2 commits into
base: 1.1
Choose a base branch
from
Open

Multiplayer Support. #153

wants to merge 2 commits into from

Conversation

YellowOnion
Copy link

Added some multiplayer support.

Favorites currently unsupported, unsure on architecture wanted (should it be consistent between peers or each peer has a different set of favorites, how do we sync the files or avoid sending a lot of data across the wire)

Everything else /should/ work, but could have missed something... most O(n) operations have been wrapped to help with bandwidth issues too.

@Karel-Kroeze
Copy link
Contributor

interesting, I was toying with the idea of putting a watcher on the priorityTracker object, but I see you've gone for wrapping all the helper functions. I'm not sure that's the best way to go, but it sure is simpler. I'll try and make some time to test this soon.

Thank you very much for the contribution!

@FluffierThanThou
Copy link
Member

this does not seem like the correct approach to take, as it will sync dozens up to thousands of calls for SetPriority for each user interaction. Watching and synchronizing trackers seems like a much better idea.

@YellowOnion
Copy link
Author

YellowOnion commented Sep 16, 2020

as it will sync dozens up to thousands of calls for SetPriority

As I said, I also wrap the aggregate functions (fixing the O(n) issues), I had this issue at first but then wrapped the higher up calls and don't suffer from thousands of calls, just a few.

You have a lot of functions with very similar application (and similar names) that made wrapping the calls a little annoying.

The problem I saw with a watcher is you would still have to manually batch the call, and you would end up serializing a lot more data (it would still update thousands of entires!) over the wire than just serializing and mirroring the abstract high level command on all clients.

I consider refactoring all the High level commands so there's just one or two and separating the GUI out a lot more concretely out from the data updates, but I just wanted to get something working.

@Komoden
Copy link

Komoden commented Jan 1, 2021

The fact that Work Tab isn't multiplayer compatible is one of the main reasons i'm hessitant to start at multiplayer game with a friend.
I hope you can get this working... Or maybe release it as an initial version with a big Asterisk saying "Can potentially cause lag problems in multiplayer" - if this change doesn't affect singleplayer games in any way.

@YellowOnion
Copy link
Author

@Komoden you can try the builds on the pull request, they work for the most part without much lag, just don't use the favourites system.

@Komoden
Copy link

Komoden commented Jan 2, 2021

@YellowOnion So just grab the WorkTab.dll from your repo? I might be a programmer, but i haven't really looked into how mods in Rimworld work :)

@FluffierThanThou
Copy link
Member

it seems like I owe you an apology, I misunderstood how 'nested' sync calls would work. I was afraid all the atomic little changes would be synced, but I've since been informed that the MP API is smart enough not to sync methods recursively.

This does then seem like the simplest and most elegant approach to implement MP, and I'll have another look at rebasing it onto 1.3 once things settle down a bit.

@hrvatskibogmars
Copy link

It's a shame that this mod is not compatible with multiplayer with at least some partial compatibility. It would be very good to have this mod in multiplayer games.

@Shrimp6387
Copy link

Please give us multiplayer support.. we play as 3 but its not as good as it could be with this mod... rimworld is to restricted in work tabs....

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.

6 participants