Skip to content

Client distinguish between eye and mob for drawing, context menu, and verbs#2477

Open
Ruzihm wants to merge 26 commits into
OpenDreamProject:masterfrom
Ruzihm:mob-sight
Open

Client distinguish between eye and mob for drawing, context menu, and verbs#2477
Ruzihm wants to merge 26 commits into
OpenDreamProject:masterfrom
Ruzihm:mob-sight

Conversation

@Ruzihm
Copy link
Copy Markdown
Contributor

@Ruzihm Ruzihm commented Dec 15, 2025

Adds a network message sent from the server to client to notify of mob/eye updates (currently, the client uses the attached entity which is ambiguous). I wasn't sure of a better way to accomplish this but am looking for feedback.

The client then uses this info to distinguish between eye/mob for context menu, drawing, and verbs.

Closes #1581
Closes #2081

Comment thread OpenDreamClient/Interface/DreamInterfaceManager.cs Fixed
Comment thread OpenDreamClient/Interface/DreamInterfaceManager.cs Fixed
Comment thread OpenDreamClient/Rendering/DreamViewOverlay.cs Fixed
Comment thread OpenDreamRuntime/DreamConnection.cs Fixed
Comment thread OpenDreamClient/Interface/DreamInterfaceManager.cs Fixed
Comment thread OpenDreamClient/Interface/DreamInterfaceManager.cs Fixed
Comment thread OpenDreamClient/Interface/DreamInterfaceManager.cs Outdated
@boring-cyborg boring-cyborg Bot added Client Involves the OpenDream client Runtime Involves the OpenDream server/runtime labels Dec 15, 2025
Comment thread OpenDreamShared/Network/Messages/MsgNotifyMobEyeUpdate.cs Outdated
Comment thread OpenDreamClient/Interface/DreamInterfaceManager.cs Outdated
Comment thread OpenDreamClient/Interface/DreamInterfaceManager.cs Outdated
Comment thread OpenDreamClient/DreamClientSystem.cs Fixed
Comment thread OpenDreamClient/DreamClientSystem.cs Fixed
Comment thread OpenDreamClient/DreamClientSystem.cs Fixed
Comment thread OpenDreamClient/DreamClientSystem.cs Fixed
Comment thread OpenDreamClient/DreamClientSystem.cs Fixed
Comment thread OpenDreamRuntime/DreamConnection.cs Fixed
Comment thread OpenDreamRuntime/Objects/Types/DreamObjectClient.cs Fixed
Comment thread OpenDreamRuntime/Objects/Types/DreamObjectClient.cs Fixed
Comment thread OpenDreamShared/Network/Messages/MsgNotifyMobEyeUpdate.cs Fixed
Comment thread OpenDreamRuntime/DreamConnection.cs Fixed
Comment thread OpenDreamClient/Interface/DreamInterfaceManager.cs
Comment thread OpenDreamRuntime/DreamConnection.cs Fixed
@Ruzihm Ruzihm requested a review from wixoaGit January 15, 2026 03:39
@Ruzihm
Copy link
Copy Markdown
Contributor Author

Ruzihm commented Jan 15, 2026

actually - one sec. z levels and mouse events are messed up when eye is a turf.

edit: should be good now

@Ruzihm Ruzihm marked this pull request as draft January 15, 2026 03:53
@Ruzihm Ruzihm marked this pull request as ready for review January 15, 2026 08:06
Comment thread OpenDreamClient/Rendering/DreamClientEye.cs Fixed
Comment thread OpenDreamClient/Rendering/DreamClientEye.cs Fixed
Comment thread OpenDreamClient/Rendering/DreamClientEye.cs Fixed
Comment thread OpenDreamRuntime/DreamConnection.cs Fixed
Comment thread OpenDreamRuntime/DreamConnection.cs Fixed
Comment thread OpenDreamClient/Rendering/DreamClientEye.cs Fixed
Comment thread OpenDreamClient/Rendering/DreamClientEye.cs Fixed
Comment thread OpenDreamClient/Rendering/DreamClientEye.cs Fixed
Comment thread OpenDreamClient/Rendering/DreamClientEye.cs Fixed
Comment thread OpenDreamShared/Network/Messages/MsgNotifyMobEyeUpdate.cs Fixed
@Ruzihm
Copy link
Copy Markdown
Contributor Author

Ruzihm commented Jan 16, 2026

Setting eye to null will stop rendering - but since that's done by setting the eye position to be in nullspace, the viewport stops clearing the render target and you just get a frozen screen:

https://github.com/space-wizards/RobustToolbox/blob/aa5cca4c7f210f3ed7b7841735b5aadb533c9d88/Robust.Client/Graphics/Clyde/Clyde.HLR.cs#L494-L497

Does this warrant a pull request to RT to add a viewport option to allow target clearing if there is no eye/the eye is in null space?

@github-actions
Copy link
Copy Markdown

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions
Copy link
Copy Markdown

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 7, 2026

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@Ruzihm
Copy link
Copy Markdown
Contributor Author

Ruzihm commented May 8, 2026

I tried my best to merge the refcounting change to Connection.Eye. I noticed that there was a redundant ref management outside of the Eye set at DreamObjectClient.SetVar so I removed it since it seems to be a duplicate of what happens inside the Eye set.

Comment thread OpenDreamRuntime/DreamConnection.cs Fixed
Comment thread OpenDreamRuntime/DreamConnection.cs Fixed
Comment thread OpenDreamRuntime/DreamConnection.cs Fixed
Comment thread OpenDreamRuntime/DreamConnection.cs Fixed
@Ruzihm
Copy link
Copy Markdown
Contributor Author

Ruzihm commented May 8, 2026

@wixoaGit I think this PR is good for another look when you get a moment

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 9, 2026

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Comment thread OpenDreamClient/ClientVerbSystem.cs Fixed
Comment thread OpenDreamClient/Interface/DreamInterfaceManager.cs
Comment on lines +21 to +33
private EntityUid _mobUid = EntityUid.Invalid;

// Sometimes we get mob info before we know all the net entities, so store the net entity and refer to it
public EntityUid MobUid {
get {
// if entity of mob is invalid but net entity isn't, try referring to known net entities
if (! _mobUid.IsValid() && _mobNet.IsValid() && _entityManager.TryGetEntity(_mobNet, out var ent)) {
_mobUid = ent.GetValueOrDefault(EntityUid.Invalid);
}

return _mobUid;
}
}
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.

This is a lot of effort to avoid the single Dictionary lookup that _entitySystem.GetEntity() does. The same NetEntity also isn't guaranteed to always refer to the same EntityUid on the client due to PVS shenanigans, so the cached value could break.

Suggested change
private EntityUid _mobUid = EntityUid.Invalid;
// Sometimes we get mob info before we know all the net entities, so store the net entity and refer to it
public EntityUid MobUid {
get {
// if entity of mob is invalid but net entity isn't, try referring to known net entities
if (! _mobUid.IsValid() && _mobNet.IsValid() && _entityManager.TryGetEntity(_mobNet, out var ent)) {
_mobUid = ent.GetValueOrDefault(EntityUid.Invalid);
}
return _mobUid;
}
}
public EntityUid MobUid => _entityManager.GetEntity(_mobNet);

Comment on lines +73 to +79
var prevMobNet = _mobNet;
_mobNet = msg.MobNetEntity;

if (prevMobNet != _mobNet) {
// mark mob cache as dirty/invalid
_mobUid = EntityUid.Invalid;
}
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.

Suggested change
var prevMobNet = _mobNet;
_mobNet = msg.MobNetEntity;
if (prevMobNet != _mobNet) {
// mark mob cache as dirty/invalid
_mobUid = EntityUid.Invalid;
}
_mobNet = msg.MobNetEntity;

}

[ViewVariables] public DreamObjectMovable? Eye {
[ViewVariables] public ClientObjectReference? Eye {
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.

ClientObjectReference is intended to only be used on the client or in network communication with the client. Using it like this adds an annoying amount of indirection. This should be a DreamObjectAtom? and DreamManager.GetClientReference() should be used when creating the MsgNotifyMobEyeUpdate.

Comment on lines +16 to +17
// Type = Client -> null
public ClientObjectReference EyeRef;
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.

Would ClientObjectReference? work better here than treating Client as null?

Comment on lines +22 to +37
RefType eyeType = (RefType)buffer.ReadInt32();
switch (eyeType) {
case RefType.Entity:
EyeRef = new(new NetEntity(buffer.ReadInt32()));
break;
case RefType.Turf:
int x = buffer.ReadInt32();
int y = buffer.ReadInt32();
int z = buffer.ReadInt32();
EyeRef = new(new(x, y), z);
break;
default:
// null eye
EyeRef = new();
break;
}
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 don't like having the process for DreamClientReference serialization in here. I'd rather this either be a helper method on DreamClientReference, use the serializer this method is given, or have this net message converted to an entity system event.

/// Draws this plane's mouse map onto the current render target
/// </summary>
public void DrawMouseMap(DrawingHandleWorld handle, DreamViewOverlay overlay, Vector2i renderTargetSize, Box2 worldAABB) {
public void DrawMouseMap(DrawingHandleWorld handle, DreamViewOverlay overlay, Vector2i renderTargetSize, Box2 worldAABB, bool onlyDrawScreenSprites = 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.

onlyDrawScreenSprites is never used anywhere.

Comment on lines +164 to +181
switch (eyeRef.Type) {
default:
_mobSightQuery.TryGetComponent(mob, out mobSight);
seeVis = mobSight?.SeeInvisibility ?? 127;
DrawNullEyeSprites(args, viewportSize, seeVis);
return;
case ClientObjectReference.RefType.Turf:
eyeCoords = new(new(eyeRef.TurfX, eyeRef.TurfY), new(eyeRef.TurfZ));
_mobSightQuery.TryGetComponent(mob, out eyeSight);
break;
case ClientObjectReference.RefType.Entity:
var eyeUid = _entityManager.GetEntity(eyeRef.Entity);
if (!_xformQuery.TryGetComponent(eyeUid, out var eyeTransform))
return;
eyeCoords = _transformSystem.GetMapCoordinates(eyeUid, eyeTransform);
_mobSightQuery.TryGetComponent(eyeUid, out eyeSight);
break;
}
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.

Could this be turned into IsNullEye, EyeCoords, and Sight helper properties on DreamClientEye? It should be reachable from Draw()'s args.Viewport.Eye.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Client Involves the OpenDream client Runtime Involves the OpenDream server/runtime size/XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

client.eye can't be set to a turf Hopping into a locker lets you see ghosts

4 participants