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

Coloring system messages yellow and added a new widget for transient messages #16355

Merged
merged 1 commit into from Apr 27, 2019

Conversation

@teinarss
Copy link
Contributor

commented Mar 26, 2019

Fixes #16310
Fixes #16231

@obrakmann
Copy link
Contributor

left a comment

Only gave the PR a quick once-over and checked it out in-game.

One problem I noticed is that the new chat display swallows the mouse input in the area of the screen where it is located (you can see the cursor change to a regular mouse cursor, for example). You will probably want to implement something like other widgets do with their ClickThrough field.

OpenRA.Mods.Common/OpenRA.Mods.Common.csproj Outdated Show resolved Hide resolved
OpenRA.Mods.Common/OpenRA.Mods.Common.csproj Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Widgets/ChatDisplayWidget.cs Outdated Show resolved Hide resolved

@teinarss teinarss force-pushed the teinarss:chat_status branch from aa3a927 to a1ed46f Mar 27, 2019

@teinarss

This comment has been minimized.

Copy link
Contributor Author

commented Mar 27, 2019

Update: Made the requested changes

@dragunoff

This comment has been minimized.

Copy link
Contributor

commented Mar 28, 2019

I tested it ingame. Yellow color looks good 👍 And just thinking about it - should debug messages also be assigned a special color?

However I don't think the position of the transient messages is good. Since the chat window is not open most of the time (and the chat is often not active) they just hang there in the middle of the screen (it becomes very obvious on smaller screens). I think they'd be better below the chat box.

@pchote

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

However I don't think the position of the transient messages is good. Since the chat window is not open most of the time (and the chat is often not active) they just hang there in the middle of the screen (it becomes very obvious on smaller screens). I think they'd be better below the chat box.

My original proposal was for them to always be just above the top regular chat line - i.e. they move down if there isn't any messages. IMO this is still better than putting them below the chat, which introduces the opposite problem when there are no transient messages.

Having it at the bottom could work if there was only one transient line fitting in the space where the chat entry box is (and therefore hidden while the chat is active). The empty space below the chat entry box must remain clear, this is reserved for the command bar for players with smaller screen resolutions.

@matjaeck

This comment has been minimized.

Copy link
Contributor

commented Mar 28, 2019

IMO the position is not a problem but the transient messages should disappear much quicker than chat messages. It's the long lasting visibility that causes the impression that they "just hang there" (at least for me).

@teinarss

This comment has been minimized.

Copy link
Contributor Author

commented Mar 28, 2019

However I don't think the position of the transient messages is good. Since the chat window is not open most of the time (and the chat is often not active) they just hang there in the middle of the screen (it becomes very obvious on smaller screens). I think they'd be better below the chat box.

My original proposal was for them to always be just above the top regular chat line - i.e. they move down if there isn't any messages. IMO this is still better than putting them below the chat, which introduces the opposite problem when there are no transient messages.

Having it at the bottom could work if there was only one transient line fitting in the space where the chat entry box is (and therefore hidden while the chat is active). The empty space below the chat entry box must remain clear, this is reserved for the command bar for players with smaller screen resolutions.

That would make the Transient message move up when new chat messages are created and move down when old gets deleted. UX wise this would be bit messy, I think @matjaeck proposal is better then, to shorten the lifetime of these messages

Requests were addressed

@dragunoff

This comment has been minimized.

Copy link
Contributor

commented Mar 28, 2019

Having it at the bottom could work if there was only one transient line fitting in the space where the chat entry box is (and therefore hidden while the chat is active). The empty space below the chat entry box must remain clear, this is reserved for the command bar for players with smaller screen resolutions.

What I am suggesting is to nudge the chat box to the top and place the transients tray in it's place. This should not affect the command bar.

That would make the Transient message move up when new chat messages are created and move down when old gets deleted. UX wise this would be bit messy, I think @matjaeck proposal is better then, to shorten the lifetime of these messages

I think having the transient above the last chat line would be alright. Shortening the message lifetime could be worse UX because that time may not be enough to grasp the message. It may be alright though - needs to be tested.

@matjaeck

This comment has been minimized.

Copy link
Contributor

commented Apr 2, 2019

What I am suggesting is to nudge the chat box to the top and place the transients tray in it's place. This should not affect the command bar.

I could imagine that they become easier to overlook then. Screen notifications are at their current position very easy to see. Chat message lifetime is designed for reading multiple lines of text and needs to last longer than notification lifetime (bound to an ingame event).

Another thing I thought about is that we might want to have some more space for additional text lines: missions currently use the chat widget for showing both mission objectives and "story chat" which does not seem ideally. We could consider to add additional dedicated lines for primary and secondary objectives in the future that have their own lifetime.

@teinarss

This comment has been minimized.

Copy link
Contributor Author

commented Apr 10, 2019

Was thinking to split this PR in two, and just keeping the color changes in this PR, the transient message part grew larger than i was hoping for and will defer it to later or if someone else wants to take over.

@teinarss teinarss referenced this pull request Apr 11, 2019

@teinarss teinarss force-pushed the teinarss:chat_status branch from a1ed46f to bbf1f02 Apr 22, 2019

@teinarss

This comment has been minimized.

Copy link
Contributor Author

commented Apr 22, 2019

Splitted out transient messages and rebased.

@pchote

This comment has been minimized.

Copy link
Member

commented Apr 22, 2019

Having the server messages in yellow in the TD lobby looks weird, but I'm not sure whether this is a case of something that needs to be fixed or just having to get used to it.

It would be a good idea to expose the default and system message text colors to the chrome metrics, though.

@pchote

This comment has been minimized.

Copy link
Member

commented Apr 22, 2019

The Game.Debug messages still display as white.

@teinarss teinarss force-pushed the teinarss:chat_status branch 2 times, most recently from dc4ace8 to 1795436 Apr 22, 2019

@teinarss

This comment has been minimized.

Copy link
Contributor Author

commented Apr 22, 2019

Updated and rebased

@pchote pchote added the PR: Needs +2 label Apr 26, 2019

@abcdefg30
Copy link
Member

left a comment

Some system messages are still white:
grafik

OpenRA.Mods.Common/Widgets/Logic/Lobby/LobbyUtils.cs Outdated Show resolved Hide resolved

@teinarss teinarss force-pushed the teinarss:chat_status branch from 1795436 to 7089c70 Apr 27, 2019

@teinarss

This comment has been minimized.

Copy link
Contributor Author

commented Apr 27, 2019

Updated!

@abcdefg30 abcdefg30 merged commit d9d2202 into OpenRA:bleed Apr 27, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@abcdefg30

This comment has been minimized.

Copy link
Member

commented Apr 27, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.