Implement shared ticket level per-player view distance API

[SlickJava]

This PR implements the per-player view distance API at the ticket level.

This solution comes from the result of many complicated ones (and research).

The normal view distance value will determine how far the player can add tickets to chunks.
The tracking view distance value will determine how far the player can track/see/load chunks.
Other actions such as mob spawning will naturally fallback onto each player's normal view-distance.
This allows shared view distance from other players without impacting performance.

This solution leaves the main ticket queue/throttler completely alone, meaning all other ticket-related events are 100% unaffected. It operates through a coordinate compression algorithm in order to calculate and properly update squares, attaching those update actions respectively to the default update queue. Details are inside the patch.

Also included is support for a player area map, which is simultaneously calculated from the algorithm mentioned earlier (very efficient indeed), this map may be used for some significant optimizations.

edit: This patch is now largely complete - all there is left to do is to review, thoroughly test and pinpoint any edge cases. jar is below if you would like to help out :))

• Base logic
  • Implement correct & efficient logic with coordinate compression & minimal cells.
    • Solve overlapping (or any weird graphed case)
    • Solve multi-queueing (only adds/removes on non-overlapping vd/oldvd cells respectively)
• Add chunk tracking view distance for shared vd per player
  • Handle higher-level view distance management
    • Packet management
    • Trackers & special entities
    • Mob distance map management
    • PCM chunk tracking & untracking
• Internal Testing
  • Test integration in existing plugins
  • Test realtime view distance changes
    • Ticket addition on changes (implemented efficient logic)
    • Ticket removal and unloading
  • Test view distance packet handling affecting client caching
  • Test mob maps & spawning
  • Test general ticket purging (disconnect/world changes/leaves)
  • Trial on 15-30 player server
• External Testing
  • ...

build ca3d2b1 for pl dev testers (from 209):
SHA1: FCD10004F63E233A242229D71BD07A1135C46A36
https://cdn.discordapp.com/attachments/495168945968513036/631038203180089374/paperclip.jar

[kickash32]

You really need to use a lot of OFBHELPERs and explain all the changes. It needs to be in a state where its the changes are understandable without resorting to different mappings.

[SlickJava]

You really need to use a lot of OFBHELPERs and explain all the changes. It needs to be in a state where its the changes are understandable without resorting to different mappings.

Since the implementation utilized one of the cleaner methods, I felt that OBFHELPERs would add unnecessary weight. I'll have another look at it.

[mikroskeem]

OBFHELPERs help core maintainers to port patches to new major release, so they are definitely needed. We can't predict what Mojang is going to do with codebase exactly.

[SlickJava]

OBFHELPERs help core maintainers to port patches to new major release, so they are definitely needed. We can't predict what Mojang is going to do with codebase exactly.

currently adding

[HexedHero]

Good job on this so far Slick, really cool for this to be back in.

[Spottedleaf]

this pr also lacks the ability to send chunks to players correctly

[SlickJava]

this pr also lacks the ability to send chunks to players correctly

edit: all solved and committed

[SlickJava]

currently in testing phase on ~30 player server alternating between different plugins. external testing would be much appreciated.

[Spottedleaf]

chunk sending is done through

1. player movement
2. chunk load
3. PlayerChunkMap#setViewDistance
4. player joining the world

If you inspect these areas of code you will find that the chunk map's view distance is used when determining whether to send the chunk
you have not touched this code and as such do not correctly change behavior depending on player view distance

[SlickJava]

chunk sending is done through

1. player movement
2. chunk load
3. PlayerChunkMap#setViewDistance
4. player joining the world

If you inspect these areas of code you will find that the chunk map's view distance is used when determining whether to send the chunk
you have not touched this code and as such do not correctly change behavior depending on player view distance

all of these things have been accounted for. I have thoroughly reversed the structure of how player tickets affect loading chunks and vice versa over the past week. the #setViewDistance calls specific functions in ChunkMapDistance that relate directly to these changes. these changes are at the center of player movement. player tickets directly count to chunk loading, and overwrite the view distance send chunk loops in playerchunkmap. player movement and player joining is all central to these subclasses in chunkmapdistance.

it is not necessary to touch this code as player tickets essentialy overwrite it. this is exactly akin to how spectatorsGenerateChunks works. it does not change any aspect of view distance or anything else at all, instead it affects player tickets only.

I suggest you test these changes yourself

edit: all of these things noted have been commited and solved

[Spottedleaf]

player tickets themselves do not send chunks to players

until you fix that issue this pr will not be accepted (amongst other issues I have pointed out)

[mibby]

@SlickJava Testing your patch (compiled as of your branch/commit SlickJava@b7609f8), you seem to break fake players (NPCs) from being visible.

However they still seem to exist since I have a test area with npcs that fight each other. Damage hit values still appear, they are just not visible and can't be interacted with.

Using Citizens2 dev 1762.

Downgrading back to official paper 1.14 dev 210, npcs are visible again.

[Spottedleaf]

It should be moved regardless, the view distance pr touches view distance - not optimization of a range check. It should also be in its own patch for that reason: it's different.

It uses the same utilities as the view distance pr, but that's actually a case to get the utils moved outside this patch and into mc-util

[Hyronymos-Old]

We are using the latest Citizens Dev Build and we don’t have the problem on our SkyBlock Server (on peaks 70 users) and our CityBuild Server (on peaks 30 users).

We don’t had a single player that reported any view distance problems/bugs in the last 2 days.

build ca3d2b1

[SlickJava]

@mibby @Hyronymos1337 do any of you happen to have EntityTrackerFixer installed?

[Hyronymos-Old]

Yes, newest version

[mibby]

@SlickJava I do not have EntityTrackerFixer installed, no. Citizens being invisible occurs on both my production and test server -- both have different sets of plugins but my test server has significantly less.

-snip-

Unless it breaks with ProtocolLib or LibsDisguises installed, the only thing really in common is the patch, which downgrading to official paper fixes.

[SlickJava]

@mibby this is useful, thanks. The issue may have come from some of the later builds.

[SlickJava]

The patch is undergoing a rewrite anyways, so once I push the latest build (in which I may have addressed your issue) let me know if it still persists

[mibby]

Sure, I'll keep an eye on the PR and rebuild your fork for retesting if there are newer commits not in your released test build. I can only assume it's because of a newer commit if Hyronymos1337 didn't build your fork.

[Hyronymos-Old]

I‘m 6 commits behind

[SlickJava]

rewrite hasn't been pushed yet, i've addressed all of these issues, just have exams coming up

[Spottedleaf]

#2802 contains a patch that will override this pr - if you still want the shared ticket api part you must pr changes after it's merged

#2802 has a well tested per player view distance implementation so I presume it overrides

[SlickJava]

#2802 contains a patch that will override this pr - if you still want the shared ticket api part you must pr changes after it's merged

#2802 has a well tested per player view distance implementation so I presume it overrides

o wow nice!, yeah go ahead, irl stuff got in the way (need to focus on informatics olympiad camp!)
was fun attempting the patch along the way (learnt a substantial amount referring to you and online material) - thanks a lot :))))