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

Mirror Support with custom transport #138

Closed
MichalPetryka opened this issue Dec 2, 2018 · 16 comments
Closed

Mirror Support with custom transport #138

MichalPetryka opened this issue Dec 2, 2018 · 16 comments

Comments

@MichalPetryka
Copy link
Contributor

MichalPetryka commented Dec 2, 2018

I know that you said in #124 that you won't support Mirror because of TCP, but what about mirror with custom transport?
There's UDP based transport for Mirror, Ignorance https://github.com/SoftwareGuy/Ignorance/
Could you consider supporting it?

@martindevans
Copy link
Member

That looks like a good backend for Mirror. However it's fairly new so we'll wait at least a few months to make sure that development on it continues and it's stable etc before really considering supporting it.

Don't forget that you don't need to wait for us to "officially" support a networking system. Mirror is meant to be a replacement for HLAPI, so you can take our HLAPI network integration and adapt it to work with Mirror. The person who initially requested Mirror support in #120 only had ~20 errors when he swapped it over (fairly minor ones, by the look of it). So I wouldn't anticipate that taking very long to fix.

@MichalPetryka
Copy link
Contributor Author

One big difference in Mirror that might cause issues is that it doesn't have channels

@martindevans
Copy link
Member

Dissonance needs unreliable transport for voice packets and reliable transport for session control packets, so if Mirror doesn't support channels we won't ever be able to support it. I just had a quick look through Ignorance and Mirror on Github and it looks like there is at least partial support for channels. Ignorance has this (note channelId). Mirror has this (added just one month ago). It might be worth contacting the mirror dev and asking what he's planning for channels, and how to use it.

Even if mirror itself does not support reliable/unreliable channels another thing you could do is send Dissonance/Reliable packets through Mirror and send Dissonance/Unreliable packets directly through ignorance. Of course that ties your network implementation to Ignorance. If you decide to do that I would suggest implementing Dissonance on top of Mirror by adapting the HLAPI integration and then once that is working you just need to modify the SendUnreliable method to use Ignorance directly.

@MichalPetryka
Copy link
Contributor Author

Looking at code makes me think that everything will be working without modifications, Dissonance gives channel number in NetworkConnection.SendBytes, which Mirror passes to Ignorance, which passes it into ENET. I think that in mirror hardcoding 0 as Reliable and 1 as unreliable would make everything working

@MichalPetryka
Copy link
Contributor Author

Another issue is that mirror doesn't have NetworkConnection.lastError and they said that they aren't planning to add it and they also said "just override OnServerError and OnServerClient in your NetworkManager and do whatever you want with the exception."

@martindevans
Copy link
Member

We only use lastError to detect if a connection has timed out. It should be fairly easy to override the method they suggest and set a flag if the connection has timed out - then where the code currently checks lastError == NetworkError.Timeout just check if the flag has been set instead.

@pallasathena1
Copy link

Is anyone else working on this? It looks like mirror 1.3 and ignorance 2017 are reasonably stable releases, and Coburn has said that ignorance2017 is just going to be in maintenance mode, to receive updates to maintain compatibility with mirror, but that all major changes to the ignorance UDP library will be in the 2018 branch.

@martindevans
Copy link
Member

This is probably going to be our next network integration. I've been keeping an eye on Ignorance development and it looks good. I think I'll open some PRs next week to fix some issues I've seen with the library, and then if they get merged I'll put some time aside to develop a Dissonance+Mirror+Ignorance integration.

@MichalPetryka how did you get on with implementing this?

@MichalPetryka
Copy link
Contributor Author

Sorry for not responding, first, i've temporarly deferred moving, second i'm creating a custom LiteNetLib UDP-based transport for Mirror, which will probably also be ok. However, i've also noticed that in mirror sendbytes is protected and you have to create a custom message with bytes for sending.

@SoftwareGuy
Copy link

I merged the ArraySegment PR of Martin's into Ignorance 1.2; so that part of the puzzle is ready to go.

I still have a few bugs plus Mac OS compatibility that I need to address before I would call Ignorance 1.2 stable. Plus there's a lot of loose ends that another developer pointed out, so I need to get those addressed too.

@martindevans
Copy link
Member

martindevans commented Feb 16, 2019

Right now I have a prototype version of a Mirror integration that seems to mostly work (one more potential bug to investigate next week) - thankyou to BigBoxVR for sponsoring development of this! I hope to release this with the next version of Dissonance within a couple of weeks.

That may use the Ignorance 1.2 ArraySegment sending system, depending upon the release schedule of that. If the initial release of the integration doesn't use it I'll update the integration as soon as Ignorance 1.2 gets released :)

@MichalPetryka
Copy link
Contributor Author

MichalPetryka commented Feb 19, 2019

@martindevans could we get those methods as an interface for transport, so that other transports will also work with it?

@martindevans
Copy link
Member

martindevans commented Feb 19, 2019

Nope, Mirror rejected my PR which would add that. Until something like that is added I can either send packets through Mirror (lots of allocations) or directly through Ignorance (less allocations, tied to Ignorance transport) :(

@MichalPetryka
Copy link
Contributor Author

Could you take a look at SoftwareGuy/Ignorance#25 ?

@martindevans
Copy link
Member

I've looked at it. With Dissonance I plan to add a runtime test which would simply check:

if (NetworkManager.instance.transport is IgnoranceTransport) {
    // Use zero alloc methods
} else {
    // Fallback to standard methods
}

So your change does help with plugging in new backends to Dissonance+Mirror. If it's rejected then of course it's easy enough to add a new check into that branch for whatever transport you specifically want to use.

@martindevans
Copy link
Member

Dissonance 6.3.1 just went live on the asset store, check out the release notes for all the details including the download link for the Mirror+Ignorance integration!

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

No branches or pull requests

4 participants