Add support for spectating entities on Bedrock#6470
Conversation
Novampr
left a comment
There was a problem hiding this comment.
Just this, but this is seriously some impressive work.
| } else if (packet.getInputData().contains(PlayerAuthInputData.JUMP_PRESSED_RAW)) { | ||
| EntitySpectateHelper.cycleMode(session); | ||
| } else { | ||
| EntitySpectateHelper.tick(session); |
There was a problem hiding this comment.
Any reason this is here and not the tick method of the GeyserSession class?
There was a problem hiding this comment.
No real reason. The sneak/jump handling was already in the translator, so I dropped the follow handling into the same block. I've moved it to GeyserSession.
onebeastchris
left a comment
There was a problem hiding this comment.
Hey! Thanks for the PR, this looks super promising 👀
Left a few initial comments; if you've got any questions just ask :)
| private EnumMap<EntityFlag, Boolean> spectateHiddenFlags() { | ||
| EnumMap<EntityFlag, Boolean> overridden = new EnumMap<>(flags); | ||
| overridden.put(EntityFlag.INVISIBLE, true); | ||
| overridden.put(EntityFlag.HIDDEN_WHEN_INVISIBLE, true); |
There was a problem hiding this comment.
fwiw this flag is currently only used for spiders' glowing eyes; does it actually make a difference for players?
There was a problem hiding this comment.
Oops, I actually mistook it for RENDER_WHEN_INVISIBLE and yeah, they do kinda overlay on the screen in first person, so it seems necessary even if only 2 entities set it.
| // Spectating: suppress block-break/use/attack, but only after processInputs so the stop-shift still flows. | ||
| // Also leaves InputCache's position-reminder un-ticked while spectating (see its note) | ||
| if (EntitySpectateHelper.isSpectating(session)) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Doesn't the block break handler already do game mode checks?
There was a problem hiding this comment.
It's there to skip BedrockMovePlayer while spectating, which forwards player movement (the body can drift off without it, though it's intermittent) and ticks the position reminder that InputCache#shouldSendPositionReminder says shouldn't be ticked while spectating.
Fixed my comment; the block-break mention was misleading.
…invisibility flag - Run the camera follow from GeyserSession#tick() instead of the auth-input translator - Use RENDER_WHEN_INVISIBLE instead of the redundant HIDDEN_WHEN_INVISIBLE so the first-person hide also covers overlays like spider eyes - Fix the input-suppression comment to point at InputCache's position-reminder note
| Entity entity = session.getEntityCache().getEntityByGeyserId(packet.getRuntimeEntityId()); | ||
| if (entity == null) | ||
| return; | ||
|
|
||
| if (session.getGameMode() == GameMode.SPECTATOR) { | ||
| session.sendDownstreamGamePacket(new ServerboundSpectatorActionPacket(OptionalInt.of(entity.getEntityId()))); | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
We should send a ServerboundSpectatorActionPacket with an empty optional when no entity was found (in spectator mode) - see https://mcsrc.dev/1/26.2/net/minecraft/client/Minecraft#L1652-1669, which calls https://mcsrc.dev/1/26.2/net/minecraft/client/multiplayer/MultiPlayerGameMode#L432
onebeastchris
left a comment
There was a problem hiding this comment.
A few more notes after a more thorough review - thanks again for the great PR!
A side-note -
also needs to check whether this living entity is the currently spectated entity (https://mcsrc.dev/1/26.2/net/minecraft/client/renderer/entity/LivingEntityRenderer#L229); probably just with adding|| this == session.getSpectatedEntity to the condition.
Another note - is this preventing the Bedrock client from moving on its own? Might be worth sending an input lock to prevent the player moving.
Sorry for the many notes! If you have any questions, just ask & i'd be happy to help.
| private TeleportCache unconfirmedTeleport; | ||
|
|
||
| @Setter | ||
| private Entity spectatedEntity; |
| * Spectate camera mode: 0 = first-person, 1 = third-person back, 2 = third-person front. | ||
| */ | ||
| @Setter | ||
| private int spectateMode; |
There was a problem hiding this comment.
can we make this an enum instead? nicer to look at & with switch statements it'd also be easier to ensure no case is being missed
| private EnumMap<EntityFlag, Boolean> spectateHiddenFlags() { | ||
| EnumMap<EntityFlag, Boolean> overridden = new EnumMap<>(flags); | ||
| overridden.put(EntityFlag.INVISIBLE, true); | ||
| overridden.put(EntityFlag.RENDER_WHEN_INVISIBLE, false); |
There was a problem hiding this comment.
personally i'd rather not touch this flag, or override the method with just the spider entity class
| Vector3f pos = entity.bedrockPosition(); | ||
| double eyeY = pos.getY() + eyeHeight(entity); |
There was a problem hiding this comment.
fwiw - entity.bedrockPosition() will include the player offset, which could end up higher for e.g. player entities. Was this tested with players?
There was a problem hiding this comment.
Actually, no, I didn't test it with players.
I suppose using the raw position() instead of bedrockPosition should fix it?
| rot = Vector2f.from(-pitch, yaw + 180f); | ||
| } | ||
|
|
||
| CameraInstructionPacket packet = new CameraInstructionPacket(); |
There was a problem hiding this comment.
I've had another look at what's theoretically possible with cameras already - a CameraSetInstruction includes a viewOffset and entityOffset field, and the CameraInstructionPacket includes a CameraAttachToEntityInstruction which allows attaching to an entity... might be worth exploring; although i'm also perfectly fine with doing that at a later point as that'd be a more substantial change
There was a problem hiding this comment.
I'll take a proper look at it sometime.
| if (packet.getInputData().contains(PlayerAuthInputData.SNEAK_DOWN) | ||
| || packet.getInputData().contains(PlayerAuthInputData.DESCEND)) { |
There was a problem hiding this comment.
Isn't this already handled by session.getInputCache().processInputs()? It would check for these flags in InputCache#isSneaking
| } else if (packet.getInputData().contains(PlayerAuthInputData.JUMP_PRESSED_RAW)) { | ||
| EntitySpectateHelper.cycleMode(session); | ||
| } |
There was a problem hiding this comment.
Personally i'd rather move this to InputCache too
|
|
||
| // This is the best way send this since most modern anticheat will expect this to be in sync with the player movement packet. | ||
| if (session.isSpawned()) { | ||
| session.sendDownstreamGamePacket(ServerboundClientTickEndPacket.INSTANCE); |
There was a problem hiding this comment.
We should still send this tick end packet in spectator mode
- Mark spectatedEntity @nullable and make the spectate mode an enum - Hide RENDER_WHEN_INVISIBLE only on the entities that set it (spider, shulker) - Move the jump view-cycle into InputCache and drop the redundant sneak handling - Keep sending the client tick-end while spectating - Suppress the spectated entity's nametag - Send an empty spectator action on a no-entity left-click, matching the vanilla client - Clear the camera on stop so the F5 perspective toggle is restored
This PR implements entity spectating support for Bedrock clients.
While in spectator mode, left-clicking an entity now attaches the camera to it, mirroring Java's click-to-spectate behavior. Jump cycles between first-person, third-person back, and third-person front views, sneak returns the camera to the player, and the spectated entity is hidden while in first-person.
The Bedrock interaction is translated into a
ServerboundSpectatorActionPacket(requires the serialization fix from GeyserMC/MCProtocolLib#927, included here via the-14bump ingradle/libs.versions.toml). The server'sClientboundSetCameraPacketstarts and stops spectating, and while active, a FREE preset camera is repositioned and rotated each client tick with linear easing to follow the spectated entity.Diverges from Java in a few spots:
0.85xthe entity height rather than using entity-specific eye heights.Tested on a 26.30 Bedrock client against Vanilla and PaperMC 26.2.