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

Minor improvements & fixes to Shuttle Console UI #31623

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

Conversation

eoineoineoin
Copy link
Contributor

@eoineoineoin eoineoineoin commented Aug 29, 2024

About the PR

Fixes a couple of bugs in the shuttle console:

  1. Grids and docs were sometimes prematurely culled, resulting in popping appearing/disappearing geometry
  2. "Undock" button was inconsistently disabled, depending on which order multiple docks were undocked

Added a minor usability improvement - when operating the shuttle from a remote location, draws the location of the controlling console. Helps players in cargo to locate where they should be attempting to dock to.

Tidied up the math - code was previously doing a bunch of redundant, inefficient calculations. To help future readers of this code, I gave useful names to variables, instead of names like "matty."

Why / Balance

Technical details

For the inconsistent culling, code was previously just using an AABB overlap test, but didn't account for relative rotation between the shuttle and the other grid/docks.

For the optimizations, the code was transforming geometry twice, once via a Vector2.Transform(), then a hacky "flip y coordinate, scale, and transform" by hand. This can all be done with a single Transform(). Also removed some sqrt() calls.

Media

New cyan dot to show the location of your controlling console:
2024-08-29_15-23

Grids disappearing/appearing prematurely (this behaviour is fixed, and no longer occurs):
https://github.com/user-attachments/assets/4e7511b9-ee45-463a-919c-4a4225b03195

Docks disappearing/appearing prematurely (this behavriour is fixed and no longer occurs):
https://github.com/user-attachments/assets/772ede5d-b3fb-4753-9afd-0e6cc7456ddd

Undock buttons inconsistently disabled, depending on what order the docs were disconnected (also fixed, you can now always undock):
https://github.com/user-attachments/assets/f3a9de2d-0241-4506-a1d1-3e6bebc2d6aa

Requirements

  • I have read and I am following the Pull Request Guidelines. I understand that not doing so may get my pr closed at maintainer’s discretion
  • I have added screenshots/videos to this PR showcasing its changes ingame, or this PR does not require an ingame showcase

Breaking changes

BaseShuttleControl.DrawGrid() now takes a transform which directly converts from grid space into view space.

Changelog

Remove lots of sketchy transforms-of-transforms, which should have been
as single matrix multiply. Assign proper names to matrices. Remove some
redundant calculations.
@github-actions github-actions bot added Status: Needs Review This PR requires new reviews before it can be merged. Changes: UI Can be reviewed or fixed by people who are knowledgeable with UI design labels Aug 29, 2024
@github-actions github-actions bot added the Merge Conflict This PR currently has conflicts that need to be addressed. label Sep 19, 2024
Copy link
Contributor

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

Comment on lines 298 to +300
public void Execute(int index)
{
var vert = Vertices[index];
var adjustedVert = Vector2.Transform(vert, Matrix);
adjustedVert = adjustedVert with { Y = -adjustedVert.Y };

var scaledVert = ScalePosition(adjustedVert, MinimapScale, MidPoint);
ScaledVertices[index] = scaledVert;
ScaledVertices[index] = Vector2.Transform(Vertices[index], Matrix);
Copy link
Contributor

Choose a reason for hiding this comment

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

You'll probably want to bump the batch size to like 64 with reduced stuff happening.

Copy link
Contributor

@metalgearsloth metalgearsloth left a comment

Choose a reason for hiding this comment

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

Conflict

@github-actions github-actions bot removed the Merge Conflict This PR currently has conflicts that need to be addressed. label Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changes: UI Can be reviewed or fixed by people who are knowledgeable with UI design Status: Needs Review This PR requires new reviews before it can be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants