1,961 Commits over 488 Days - 0.17cph!
Update: rename BasePlayer.playerCache member var -> BasePlayer.PlayerCache
It was hiding the source of a bug, as func params have lower case name
Tests: compiles in editor
Clean: prefer ReadOnlySpan<BasePlayer> instead of playerCache where possible
Makes it easier to guess things at a glance.
Tests: ran all relevant unit tests(discovered WaterLevelTests are failing, will investigate next) + 2p session on craggy with UsePlayerUpdateJobs 2
Update: demote PlayersToFinalize, PlayersToValidate, PlayersToRecache from being global caches
Tests: 2p session on Craggy with UsePlayerUpdateJobs 2
Update: demote PositionChanges from being a global cache
Tests: 2p session on Craggy with UsePlayerUpdateJobs 2
Merge: from occlusion_rework
- Fix for occlusion queries not being commutative
- Reenable occlusion frame cache and expand it's use to full frame
- Server occlusion deduplicates queries
- minor API restructure (might affect mods)
Optim: ServerUpdateParallel - update player subscription groups just before we run sever occlusion update
This ensures that server occlusion frame cache is built from up-to-date occluders, and allows to save on more occlusion checks downstream in ConnectedPlayersUpdate
Tests: 2p session on Craggy with UsePlayerUpdateJobs 2
Clean: split up FinalizeTickParallel monstrosity
This is end of prep to rearrange systems for better occlusion cache reuse
Tests: 2p session on craggy with UsePlayerUpdateJobs 1 and 2
Update: add native list expand to GatherPlayersToUpdate
No effect in current case, but can prevent surprise exceptions in the future if reused
Tests: none, trivial change
Clean: minor refactor of ServerUpdatePlayerTick
Getting ready to shuffle system steps around, to re-expand occlusion cache and potentially reduce network traffic
Tests: none, trivial changes
Bugfix: avoid stale occlusion results for new occludeeds
Didn't realize UpdateSubscriptions modifies occludees - this returns a bunch of overhead. Will try to fix next.
Tests: none, trivial change
Optim: deduplicate occlusion queries
Saves 15-50% in 1k baseplayer pairs parallel occlusion perf tests.
Tests: occlusion unit tests
Clean: ServerOcclusionJobs.Algorithm uses int3 instead of SubGrid
Simplifies the code a bit
Tests: ran unit tests
Clean: get rid of non-burst CalculatePathBetweenGrids
- Also removed obsolete tests
We've been using burst version for a bit, and with current fixes it balooned to too many lines of code
Tests: ran unit tests
Bugfix: apply same neighbour occlusion fix to jobs
Perf tests of 100k paths show degradation (serial +25%, parallel +52%), but perf tests for 10k baseplayer occlusion show negligeble impact (serial -9%, parallel +1%).
Tests: occlusion unit tests pass
Bugfix: reimplement occlusion neighbour logic in non-burst flow
- Added anothed optim todo now that we have axis-count-specific neighbour checks
This partially fixes the failing test (it still fails since now it trips up in burst version)
Tests: ran unit tests
Update: Re-enable sorted pair occlusiuon visibility caching
Tests: none, I know it's borked
Merge: from baseplayer_serverupdateparallel
Update: disable bidirectional occlusion caching for now
- Left a couple optim/safety todos
This fixes the batch vs serial baseplayer occlusion consistency test, but one CantSee fails as expected. Going to fix it in a child branch, as that currently blocks more optims.
Tests: ran relevant unit tests
Tests: TestLineOfSight - properly enforce non-job mode in relevant cases
Tests: ran test sets
Tests: TestLineOfSight - validate reverse paths
Looks like some occlusion checks might fail reverse paths. That's an uh-oh.
Tests: ran above set, got a failure
Tests: ServerOcclusionTests - generated data set now contains similar pairs
Interestingly, this trips up consistency test - will investigate
Tests: ran TestOcclusionLineOfSight_Consistency
Tests: PerfOcclusionLineOfSight - reset internal recent visibility cache between runs
Tests: ran the set
Tests: ServerOcclusionTests - standardize test naming
Tests: ran test set
Tests: ServerOcclusionTests.GeneratePairs - generate correct number of pairs
This further shrinks runtimes, as previously we generated waaaay too many
Tests: ran ServerOcclusionTests set
Tests: optim TestOcclusionLineOfSight_PerfSerial/-Parallel
By properly constructing and caching base players - tests now take less than a second.
Tests: ran test set
Tests: add TestOcclusionLineOfSight_PerfSerial/-Parallel
Both run for too long - likely due to how I create players for the tests. Will fix next
Tests: ran the new set
Tests: TestOcclusionLineOfSight_Consistency - rset occlusion cache between serial and parallel runs
Tests: ran the test set
Tests: add ServerOcclusionTests.TestOcclusionLineOfSight_Consistency
Compares results between serial and batched versions.
Tests: ran the new test set
Update: prep BasePlayer.OcclusionLineOfSight (both serial and batched) for use in tests
Tests: none, trivial change
Tests: rewrite ServerOcclusionTests to make ServerOcclusion usage clear
Is it ugly on some lines? Yes. But is it explicit and beautiful? Also yes.
Tests: ran the unit tests
Clean: propagate network time from FinalizeTickParallel
Tests: none, trivial change
Optim: NetworkPositionTick - remove extra InvalidateNetworkCache
Cache has been previously invalidated in FinalizeTickParallel, so no need to discard it again
Tests: none, trivial change
Optim: OcclusionSendUpdates can now reuse occlusion results
- Got rid of old OcclusionLineOfSight that used to send updates internally, as there's no need for it now
Tests: 2p session on Craggy with UsePlayerUpdateJobs 2
Clean: remove couple TODOs
- one was just completed
- another was overzealous
Tests: none, trivial changes
Optim: SendNetworkPositions - reuse occlusion query results
Tests: 2p on Craggy with UsePlayerUpdateJobs 2
Update: move ServerUpdateOcclusionParallel inside FinalizeTickParallel
- FinalizeTickParallel invalidates players network cache - with UsePlayerUpdateJobs 2 we can skip it later, but 1 is has double-invalidate
This increases the coverage of OcclusionFrameCache, allowing to simplify a bunch of code.
Tests: 2p on Craggy with UsePlayerUpdateJobs 0, 1, 2 and disconnects. 0p server with UsePlayerUpdateJobs 1, 2
Merge: from connectedplayer_rewrite
Got far enough along in this direction and things seem to work
Bugfix: ServerUpdatePlayerTick - restore Player.serverTickInterval functionality
Got lost during the rewrite
Tests: 2p on Craggy with UsePlayerUpdateJobs 2
Update: ServerOcclusion - add a global cache of all player pair results that lives for a frame
- Cache is valid after it's been updated, controlled via OcclusionCanUseFrameCache
Optimizes SendEntityUpdates and anything in the end of frame invokes by skipping LOS checks. This doesn't affect tick confirmation due to code ordering - will have to reorganize that
Tests: 2p on Craggy with UsePlayerUpdateJobs 2
Clean: use ReadOnlySpan in SendEntitySnapshots/-WithChildren
Tests: compiles in editor
Update: SendEntityUpdates - don't try to skip occlusion explicitly
Bit of a 180 turn. Current code is problematic to optimize at high level - but it will be easier if we introduce a global occlusion pair cache. At least I hope.
Tests: compiles in editor
Buildfix: TryRemove -> Remove
Tests: editor compiles
Update: bring back non-concurrent dict for BasePlayer.lastPlayerVisibility
Tests: none, trivial change
Update: OcclusionSendUpdates - rewrite lost pair handling to gather->send form
Should enable to revert back to non-concurrect dictionary for BasePlayer.lastPlayerVisibility
Tests: 2p session on craggy with UsePlayerUpdateJobs 0 and 2
Clean: OcclusionSendUpdates - factor out SendEntitySnapshotsWithChildren
Decided against merging SendEntitySnapshotsWithChildren and SendEntitySnapshots, as -WithChildren relies on grouped sending - it would complicate the code unnecessarily.
Tests: 2p session on Craggy with UsePlayerUpdateJobs 2
Clean: SendEntityUpdates - refactor out SendEntitySnapshots
Prepping for server occlusion unification
Tests: none, simple change
Update: SendEntityUpdates - add parallel sending of snapshots
It's incomplete, but now the similarities with OcclusionSendUpdates are more clear and can be unified. Also revelaed a bunch of concerns with ShouldNetworkTo and deferring to parent choice.
Tests: 2p on Craggy with UsePlayerUpdateJobs 2. Observed animal desync, but weirdly it was still there after setting UsePlayerUpdateJobs 0 and reconnecting - will investigate later.
Bugfix: OcclusionSendUpdates - fix up wrong last batch size calculation
- also removed dead variables
- removed FoundMain/Worker to SendAsSnapshotMain/Worker
Tests: none, trivial changes
Update: SendEntityUpdates - plug in occlusion fast path
- use-after-free bugfix
Tests: 2p on Craggy with UsePlayerUpdateJobs 2
Update: shaping up SendEntityUpdates, not complete
I'll need to refactor out sending logic from parallel server occlusion, and reuse some previous results, but at this point the direction is clear
Tests: compiles in editor
