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

Print warning when disabling SIO_UDP_CONNRESET fails. #124

Merged
merged 3 commits into from
Sep 10, 2024

Conversation

dbechrd
Copy link
Contributor

@dbechrd dbechrd commented Sep 7, 2024

It's tagged as "IMPORTANT" in the comment, so perhaps add a message when it fails? I don't think this particular setting failing to be set warrants destroying the socket, though, so I opted not to do that.

@gafferongames
Copy link
Contributor

Please make this an error and destroy the socket. In fact, without this setting working successfully -- the windows build is quite broken. So it's OK to make it a fatal error. No need for "warning:". ps. if in future you do find actual warning conditions, you can just use NETCODE_LOG_LEVEL_WARN

@dbechrd
Copy link
Contributor Author

dbechrd commented Sep 9, 2024

Updated level to error and will now the destroy socket on failure.

@gafferongames
Copy link
Contributor

There's no need to add "error: " to the message -- when you use error log level, it prepends it to your text automatically

@dbechrd
Copy link
Contributor Author

dbechrd commented Sep 9, 2024

There's no need to add "error: " to the message -- when you use error log level, it prepends it to your text automatically

Are you sure? I don't see any logic in netcode_printf that prints the error level. I was just following the pattern that all of the other error messages use. That would be a logical improvement to make to the default logger implementation though.

@gafferongames
Copy link
Contributor

Sorry, I'm getting netcode mixed up with my newer projects (network next etc...) that do the prefixing. Apologies.

@gafferongames gafferongames merged commit efd0c6d into mas-bandwidth:main Sep 10, 2024
1 of 6 checks passed
@dbechrd dbechrd deleted the patch-1 branch September 19, 2024 00:46
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.

2 participants