Multiplayer player count from 8 -> actual max byte increase + small connection optimizations#722
Conversation
Made-with: Cursor
|
by any chance can you make it we can change with a server start parameter or server.properties |
| @@ -3,7 +3,7 @@ using namespace std; | |||
|
|
|||
| // If we have more than MAX_PLAYER_DATA_SAVES player.dat's then we delete the oldest ones | |||
| // This value can be no higher than MAXIMUM_MAP_SAVE_DATA/3 (3 being the number of dimensions in future versions) | |||
There was a problem hiding this comment.
Are you considering this in your PR?
There was a problem hiding this comment.
I believe MAXIMUM_MAP_SAVE_DATA relates to map items generated in a save. I've looked into it and there is a weird limit on it as the value is stored as a short in the item's NBT as damage (I believe). -- Java uses ints in NBT which allows WAY larger limits.
The code I changed just allows infinite player data to be stored without wiping it out from the save. I don't think maps are really needing the fix in this PR specifically. It'll likely be something reviewed later on in another PR to extend map generation beyond the 8192 limit for large worlds.
In this version, maps are super weird. Their ids vary based on _LARGE_WORLDS being on or off and there's a whole bunch of quirks to it. (I went down a rabbit hole and that's largely why it's taken me a while to respond.)
I think this would be more important for someone working on infinite worlds to address how maps generate and are stored / another map specific PR. I might look into it tomorrow to propose a better system. (Propose as it would take a decently large refactor and I'm not sure if people are interested in dealing with that / breaking compatibility with default/other clients.)
The benefit of having more and more players versus the potential risk of 8192 unique maps being generated and causing issues. I think it's better to just move forward with something like this and address maps when we need it.
|
Worth note that we're having some major changes to how servers work once #498 is merged. This might be related to that PR |
…ication fix, added to server.properties.
|
Added in the server.properties setting for max-players and made some optimizations to chunks. Let me know if I need to address maps more, but that'll be a pretty major refactor. |
|
Fixes like these should go to the main MP repo first https://github.com/LCEMP/LCEMP/pulls and then merged here, for the sake of having the same implementations and not having any issues in the future |
|
@NOTPIES We're not strictly building on top of your MP at this point unfortunately, but feel free to pull in PRs we receive if you like |
|
@lag Your PR says no AI was used but one of your commits says you used Cursor, what's up with that? |
|
@codeHusky I used Cursor to do the git commands because I made a mess. I think it auto appends the "Made with Cursor" on git commands like that. I use the IDE to search the codebase since the semantic search is amazing. (I've used it on previous PRs I've submitted and been clear on it.) AI didn't write the code, just did the git commands. |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
|
It's likely this code was written fully by AI per a number of things people have pointed out after the fact. If anyone wants to clean up this code and make it "not AI" they're welcome to do so in another PR. |
Description
This extends player count from 8 to actually fill the max byte value. Byte is hard limit for now since many player packets rely on the small id (byte) and converting to a better system like uint would take major refactoring.
Changes
Previous Behavior
Player count was maxed at 8 and had hardcoded colors for all players.
Root Cause
Was intended design.
New Behavior
Players can join exceeding 8 players.
Fix Implementation
Minimal edits were needed, but there were some small things like the player color that had to be redone so that we wouldn't read out of bounds in an array.
AI Use Disclosure
No AI used for this PR.
Video: https://streamable.com/xh6gd1