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

enet: Search for shared library #2153

Merged
merged 2 commits into from
Mar 14, 2015
Merged

enet: Search for shared library #2153

merged 2 commits into from
Mar 14, 2015

Conversation

degasus
Copy link
Member

@degasus degasus commented Mar 1, 2015

No description provided.

@degasus degasus force-pushed the enet branch 2 times, most recently from 8ffcf45 to 8d00fa5 Compare March 1, 2015 01:53
@comex
Copy link
Contributor

comex commented Mar 2, 2015

I don't think this is a good idea. There are a few modifications to enet I want to make soon:

  • IPv6 support - some enet forks have implemented this (like, there is an almost 2 year old PR), but upstream doesn't have it. IMO, in 2015 no new application should omit IPv6, so this is important.
  • Opportunistic resending - the current merged enet-using code doesn't have this at all. My original enet branch supported it using a wrapper around stock enet, it would be more elegant and have a bit less overhead to do it in enet itself (it wouldn't require changing the protocol). This might be counteracted by the benefit of being able to use stock enet, but considering the first point...

@degasus
Copy link
Member Author

degasus commented Mar 2, 2015

rebased.

@comex I think we should try to not fork our externals if possible. Are those features possible to include within enet directly? So projects will win.
However, do you want me to drop this part of the PR or will you just revert it when you're going to break enet compatibility?

@comex
Copy link
Contributor

comex commented Mar 2, 2015

Well, I can try. The problem is that judging by the history of this pull request:

lsalzman/enet#21

I'm not too enthusiastic about upstream accepting patches.

But maybe if I implement IPv6 without breaking ABI compatibility, @lsalzman will be more interested.

@degasus
Copy link
Member Author

degasus commented Mar 5, 2015

@comex So do you think this should be merged as it is or shall I remove the global enet usage? I mean you're always free to revert this change on forking enet.

degasus added a commit that referenced this pull request Mar 14, 2015
enet: Search for shared library
@degasus degasus merged commit ec8a074 into dolphin-emu:master Mar 14, 2015
@degasus degasus deleted the enet branch August 9, 2015 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants