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.

Sign up for GitHub

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

Jump to bottom

Add per player mutes #20100

Merged
merged 4 commits into from Aug 20, 2022
Merged

Add per player mutes #20100

merged 4 commits into from Aug 20, 2022

Conversation

PunkPun
Copy link
Member

@PunkPun PunkPun commented Jul 4, 2022 •
edited

Closes #20073
Closes #7448
Closes #5775
Closes #12421

@PunkPun
Copy link
Member Author

PunkPun commented Jul 4, 2022

svg's for icons can be found in OpenRA/ArtSrc#16

How it looks ingame
Imagine AI's here are players

ok not ok ok not ok

@PunkPun PunkPun mentioned this pull request Jul 7, 2022
Closed
dragunoff
dragunoff reviewed Jul 8, 2022
View reviewed changes
Copy link
Contributor

@dragunoff dragunoff left a comment

Choose a reason for hiding this comment

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

Not a comprehensive review or anything, just a couple of remarks:
image
The button appears for the local player when spectating.

And regarding the tooltip - I think it will be better to change the text depending on the state ("Mute this player" / "Unmute this player").

@PunkPun
Copy link
Member Author

PunkPun commented Jul 8, 2022

The button appears for the local player when spectating.

Fixed. I had fixed this issue before but only for the player mute checkbox and forgot to duplicate the changes to spectator

And regarding the tooltip - I think it will be better to change the text depending on the state ("Mute this player" / "Unmute this player").

I implemented it. Perhaps I should make tooltip toggling part of the checkbox refactor so doesn't need to be hardcoded

dragunoff
dragunoff suggested changes Jul 8, 2022
View reviewed changes
mods/cnc/chrome/ingame-infostats.yaml Outdated Show resolved Hide resolved
@PunkPun PunkPun force-pushed the mute-players branch 5 times, most recently from 5c0f7e2 to 8992238 Compare July 11, 2022 21:23
@pchote pchote added this to the Next release milestone Jul 12, 2022
@PunkPun PunkPun mentioned this pull request Jul 12, 2022
Merged
@PunkPun PunkPun force-pushed the mute-players branch 2 times, most recently from 5771342 to dfc0774 Compare July 15, 2022 16:36
@Smittytron
Copy link
Member

Needs rebase

@PunkPun
Copy link
Member Author

PunkPun commented Jul 26, 2022

Rebased

@teinarss
Copy link
Contributor

teinarss commented Aug 5, 2022

Needs another rebase.

@PunkPun
Copy link
Member Author

PunkPun commented Aug 5, 2022

rebased

Mailaender
Mailaender previously approved these changes Aug 6, 2022
View reviewed changes
Copy link
Member

@Mailaender Mailaender left a comment

Choose a reason for hiding this comment

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

Works as promised.

@PunkPun
Fixup faction and score info UI
cc3fb02 
And increase spectator name length to match regular players
@dragunoff dragunoff mentioned this pull request Aug 15, 2022
Merged
@PunkPun
Increase the size of ingame-info panel
dfbd14b 
And add an Actions section for Kick
@PunkPun
Copy link
Member Author

PunkPun commented Aug 15, 2022 •
edited

Now using the icons added in OpenRA/ArtSrc#17
I've also used fluent for the mute tooltip

@PunkPun PunkPun force-pushed the mute-players branch 2 times, most recently from dde2474 to 746d605 Compare August 16, 2022 08:15
dragunoff
dragunoff previously approved these changes Aug 16, 2022
View reviewed changes
Copy link
Contributor

@dragunoff dragunoff left a comment

Choose a reason for hiding this comment

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

Code and UI changes LGTM and I didn't spot any regressions when testing 👍

@PunkPun
Copy link
Member Author

PunkPun commented Aug 19, 2022

git compare is failing, using regular foreach() now

@teinarss teinarss merged commit ce254f8 into OpenRA:bleed Aug 20, 2022
@teinarss
Copy link
Contributor

https://github.com/OpenRA/OpenRA/wiki/Changelog-(bleed)/_compare/9503433e214b38ca5c9834881ecad12fe69cf703...08dff7f86c58d6b488f091c8ca3d9042984f59b6

@PunkPun PunkPun deleted the mute-players branch August 21, 2022 18:51
@PunkPun PunkPun mentioned this pull request Nov 9, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abcdefg30 abcdefg30 abcdefg30 left review comments

@teinarss teinarss teinarss approved these changes

@Mailaender Mailaender Mailaender left review comments

@dragunoff dragunoff dragunoff left review comments

Assignees
No one assigned
Labels
PR: Needs +2
Projects
None yet
Milestone
Release 20230225
7 participants
@PunkPun @Smittytron @teinarss @Mailaender @dragunoff @abcdefg30 @pchote