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

tgui teleconsole #16319

Merged
merged 42 commits into from Oct 15, 2023
Merged

tgui teleconsole #16319

merged 42 commits into from Oct 15, 2023

Conversation

Valtsu0
Copy link
Contributor

@Valtsu0 Valtsu0 commented Oct 7, 2023

About the PR

Replaces teleconsole's old ui with a tgui one
As a consequence, cyborgs can no longer interact with it from off-screen
image
image

Why's this needed?

tgui good

Changelog

(u)Valtsu0 and Mordent
(+)Science teleporter console now has a fancy new tgui ui
(+)Cyborgs can no longer use the science teleporter console from more than a screen away

@boring-cyborg boring-cyborg bot added the A-UI Modifies UI in some way. Automatically applied on a change to tgui/ label Oct 7, 2023
@github-actions github-actions bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Oct 7, 2023
@DisturbHerb
Copy link
Contributor

Why's the window so big compared to its content?

@Valtsu0
Copy link
Contributor Author

Valtsu0 commented Oct 7, 2023

I have made the window slightly smaller by default. Images updated to reflect this

@Valtsu0 Valtsu0 requested a review from ZeWaka October 8, 2023 16:34
@github-actions github-actions bot added the S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict label Oct 8, 2023
@github-actions github-actions bot added S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict and removed S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict labels Oct 9, 2023
@Paaiii
Copy link
Contributor

Paaiii commented Oct 12, 2023

hmm.. not really an issue exactly, but with the current telescience console UI, cyborgs are able to interact with it from anywhere.
which can be incredibly useful for quick motion and such for a lot of things, benign or otherwise - i personally use it constantly just casually for moving around or gimmicks because it's so handy.

making it TGUI would make it only interactable to cyborgs within viewing range. Which, while it seems to be the intention with computer interfaces as a general rule, should be at least noted down, considering how valuable a tool telescience can be in the silicon arsenal.

@Valtsu0
Copy link
Contributor Author

Valtsu0 commented Oct 12, 2023

hmm.. not really an issue exactly, but with the current telescience console UI, cyborgs are able to interact with it from anywhere. which can be useful for quick motion and such for a lot of things - i personally use it for that constantly just casually for moving around or gimmicks because it's so handy

making it TGUI would make it only interactable to cyborgs within viewing range. Which, while it seems to be the intention with computer interfaces as a general rule, should be at least noted down, because it was a pretty valuable tool in the silicon arsenal.

I feel like this should be part of sciborgs kit/an upgrade. In the meantime you can always ask the AI

@mordent-goonstation
Copy link
Contributor

hmm.. not really an issue exactly, but with the current telescience console UI, cyborgs are able to interact with it from anywhere. which can be incredibly useful for quick motion and such for a lot of things, benign or otherwise - i personally use it constantly just casually for moving around or gimmicks because it's so handy.

making it TGUI would make it only interactable to cyborgs within viewing range. Which, while it seems to be the intention with computer interfaces as a general rule, should be at least noted down, considering how valuable a tool telescience can be in the silicon arsenal.

Standardizing this is worthy of a note, but I would think is worth doing as it reduces surprises. The AI (and I believe AI controlled cyborgs, though would have to double check that) still have access to it from anywhere.

@mordent-goonstation
Copy link
Contributor

I feel like this should be part of sciborgs kit/an upgrade. In the meantime you can always ask the AI

An upgrade that increases the interaction range from ~1 screen to more (or even the whole z-level, a la AI) sounds neat.

Copy link
Contributor

@mordent-goonstation mordent-goonstation left a comment

Choose a reason for hiding this comment

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

I have some additional design feedback but that can wait for a follow-up PR from myself or someone else, the port as the main chunk of the work seems mostly decent.

code/modules/networks/computer3/mainframe2/telesci.dm Outdated Show resolved Hide resolved
code/modules/networks/computer3/mainframe2/telesci.dm Outdated Show resolved Hide resolved
code/modules/networks/computer3/mainframe2/telesci.dm Outdated Show resolved Hide resolved
code/modules/networks/computer3/mainframe2/telesci.dm Outdated Show resolved Hide resolved
code/modules/networks/computer3/mainframe2/telesci.dm Outdated Show resolved Hide resolved
tgui/packages/tgui/interfaces/TeleConsole.tsx Outdated Show resolved Hide resolved
tgui/packages/tgui/interfaces/TeleConsole.tsx Outdated Show resolved Hide resolved
tgui/packages/tgui/interfaces/TeleConsole.tsx Outdated Show resolved Hide resolved
code/modules/networks/computer3/mainframe2/telesci.dm Outdated Show resolved Hide resolved
tgui/packages/tgui/interfaces/TeleConsole.tsx Outdated Show resolved Hide resolved
Valtsu0 and others added 3 commits October 13, 2023 21:04
Co-authored-by: Mordent <62817778+mordent-goonstation@users.noreply.github.com>
Co-authored-by: Mordent <62817778+mordent-goonstation@users.noreply.github.com>
Copy link
Contributor

@mordent-goonstation mordent-goonstation left a comment

Choose a reason for hiding this comment

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

Broadly approving (with changes below), I think there's room for some design enhancements here but the base goal of "get it into tgui" seems to be fine, and I don't want to block that with my (subjective) design thoughts.

tgui/packages/tgui/interfaces/TeleConsole.tsx Outdated Show resolved Hide resolved
tgui/packages/tgui/interfaces/TeleConsole.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@mordent-goonstation mordent-goonstation left a comment

Choose a reason for hiding this comment

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

Putting a "request changes" block on this until I've got some improvements added to the branch. Working on those actively, please hassle me if this takes >24 hours.

@mordent-goonstation
Copy link
Contributor

PRed my changes to your branch, so they can be merged in in one go rather than incrementally.

Valtsu0#1

@github-actions github-actions bot added the S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict label Oct 15, 2023
@mordent-goonstation mordent-goonstation merged commit 849475b into goonstation:master Oct 15, 2023
26 checks passed
github-actions bot pushed a commit that referenced this pull request Oct 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-UI Modifies UI in some way. Automatically applied on a change to tgui/ S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants