-
-
Notifications
You must be signed in to change notification settings - Fork 34
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
Make fields public for ServerListPingEvent #75
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #75 +/- ##
========================================
+ Coverage 7.29% 8.02% +0.73%
========================================
Files 55 62 +7
Lines 3071 4384 +1313
Branches 1349 1895 +546
========================================
+ Hits 224 352 +128
- Misses 2669 3776 +1107
- Partials 178 256 +78 ☔ View full report in Codecov by Sentry. |
This PR instead of making the needed fields public, adds methods to set the local port (ipv4 & pv6) as well as the guid |
Thank you for the pull request! These fields were intentionally kept read-only, as modifying these values doesn't affect their appearance in the server list. I am wondering about the practical usage of making these fields writable with these setters? |
Hey, the reason why local port should be writable as there is a chance for a port mismatch to happen when dealing with proxies. As when the proxy is on one port while the server is on another port the server MOTD ping will container the servers port and not the port of the proxy. This can confuse clients into believing the server does not exist in the server list. |
@Dingsel That makes sense to me, although I think the proxy should ideally handle the ping response from backend servers, rather than the backend servers managing it directly. But I assume the proxies aren't that advanced yet. @theaddonn One more thing before I merge this PR—could you also add the Python bindings for these setters? |
@wu-vincent The proxy are that advanced, but it introduces a lot more overhead.. which can easily be avoided by just changing the needed data on the backend server directly. |
c9c8559
to
817a558
Compare
* fix: fix material.h and material_type.h * refactor: rename canDropWithAnyTool to requiresCorrectToolForDrops * refactor: remove material related things since these things are unused * docs: add a FIXME notice to biome.h
…ile (EndstoneMC#110) * fix: fix EndstonePluginManager::loadPlugin EndstoneMC#98 * refactor: improve plugin loading logic in EndstonePluginManager * refactor: revert back to range-based loop for plugin loading --------- Co-authored-by: Vincent <[email protected]>
…patch-2 # Conflicts: # include/endstone/event/server/server_list_ping_event.h
This pr just makes some fields public for the
ServerListPingEvent
. This is helpful since you may want to modify stuff like the port or Minecraft Version shown.This PR is likely missing some stuff since this would also require to change some underlying python bindings, but eh..
Also, any reason we are using
.h
instead of.hpp
?