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

Oldchat #11913

Merged
merged 31 commits into from
Oct 17, 2022
Merged

Oldchat #11913

merged 31 commits into from
Oct 17, 2022

Conversation

vulppine
Copy link
Contributor

@vulppine vulppine commented Oct 14, 2022

About the PR

SS13 style chat. This includes a new layout for the HUD, called 'Separated', which is similar to Space Station 13's UI layout. This also includes a new setting to set the width of the viewport, so that a user can make it more square, or more wider - the minimum size is 1:1, and the maximum size is the current ratio of SS14's viewport.

This also fixes a chat focusing bug seen when a player is in the lobby, and tries to focus on the chatbox using hotkeys.

Screenshots

image

image

image

image

Changelog

🆑

  • add: The classic Space Station 13 style HUD layout has been ported to Space Station 14! Enjoy having your chat in its own separate section of game HUD!

@github-actions github-actions bot added the Changes: UI Can be reviewed or fixed by people who are knowledgeable with UI design label Oct 14, 2022
@Zumorica Zumorica added the Holy Shit This is real important!! label Oct 15, 2022
@github-actions
Copy link
Contributor

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

@github-actions github-actions bot added the Merge Conflict This PR currently has conflicts that need to be addressed. label Oct 16, 2022
# Conflicts:
#	Content.Client/Lobby/LobbyState.cs
#	Content.Client/UserInterface/Systems/Chat/ChatUIController.cs
@vulppine vulppine marked this pull request as ready for review October 17, 2022 18:29
@github-actions github-actions bot added Status: Needs Review This PR requires new reviews before it can be merged. and removed Merge Conflict This PR currently has conflicts that need to be addressed. labels Oct 17, 2022
@ike709
Copy link
Contributor

ike709 commented Oct 17, 2022

I really think that the top bar of buttons should be reversed, so that Esc is next to the top right corner instead of just kind of in the middle of the screen. Otherwise, incredibly based.

@mirrorcult
Copy link
Contributor

NRE when opening client

[FATL] unhandled: System.NullReferenceException: Object reference not set to an instance of an object.
   at Robust.Shared.GameObjects.EntitySystemManager.GetEntitySystem[T]() in K:\github.com\mirrorcult\space-station-14\RobustToolbox\Robust.Shared\GameObjects\EntitySystemManager.cs:line 65
   at Robust.Shared.GameObjects.EntitySystem.Get[T]() in K:\github.com\mirrorcult\space-station-14\RobustToolbox\Robust.Shared\GameObjects\EntitySystem.cs:line 143
   at Content.Client.UserInterface.Systems.Chat.Widgets.ChatBox.OnTextChanged(LineEditEventArgs args) in K:\github.com\mirrorcult\space-station-14\Content.Client\UserInterface\Systems\Chat\Widgets\ChatBox.xaml.cs:line 200

@mirrorcult
Copy link
Contributor

(on startup, i hadn't typed anything)

@mirrorcult
Copy link
Contributor

mirrorcult commented Oct 17, 2022

ok that might be a master bug actually (edit it was)

@vulppine
Copy link
Contributor Author

ok that might be a master bug actually (edit it was)

i moved that system get into the controller anyways since there's already a dependency for it

@vulppine
Copy link
Contributor Author

I really think that the top bar of buttons should be reversed, so that Esc is next to the top right corner instead of just kind of in the middle of the screen. Otherwise, incredibly based.

the layout of the menu bar buttons isn't dynamic and would require another separate widget to be implemented in order to achieve this (not necessarily a bad idea, still)

Copy link
Contributor

@mirrorcult mirrorcult left a comment

Choose a reason for hiding this comment

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

scaling, cvar stuff all works fine, no bugs ingame afaict

Content.Client/Gameplay/GameplayState.cs Show resolved Hide resolved
Comment on lines +134 to +138
// TODO: This could just be like, the equivalent of an event or something
_ghostController.UpdateGui();
_actionController.RegisterActionContainer();
_alertsController.SyncAlerts();
_hotbarController.ReloadHotbar();
Copy link
Contributor

Choose a reason for hiding this comment

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

can this not just be an action/c# event that gets invoked and which all of these subscribe to, seems to be a pattern elsewhere in UI

Copy link
Contributor Author

Choose a reason for hiding this comment

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

state startup happens before the call that occurs when a state is entered, so i can't use that unfortunately - and since the state entered call also gives controllers a reference to the current state, it ends up being that i have to manually do this

Copy link
Contributor

Choose a reason for hiding this comment

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

well thats annoying

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i can try to do one thing though, and that's just move all this to its own controller

Copy link
Contributor

Choose a reason for hiding this comment

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

actually could you do this in a separate UI controller made for this purpose on the state enter

Copy link
Contributor

Choose a reason for hiding this comment

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

ok github didnt load that comment glad we had the same idea

@mirrorcult
Copy link
Contributor

i agree w ike on the order of the buttons but doesnt need to be done in this PR imo

@moonheart08 moonheart08 merged commit a3dafd8 into space-wizards:master Oct 17, 2022
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 Holy Shit This is real important!! 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.

None yet

5 participants