Skip to content

Add support for spectating entities on Bedrock#6470

Open
darkcupid412 wants to merge 4 commits into
GeyserMC:feature/26.2from
darkcupid412:feature/spectate-entities-v2
Open

Add support for spectating entities on Bedrock#6470
darkcupid412 wants to merge 4 commits into
GeyserMC:feature/26.2from
darkcupid412:feature/spectate-entities-v2

Conversation

@darkcupid412

Copy link
Copy Markdown

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 -14 bump in gradle/libs.versions.toml). The server's ClientboundSetCameraPacket starts 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:

  • View cycling is bound to jump rather than F5 (Bedrock's perspective toggle is client-side and would conflict with camera instructions).
  • First-person eye height is approximated as 0.85x the entity height rather than using entity-specific eye heights.
  • The camera follows the entity's head yaw rather than body yaw for smoother tracking.

Tested on a 26.30 Bedrock client against Vanilla and PaperMC 26.2.

@Novampr Novampr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason this is here and not the tick method of the GeyserSession class?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 onebeastchris left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw this flag is currently only used for spiders' glowing eyes; does it actually make a difference for players?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +91 to +95
// 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;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't the block break handler already do game mode checks?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Comment on lines 476 to +484
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;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 onebeastchris left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's mark this nullable

* Spectate camera mode: 0 = first-person, 1 = third-person back, 2 = third-person front.
*/
@Setter
private int spectateMode;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

personally i'd rather not touch this flag, or override the method with just the spider entity class

Comment on lines +129 to +130
Vector3f pos = entity.bedrockPosition();
double eyeY = pos.getY() + eyeHeight(entity);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw - entity.bedrockPosition() will include the player offset, which could end up higher for e.g. player entities. Was this tested with players?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll take a proper look at it sometime.

Comment on lines +76 to +77
if (packet.getInputData().contains(PlayerAuthInputData.SNEAK_DOWN)
|| packet.getInputData().contains(PlayerAuthInputData.DESCEND)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this already handled by session.getInputCache().processInputs()? It would check for these flags in InputCache#isSneaking

Comment on lines +79 to +81
} else if (packet.getInputData().contains(PlayerAuthInputData.JUMP_PRESSED_RAW)) {
EntitySpectateHelper.cycleMode(session);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants