-
Notifications
You must be signed in to change notification settings - Fork 18
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
[Port] Emotions menu / Меню эмоций #55
Conversation
Note Currently processing new changes in this PR. This may take a few minutes, please wait... Files selected for processing (2)
WalkthroughThe pull request introduces a comprehensive update to the user interface, primarily focusing on the integration of an emotions menu. Key changes include the addition of an Changes
Tip CodeRabbit can suggest fixes for GitHub Check annotations.Configure Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Outside diff range and nitpick comments (2)
Content.Shared/Chat/Prototypes/EmotePrototype.cs (1)
37-44
: Consider improving the property names and removing non-standard comments.
The default value for
ButtonText
is set to "Unknown". Consider using a more meaningful default value that aligns with the intended purpose of the property.The naming of the
AllowToEmotionsMenu
property is inconsistent with theButtonText
property. For better readability and consistency, consider renaming it toAllowInEmotionsMenu
.The
// WD EDIT START
and// WD EDIT END
comments appear to be non-standard and may not adhere to the project's coding conventions. Consider removing them to maintain a clean and consistent codebase.Content.Client/Input/ContentContexts.cs (1)
62-62
: LGTM, but consider removing the "WD EDIT" comment.The code change looks good and is consistent with the PR objective of adding an emotions menu.
However, the "WD EDIT" comment doesn't add much value and can be removed to keep the code clean.
Apply this diff to remove the comment:
- human.AddFunction(ContentKeyFunctions.OpenEmotionsMenu); // WD EDIT + human.AddFunction(ContentKeyFunctions.OpenEmotionsMenu);
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
Resources/Textures/_White/Interface/emotions.svg.192dpi.png
is excluded by!**/*.png
Files selected for processing (23)
- Content.Client/Input/ContentContexts.cs (1 hunks)
- Content.Client/Options/UI/Tabs/KeyRebindTab.xaml.cs (1 hunks)
- Content.Client/UserInterface/Systems/MenuBar/GameTopMenuBarUIController.cs (4 hunks)
- Content.Client/UserInterface/Systems/MenuBar/Widgets/GameTopMenuBar.xaml (2 hunks)
- Content.Client/_White/UI/Emotions/EmotionsUIController.cs (1 hunks)
- Content.Client/_White/UI/Emotions/Windows/EmotionsWindow.xaml (1 hunks)
- Content.Client/_White/UI/Emotions/Windows/EmotionsWindow.xaml.cs (1 hunks)
- Content.Server/Chat/Systems/ChatSystem.Emote.cs (1 hunks)
- Content.Shared/Chat/Prototypes/EmotePrototype.cs (1 hunks)
- Content.Shared/Input/ContentKeyFunctions.cs (1 hunks)
- Resources/Locale/en-US/_white/HUD/game-hud.ftl (1 hunks)
- Resources/Locale/en-US/_white/emotes/speech-emotes.ftl (1 hunks)
- Resources/Locale/en-US/_white/escape-menu/options-menu.ftl (1 hunks)
- Resources/Locale/en-US/_white/ui/emotions-menu.ftl (1 hunks)
- Resources/Locale/ru-RU/HUD/game-hud.ftl (1 hunks)
- Resources/Locale/ru-RU/_white/HUD/game-hud.ftl (1 hunks)
- Resources/Locale/ru-RU/_white/emotes/speech-emotes.ftl (1 hunks)
- Resources/Locale/ru-RU/_white/escape-menu/options-menu.ftl (1 hunks)
- Resources/Locale/ru-RU/_white/ui/emotions-menu.ftl (1 hunks)
- Resources/Prototypes/Voice/speech_emotes.yml (1 hunks)
- Resources/Prototypes/_White/Voice/speech_emotes.yml (1 hunks)
- Resources/Textures/_White/Interface/emotions.svg.192dpi.png.yml (1 hunks)
- Resources/keybinds.yml (2 hunks)
Files skipped from review due to trivial changes (5)
- Resources/Locale/en-US/_white/HUD/game-hud.ftl
- Resources/Locale/en-US/_white/ui/emotions-menu.ftl
- Resources/Locale/ru-RU/_white/HUD/game-hud.ftl
- Resources/Locale/ru-RU/_white/ui/emotions-menu.ftl
- Resources/Textures/_White/Interface/emotions.svg.192dpi.png.yml
Additional comments not posted (57)
Resources/Locale/en-US/_white/escape-menu/options-menu.ftl (1)
1-1
: LGTM!The new localization entry for "Open emotions menu" UI option is correctly added.
Resources/Locale/ru-RU/_white/escape-menu/options-menu.ftl (1)
1-1
: LGTM!The addition of the localization string for opening the emotions menu in the Russian language resource file is approved. This change enhances the user interface for Russian-speaking users without altering any existing functionality or logic.
Content.Client/_White/UI/Emotions/Windows/EmotionsWindow.xaml.cs (1)
8-14
: LGTM!The
EmotionsWindow
class is correctly implemented and follows the standard pattern for defining a window in the project. The code changes are approved.Content.Client/_White/UI/Emotions/Windows/EmotionsWindow.xaml (1)
1-12
: LGTM!The
EmotionsWindow
element is well-defined and follows the XAML syntax. The use of localization for the window's title is a good practice.The
EmotionsContainer
is currently empty, but it is expected to be populated with the emotions menu content in the future.Resources/Prototypes/_White/Voice/speech_emotes.yml (4)
1-7
: LGTM!The
Nod
emote prototype is well-defined and follows a consistent structure. The use of localization keys for the button text and chat messages is a good practice for supporting multiple languages. SettingallowMenu
totrue
ensures that this emote can be accessed from a menu.
8-14
: LGTM!The
ShakeHead
emote prototype follows the same consistent structure as the previous emote. The localization keys andallowMenu
property are used appropriately.
15-21
: LGTM!The
Frown
emote prototype maintains consistency with the previous emotes in terms of structure and usage of localization keys and theallowMenu
property.
22-27
: LGTM!The
Smile
emote prototype follows the established pattern and is consistent with the other emotes in the file.Content.Client/UserInterface/Systems/MenuBar/GameTopMenuBarUIController.cs (3)
1-1
: LGTM!The new using statement is necessary to use the
EmotionsUIController
class in this file.
28-28
: LGTM!The new
EmotionsUIController
dependency is correctly added to theGameTopMenuBarUIController
class. This will allow integrating the emotions menu with the game top menu bar.
52-52
: LGTM!Calling the
LoadButton
method of theEmotionsUIController
in theUnloadButtons
andLoadButtons
methods ensures that the emotions menu button is properly loaded and unloaded when the game top menu bar is loaded or unloaded.Also applies to: 66-66
Resources/Locale/ru-RU/_white/emotes/speech-emotes.ftl (18)
1-7
: Scream emote translations look good.The button text and chat message variations for the scream emote are translated correctly.
9-15
: Laugh emote translations look good.The button text and chat message variations for the laugh emote are translated correctly.
17-18
: Honk emote translation looks good, but button text is missing.The chat message for the honk emote is translated correctly. However, there is no button text translation provided. This may be intentional if the honk emote does not have a dedicated button.
20-23
: Sigh emote translations look good.The button text and chat message for the sigh emote are translated correctly.
25-28
: Whistle emote translations look good.The button text and chat message for the whistle emote are translated correctly.
30-33
: Crying emote translations look good.The button text and chat message for the crying emote are translated correctly.
35-36
: Squish emote translation looks good, but button text is missing.The chat message for the squish emote is translated correctly. However, there is no button text translation provided. This may be intentional if the squish emote does not have a dedicated button.
38-39
: Chitter emote translation looks good, but button text is missing.The chat message for the chitter emote is translated correctly. However, there is no button text translation provided. This may be intentional if the chitter emote does not have a dedicated button.
41-42
: Squeak emote translation looks good, but button text is missing.The chat message for the squeak emote is translated correctly. However, there is no button text translation provided. This may be intentional if the squeak emote does not have a dedicated button.
44-46
: Click emote translations look good, but button text is missing.The two variations of the chat message for the click emote are translated correctly. However, there is no button text translation provided. This may be intentional if the click emote does not have a dedicated button.
48-51
: Clap emote translations look good.The button text and chat message for the clap emote are translated correctly.
53-62
: Snap emote translations look good.The button text and chat message variations for the snap emote are translated correctly, covering different verb conjugations.
64-67
: Salute emote translations look good.The button text and chat message for the salute emote are translated correctly.
69-72
: Deathgasp emote translations look good.The chat message variations for the deathgasp emote are translated correctly, accounting for different entity types (default, silicon, IPC). The use of placeholders for possessive adjectives based on the entity is a good approach.
74-99
: Translations for Buzz, Weh, Chirp, Beep, Boop, Chime, Buzz-Two, Ping, and Whirr emotes look good, but button texts are missing.The chat messages for the Buzz, Weh, Chirp, Beep, Boop, Chime, Buzz-Two, Ping, and Whirr emotes are translated correctly. However, there are no button text translations provided for any of these emotes. This may be intentional if these emotes do not have dedicated buttons.
101-104
: Nod emote translations look good.The button text and chat message for the nod emote are translated correctly.
106-109
: ShakeHead emote translations look good.The button text and chat message for the shake head emote are translated correctly.
111-119
: Frown and Smile emote translations look good.The button texts and chat messages for the frown and smile emotes are translated correctly.
Content.Client/_White/UI/Emotions/EmotionsUIController.cs (1)
17-131
: LGTM!The
EmotionsUIController
class is well-structured and follows the Single Responsibility Principle. It properly handles the lifecycle of the emotions window and button based on the gameplay state. The class efficiently populates the emotions window by enumerating emote prototypes and creating buttons for each one. It also implements a cooldown mechanism to prevent spamming emotes.The code changes are approved.
Resources/Prototypes/Voice/speech_emotes.yml (20)
5-12
: LGTM!The changes to the
Scream
emote look good. The addition ofbuttonText
, updates tochatMessages
andchatTriggers
, and inclusion ofallowMenu
are consistent with the new emote structure.
17-24
: LGTM!The changes to the
Laugh
emote look good. The addition ofbuttonText
, updates tochatMessages
andchatTriggers
, and inclusion ofallowMenu
are consistent with the new emote structure.
29-31
: LGTM!The changes to the
Honk
emote look good. The updates tochatMessages
andchatTriggers
are consistent with the new emote structure.
36-40
: LGTM!The changes to the
Sigh
emote look good. The addition ofbuttonText
, updates tochatMessages
andchatTriggers
, and inclusion ofallowMenu
are consistent with the new emote structure.
45-49
: LGTM!The changes to the
Whistle
emote look good. The addition ofbuttonText
, updates tochatMessages
andchatTriggers
, and inclusion ofallowMenu
are consistent with the new emote structure.
54-59
: LGTM!The changes to the
Crying
emote look good. The addition ofbuttonText
, updates tochatMessages
andchatTriggers
, and inclusion ofallowMenu
are consistent with the new emote structure.
64-66
: LGTM!The changes to the
Squish
emote look good. The updates tochatMessages
andchatTriggers
are consistent with the new emote structure.
71-73
: LGTM!The changes to the
Chitter
emote look good. The updates tochatMessages
andchatTriggers
are consistent with the new emote structure.
78-80
: LGTM!The changes to the
Squeak
emote look good. The updates tochatMessages
andchatTriggers
are consistent with the new emote structure.
85-88
: LGTM!The changes to the
Click
emote look good. The updates tochatMessages
andchatTriggers
are consistent with the new emote structure.
94-98
: LGTM!The changes to the
Clap
emote look good. The addition ofbuttonText
, updates tochatMessages
andchatTriggers
, and inclusion ofallowMenu
are consistent with the new emote structure.
103-113
: LGTM!The changes to the
Snap
emote look good. The addition ofbuttonText
, updates tochatMessages
andchatTriggers
, and inclusion ofallowMenu
are consistent with the new emote structure.
118-122
: LGTM!The changes to the
Salute
emote look good. The addition ofbuttonText
, updates tochatMessages
andchatTriggers
, and inclusion ofallowMenu
are consistent with the new emote structure.
126-126
: LGTM!The changes to the
DefaultDeathgasp
emote look good. The update tochatMessages
is consistent with the new emote structure.
130-130
: LGTM!The changes to the
SiliconDeathgasp
emote look good. The update tochatMessages
is consistent with the new emote structure.
135-137
: LGTM!The changes to the
Buzz
emote look good. The updates tochatMessages
andchatTriggers
are consistent with the new emote structure.
142-142
: LGTM!The changes to the
Weh
emote look good. The update tochatMessages
is consistent with the new emote structure.
147-149
: LGTM!The changes to the
Chirp
emote look good. The updates tochatMessages
andchatTriggers
are consistent with the new emote structure.
155-157
: LGTM!The changes to the
Beep
emote look good. The updates tochatMessages
andchatTriggers
are consistent with the new emote structure.
162-164
: LGTM!The changes to the
Boop
emote look good. The updates tochatMessages
andchatTriggers
are consistent with the new emote structure.Content.Client/UserInterface/Systems/MenuBar/Widgets/GameTopMenuBar.xaml (2)
22-22
: LGTM!The code change is approved.
46-55
: LGTM!The code changes are approved.
The addition of the
EmotionsButton
with its properties and key binding looks good. It will provide users with access to the emotions menu from the game top menu bar.Content.Server/Chat/Systems/ChatSystem.Emote.cs (1)
31-31
: LGTM!The change to use
Loc.GetString(word).ToLower()
for assigninglowerWord
is a good improvement to ensure emote triggers are correctly localized. This will help address potential issues with case sensitivity in different languages or contexts.Content.Shared/Input/ContentKeyFunctions.cs (1)
28-28
: LGTM!The addition of the
OpenEmotionsMenu
field to theContentKeyFunctions
class is approved. This new bound key function will allow users to open the emotions menu using a specific key binding.Resources/keybinds.yml (2)
190-194
: LGTM!The new key binding for the
OpenEmotionsMenu
function looks good. TheY
key is a suitable choice and the changes are clearly marked with comments.
551-555
: LGTM!The addition of comment markers around the
LookUp
binding improves the readability and maintainability of the file without altering its functionality.Content.Client/Options/UI/Tabs/KeyRebindTab.xaml.cs (1)
231-231
: LGTM!The code changes are approved.
Описание PR
Порт меню эмоций.
Изменения
🆑 Spatison