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

[Net] Split Unix/Windows NetSocket implementation #98969

Merged
merged 1 commit into from
Nov 12, 2024

Conversation

Faless
Copy link
Collaborator

@Faless Faless commented Nov 8, 2024

As expected, this results in quite some code duplication, but also make a clearer split between Unix and Windows socket implementation.

@adamscott adamscott added this to the 4.4 milestone Nov 8, 2024
drivers/unix/net_socket_unix.h Outdated Show resolved Hide resolved
@Repiteo
Copy link
Contributor

Repiteo commented Nov 9, 2024

Oh hey, if/when ip_unix gets the same treatment, we should be able to completely exclude the unix/windows folders based on platform in drivers/SCsub

@Faless
Copy link
Collaborator Author

Faless commented Nov 10, 2024

Oh hey, if/when ip_unix gets the same treatment, we should be able to completely exclude the unix/windows folders based on platform in drivers/SCsub

Yeah, I can work on that too, I think we would still not want to hard-code the platform names there, so probably the platforms detect.py should specify which drivers it wants (windows. unix, none).

drivers/windows/net_socket_winsock.h Outdated Show resolved Hide resolved
drivers/windows/net_socket_winsock.cpp Outdated Show resolved Hide resolved
drivers/windows/net_socket_winsock.cpp Outdated Show resolved Hide resolved
@Repiteo Repiteo merged commit 1cbe971 into godotengine:master Nov 12, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Nov 12, 2024

Thanks!

@Faless Faless deleted the net/split_sockets branch November 12, 2024 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants