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

New tgui based orbit menu #51585

Merged
merged 13 commits into from Jun 14, 2020
Merged

New tgui based orbit menu #51585

merged 13 commits into from Jun 14, 2020

Conversation

Mothblocks
Copy link
Member

About The Pull Request

Replaces the default BYOND input window with a new tgui window that provides a friendly layout to ghosts.

Will show antagonists if those antagonists aren't meant to be stealthy, such as nightmare and ninjas. Basically, antags you would be able to obviously figure out just from being a ghost anyway.

For alive players and antagonists, will show the number of ghosts orbiting them, if there are any.

Previews:

iX7P1DALpd

reHRfOG6RV

Why It's Good For The Game

The current Orbit menu is infuriating to use. It's not even in alphabetical order so you have to just scroll through to find who you want to orbit, or just keep pressing the first initial until you're lucky. It also treats objects most players don't care about (such as the cleaner bots) alongside more important figures such as alive players.

Changelog

🆑
add: Added a new organized orbit menu.
/:cl:

@tgstation-server tgstation-server added Feature Exposes new bugs in interesting ways UI We make the game less playable, but with round edges labels Jun 11, 2020
@zxaber
Copy link
Contributor

zxaber commented Jun 11, 2020

To be honest the only thing about this I don't like is how annoying it'll be to find that one person out of a clump of sixty. Some sort of way to search would be neat (emulating how with the current menu we can tap a letter to cycle through all mobs with that name).

Maybe a search bar at the top that you can start typing in, and it only lists mobs below who's names start with the text you entered?

Also don't forget that we have the ability to orbit ghosts too.

@Mothblocks
Copy link
Member Author

Mothblocks commented Jun 11, 2020

Completely meant to add a search but forgot! Thanks for reminding me.

Yes, ghosts are supported in here, I just forgot to scroll down I believe.

@Mothblocks
Copy link
Member Author

Mothblocks commented Jun 11, 2020

Just using startsWith is a bit too naiive in my opinion since you won't be able to search stuff like CTF, but it's better than nothing and can be improved later. Committing this soon.

MVri6XT7bK

Copy link
Contributor

@MrDoomBringer MrDoomBringer left a comment

Choose a reason for hiding this comment

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

What a based PR, I had been meaning to do something similar for a bit

tgui/packages/tgui/interfaces/Orbit.js Outdated Show resolved Hide resolved
tgui/packages/tgui/interfaces/Orbit.js Outdated Show resolved Hide resolved
const PATTERN_NUMBER = / \(([0-9]+)\)$/;

const searchFor = bySearch => thing => {
return thing.name.toLowerCase().startsWith(bySearch.toLowerCase());
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know shit about JS tbh, but couldn't this be a regex search? That way you don't need to exactly type the starting characters of your search.

Copy link
Member Author

Choose a reason for hiding this comment

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

It could just as easily just check contains, I can change that later.

Copy link
Member

@stylemistake stylemistake left a comment

Choose a reason for hiding this comment

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

I am a bit of a purist when it comes to javascript, so sorry if this is too much for you. Poke me in discord if you have questions.

tgui/packages/tgui/interfaces/Orbit.js Outdated Show resolved Hide resolved
tgui/packages/tgui/interfaces/Orbit.js Outdated Show resolved Hide resolved
tgui/packages/tgui/interfaces/Orbit.js Outdated Show resolved Hide resolved
tgui/packages/tgui/interfaces/Orbit.js Outdated Show resolved Hide resolved
tgui/packages/tgui/interfaces/Orbit.js Outdated Show resolved Hide resolved
tgui/packages/tgui/interfaces/Orbit.js Outdated Show resolved Hide resolved
tgui/packages/tgui/interfaces/Orbit.js Outdated Show resolved Hide resolved
tgui/packages/tgui/interfaces/Orbit.js Outdated Show resolved Hide resolved
tgui/packages/tgui/interfaces/Orbit.js Outdated Show resolved Hide resolved
tgui/packages/tgui/interfaces/Orbit.js Outdated Show resolved Hide resolved
@MrDoomBringer
Copy link
Contributor

image

stylemistake
stylemistake previously approved these changes Jun 13, 2020
@Mothblocks
Copy link
Member Author

Whoops, rebasing didn't do what I expected it to--will just do a normal merge commit next time.

@PKPenguin321
Copy link
Contributor

Hitting Enter in the search bar should snap you to orbit the most relevant result

@Mothblocks
Copy link
Member Author

CC @stylemistake for a quick check on the latest commit, not sure if you get notifications for it.

@stylemistake
Copy link
Member

I do get notifications. All looks good. Rebasing is preferred; you can use tgui git hooks, which make rebasing easier.

@Mothblocks
Copy link
Member Author

Will definitely set those up when I can, thanks.

@PKPenguin321
Copy link
Contributor

Late on the draw to this but it would also be nice if this could automatically give focus to the text input when the window is opened so you don't have to click it

@Mothblocks
Copy link
Member Author

@PKPenguin321 I tried this, but couldn't get it to work. I think even in componentDidMount it's not rendered to the DOM at that point. Definitely something I want to look into though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Exposes new bugs in interesting ways UI We make the game less playable, but with round edges
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants