Server-Authoritative Immersives
hammy275 opened this issue ยท 10 comments
Depends on #351, since I'm not doing this before that refactor.
Right now, the way immersives are run is pretty atrocious, with the client effectively asking the server non-stop for updates on inventory contents and being the main decider on whether to show an immersive or not, even if inventory contents aren't ready yet or if no changes to the inventory have happened! Instead, the server should be authority on this!
The detection to start interacting with an immersive should be done server-side. To start, this can just be the system we have now, though it could easily be expanded on later, if not when this is implemented, just running it on the server instead. From there, the server sends an initial FetchInventoryPacket
to the client, which the client uses to initialize an immersive and process the storage from there. The server also tracks which players have been sent this.
FetchInventoryPacket
's C->S will be killed off. Instead, the server uses the list of players it has active for a given immersive to send storage data to the client.
Storage data is sent to the client under any of the following circumstances:
- A new player is registered for the immersive. Data is only sent to this new player.
- The contents of the inventory have changed.
When a client should no longer track an immersive, the server sends a StopTrackingImmersive
packet to the client, which causes the client to remove that immersive.
For immersives that make much more sense to be client-initiated (the ImmersiveBackpack
for the time being), the client sends the server a ImmersiveTrackingAlert
packet, so the server knows the client has that immersive active. The client will also send the server the same packet when it's done.
For immersives that only need to be run client-side (just ImmersiveHitboxes
right now), the client will add and remove infos as it does for all immersives now.
Note that storage data is sent when inventory contents change. Currently, ImmersiveMC has storage that either originates from an in-world container, or from ImmersiveMC's world storage. For in-world Container
s, we don't entirely know how their contents will change nor how often, so every tick, we check if inventory contents have changed from before. If they have, we send a change in storage*. For ImmersiveMC's world storages, we allow adding callbacks to storages for when they're marked dirty (and, of course, have the storages handle marking the actual world data dirty instead of letting other code do so). Once per tick, if it was marked dirty during that tick, we send storage updates to everyone.
Additionally, with this change, we'll remove the limit of the number of active immersives. The code for limits will still be in place for things like ImmersiveHitboxes
, or anything else that we may need limits for in the future, but all immersives that don't still need a limit will have the max set to -1 to be unlimited.
So guide boxes don't get shown everywhere, we only show guide boxes for immersives either being actively looked at or for ones that are within a few blocks of the player.
*: The best way I've thought of so far to detect these changes is to Mixin to when the block is marked as dirty, and if it's also a BlockEntity
of some kind (alongside being a Container
), use that as our cue to send a packet.
Much progress has been made on the branch for this issue, however several things still remain
- Unmark block entities marked as dirty if no player is tracking it. Especially important for players who disconnect. Since changes are only sent each tick, it's also extremely reasonable to simply clear dirtiness marks at the end of each tick after the values are used.
- [ ] If we move to simply sending data for all visible immersives nearby, add some hard limit to prevent too much data going across (don't need a large item sorter causing massive amounts of network traffic). - If we don't move to sending data for all visible immersives nearby, we should likely expire immersives after a certain amount of time again.
- Respect config values. Note that this also means we need to clean up
TrackedImmersiveData
if the enablement becomes false due to the client changing its settings. - Only render item guides when nearby
I want to do all immersives nearby, however that has some pretty brutal performance implications if not implemented properly. As such, there's a few things I'll do to optimize:
- On first log-in or on level change, we send immersive data for all immersives nearby and store the player's
BlockPos
, When the currentBlockPos
changes from the stored one, we can pretty easily use some math to determine the new area that should now be tracked, then update the storedBlockPos
to the new one. Note that this isn't necessarily a 16x16 area, as the player may be moving faster than 1 block/tick. - We'll place a hard limit on the amount of immersive data we sync with the client per-tick. This is mainly because large machines using blocks that ImmersiveMC supports (such as hoppers) may be use en-masse and change very often.
- This means client-updates themselves will need to be queued so we can make sure we don't send more than the limit number of updates over the network.
- The updates will use a priority queue, which factors in the following:
- Whether the block is being looked at by the player getting the update. Should be weighted maximally, since we always want to propagate changes the player makes.
- Whether the block is being looked at by another player. Should be weighted relatively highly, as it's likely to change. The closer to the player, the higher the rating.
- The rest should be weighted by distance.
- I will not be only sending the diffs between old contents and new contents, at least for now, as that requires storing the old contents server-side. With things like Lootr integration, this would also have to be done per-player, which is a lot.
- For servers with bandwidth constraints or who just otherwise don't want to deal with this, a limited bandwidth option will be made that only sends updates for the immersive being looked at by the player. When enabled on the client will have the server treat the client in that way.
Note that "looked at" refers to being pointed at and/or being looked at when in VR
Can't do blocks nearby, not without far too much engineering effort. Network traffic was actually least of the issues, getting all the blocks nearby proved to be insanely expensive, with even checking 6 blocks nearby (not enough to make it worthwhile) taking 2ms on average, and that's on a i7-12700K. Would be far much worse for lower-power devices.
Not expiring immersives, at least for now, since we already stop tracking when the player isn't close anymore.
Despite the commit, chests are still broken, namely that if you break a double chest to a single chest, the client clears the immersive but the server still thinks it's being tracked.
Have tried a lot of hacks, but they just lead to other buggy results. Might use a C->S packet here to signal the removal.
Chests work mostly-enough now, only having breakages when a chest is broken. I think this is more down to the open/close marking that we do.
Calling it for now, and will hopefully get around to properly supporting multiblock immersives in the future, instead of it being hacked together like it is with chests right now.
This is done! No longer shall ImmersiveMC send way too many packets to do what it needs to do.
ImmersiveHandler
To make room for an eventual API and just to keep all of the storage retrieval for all immersives from being forced to be written in one file, we're also going to refactor all the server-side parts of immersive handling. A new interface, ImmersiveHandler
will exist, with these required methods:
HandlerStorage writeInventoryContents(ServerPlayer player, BlockPos pos)
: Given the player and block position, returns a HandlerStorage
containing all data that needs to be transmitted to the client (including block position!).
void swap(int slot, InteractionHand hand, BlockPos pos, ServerPlayer player, PlacementMode mode)
: Handles swapping the item between the player and the immersive they're interacting with. Will all be pre-existing code moved out of Swap.java
.
boolean usesWorldStorage()
: Whether this immersive uses world storage or not, since that info is needed.
boolean isValidBlock(BlockPos blockPos, BlockState blockState, BlockEntity blockEntity, Level level)
: Whether a given block matches with this immersive. This will replace ImmersiveCheckers
and CheckerFunction
, as everything in there should just use this function on these objects instead.
boolean enabledInServerConfig()
: Whether this immersive is enabled in the config using server-side logic (check ActiveConfig.FILE
).
As one would imagine, the above replaces Swap
, ImmersiveCheckers
and CheckerFunction
.
HandlerStorage
Additionally, the interface HandlerStorage
will also be made, containing the following:
void encode(FriendlyByteBuf buffer)
: Writes the contents of this storage into a buffer.
void decode(FriendlyByteBuf buffer)
: Reads the contents of a buffer into this storage.
ResourceLocation getId()
: A unique ID that identifies a given HandlerStorage
implementation. This is used so when decoding, the correct decoding implementation is used.
Other Changes
AbstractImmersive
will have itsprocessStorageFromNetwork
function replaced to take aHandlerStorage
instead. This should be the only way data makes it to a client immersive now!
All of these objects will be kept in common/immersive/
.
The above has been spun off into #351