Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make crew monitor update blips at consistent rates #32555

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

eoineoineoin
Copy link
Contributor

About the PR

Made crew monitor update off-grid entities at same rate as on-grid ones. Fixes #30338.

Why / Balance

UI was inconsistently being updated for monitored crew on different grids.

Technical details

Great big comment in the change. Map blips were using EntityCoordinates, so would see changes immediately when the transform of the parent entity changed. Just converts all the blips to use the same, fixed coordinate system.

Media

Peek.2024-09-30.20-42.mp4

Requirements

Breaking changes

Changelog

@github-actions github-actions bot added S: Needs Review Status: Requires additional reviews before being fully accepted Changes: UI Changes: Might require knowledge of UI design or code. labels Sep 30, 2024
@SlamBamActionman SlamBamActionman added the S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. label Nov 14, 2024
@beck-thompson beck-thompson added T: Bugfix Type: Bugs and/or bugfixes P2: Raised Priority: Item has a raised priority, indicating it might get increased maintainer attention. D2: Medium Difficulty: A good amount of codebase knowledge required. A: General Interactions Area: General in-game interactions that don't relate to another area. A: Medical Area: Medical department, including Chemistry size/S Denotes a PR that changes 10-99 lines. and removed S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. labels Nov 21, 2024
Copy link
Contributor

@chromiumboy chromiumboy left a comment

Choose a reason for hiding this comment

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

Looks good, just a few things from me

Comment on lines 439 to 440
else
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
else
{

var worldToMonitor = _transformSystem.GetInvWorldMatrix((EntityUid)NavMap.MapUid);
var refInMonitor = Vector2.Transform(refPosition, worldToMonitor);
return new EntityCoordinates((EntityUid)NavMap.MapUid, refInMonitor);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
}

@@ -290,7 +291,7 @@ private void PopulateDepartmentList(IEnumerable<SuitSensorStatus> departmentSens
{
NavMap.TrackedEntities.TryAdd(sensor.SuitSensorUid,
new NavMapBlip
(coordinates.Value,
(CoordinatesToLocal(coordinates!.Value),
Copy link
Contributor

Choose a reason for hiding this comment

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

It's best to avoid null-suppression if possible

@chromiumboy chromiumboy self-assigned this Dec 20, 2024
@chromiumboy chromiumboy added the S: Awaiting Changes Status: Changes are required before another review can happen label Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: General Interactions Area: General in-game interactions that don't relate to another area. A: Medical Area: Medical department, including Chemistry Changes: UI Changes: Might require knowledge of UI design or code. D2: Medium Difficulty: A good amount of codebase knowledge required. P2: Raised Priority: Item has a raised priority, indicating it might get increased maintainer attention. S: Awaiting Changes Status: Changes are required before another review can happen S: Needs Review Status: Requires additional reviews before being fully accepted size/S Denotes a PR that changes 10-99 lines. T: Bugfix Type: Bugs and/or bugfixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dot on crew monitor moves smoothly if entity moving with a grid
4 participants