Alt-Z #135
Alt-Z #135
Conversation
WalkthroughВ данном пулл-запросе внесены изменения в несколько классов, связанных с системой взаимодействия и использования предметов в игре. Основные изменения включают добавление новых методов и свойств, которые позволяют различать взаимодействия с активной рукой и другими руками. В частности, в классе Changes
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (6)
Content.Shared/Wieldable/Components/WieldableComponent.cs (1)
40-46: Хорошее добавление, предлагаю улучшить документацию.Добавление флага
AltUseInHandлогично реализовано. Для улучшения документации предлагаю добавить пример использования./// <summary> /// Requires item to be alt-used in hand (alt-Z / alt-click in active hand) to be wielded. /// </summary> +/// <example> +/// This is typically enabled for firearms while being disabled for melee weapons. +/// </example> [DataField] public bool AltUseInHand = false;Content.Shared/Hands/EntitySystems/SharedHandsSystem.Interactions.cs (1)
152-155: Предлагаю улучшить структуру условий.Текущая структура условий может быть упрощена для лучшей читаемости.
- if(hand == handsComp.ActiveHand) // WD EDIT START - return _interactionSystem.ActiveHandAltInteract(uid, held) || _interactionSystem.AltInteract(uid, held); // todo: should these be merged into one method? - else // WD EDIT END - return _interactionSystem.AltInteract(uid, held); + return hand == handsComp.ActiveHand + ? _interactionSystem.ActiveHandAltInteract(uid, held) || _interactionSystem.AltInteract(uid, held) + : _interactionSystem.AltInteract(uid, held);Content.Shared/Wieldable/WieldableSystem.cs (2)
181-181: Улучшить читаемость условия.Предлагаю переписать условие для лучшей читаемости.
- if (args.Handled || component.AltUseInHand) // WD EDIT + if (args.Handled || component.AltUseInHand) return;
Line range hint
282-290: Рассмотреть выделение логики проверки патронов в отдельный компонент.Текущая реализация добавляет специфичную для оружия логику в общий компонент
WieldableComponent. Предлагаю создать отдельный компонент для этой функциональности.Создайте новый компонент
AmmoWieldableComponentи переместите в него логику проверки патронов:[RegisterComponent] public sealed class AmmoWieldableComponent : Component { public bool CanUnwield(EntityUid uid) { if (!TryComp<BallisticAmmoProviderComponent>(uid, out var provider)) return true; return provider.Entities.Count == 0 || !TryComp<CartridgeAmmoComponent>(provider.Entities[^1], out var ammo) || !ammo.Spent; } }Content.Shared/Verbs/Verb.cs (1)
313-323: Изменения в классе AlternativeVerb выглядят правильными!Добавление флага
InActiveHandOnlyхорошо документировано и соответствует целям PR. Можно рассмотреть более описательное название для флага, напримерRequiresActiveHandилиActiveHandRestricted, но текущее название тоже приемлемо.Content.Shared/Interaction/SharedInteractionSystem.cs (1)
1102-1118: Метод ActiveHandAltInteract реализован правильно, но нуждается в дополнительной документации!Реализация логична и соответствует общей архитектуре, но стоит добавить документацию о том, когда этот метод должен вызываться и как он взаимодействует с существующей системой взаимодействия.
Предлагаю добавить следующую документацию:
/// <summary> /// Very alternative interactions on an entity. /// </summary> +/// <remarks> +/// This method should be called when processing alt-interactions specifically for items in the active hand. +/// It complements the regular AltInteract method by handling verbs that require the item to be in the active hand. +/// </remarks> /// <returns>True if the interaction was handled, false otherwise.</returns> public bool ActiveHandAltInteract(EntityUid user, EntityUid target)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
Content.Shared/Hands/EntitySystems/SharedHandsSystem.Interactions.cs(1 hunks)Content.Shared/Interaction/SharedInteractionSystem.cs(1 hunks)Content.Shared/Verbs/Verb.cs(1 hunks)Content.Shared/Wieldable/Components/WieldableComponent.cs(1 hunks)Content.Shared/Wieldable/WieldableSystem.cs(2 hunks)
🔇 Additional comments (1)
Content.Shared/Interaction/SharedInteractionSystem.cs (1)
1093-1093: Фильтрация альтернативных глаголов реализована корректно!
Изменение метода AltInteract для исключения глаголов с InActiveHandOnly = true логично и эффективно реализовано.
| // WD EDIT START | ||
| /// <summary> | ||
| /// Copypasted <see cref="AddToggleWieldVerb(EntityUid, WieldableComponent, GetVerbsEvent{InteractionVerb})"/> | ||
| /// </summary> | ||
| private void AddAltWieldVerb(EntityUid uid, WieldableComponent component, GetVerbsEvent<AlternativeVerb> args) | ||
| { | ||
| if (!component.AltUseInHand) | ||
| return; | ||
|
|
||
| if (args.Hands == null || !args.CanAccess || !args.CanInteract) | ||
| return; | ||
|
|
||
| if (!_handsSystem.IsHolding(args.User, uid, out _, args.Hands)) | ||
| return; | ||
|
|
||
| // TODO VERB TOOLTIPS Make CanWield or some other function return string, set as verb tooltip and disable | ||
| // verb. Or just don't add it to the list if the action is not executable. | ||
|
|
||
| // TODO VERBS ICON | ||
| AlternativeVerb verb = new() | ||
| { | ||
| Text = component.Wielded ? Loc.GetString("wieldable-verb-text-unwield") : Loc.GetString("wieldable-verb-text-wield"), | ||
| Act = component.Wielded | ||
| ? () => TryUnwield(uid, component, args.User) | ||
| : () => TryWield(uid, component, args.User), | ||
| InActiveHandOnly = true | ||
| }; | ||
|
|
||
| args.Verbs.Add(verb); | ||
| } | ||
| // WD EDIT END |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Необходимо устранить дублирование кода.
Метод AddAltWieldVerb практически полностью дублирует AddToggleWieldVerb. Предлагаю выделить общую логику в отдельный приватный метод.
+private void AddWieldVerb<T>(EntityUid uid, WieldableComponent component, GetVerbsEvent<T> args, bool requireActiveHand = false) where T : BaseVerb
+{
+ if (requireActiveHand && !component.AltUseInHand)
+ return;
+
+ if (args.Hands == null || !args.CanAccess || !args.CanInteract)
+ return;
+
+ if (!_handsSystem.IsHolding(args.User, uid, out _, args.Hands))
+ return;
+
+ var verb = Activator.CreateInstance<T>();
+ verb.Text = component.Wielded ? Loc.GetString("wieldable-verb-text-unwield") : Loc.GetString("wieldable-verb-text-wield");
+ verb.Act = component.Wielded
+ ? () => TryUnwield(uid, component, args.User)
+ : () => TryWield(uid, component, args.User);
+
+ if (verb is AlternativeVerb altVerb)
+ altVerb.InActiveHandOnly = true;
+
+ args.Verbs.Add(verb);
+}Затем используйте этот метод:
-private void AddToggleWieldVerb(EntityUid uid, WieldableComponent component, GetVerbsEvent<InteractionVerb> args)
-{
- // ... existing implementation
-}
+private void AddToggleWieldVerb(EntityUid uid, WieldableComponent component, GetVerbsEvent<InteractionVerb> args)
+{
+ AddWieldVerb(uid, component, args);
+}
-private void AddAltWieldVerb(EntityUid uid, WieldableComponent component, GetVerbsEvent<AlternativeVerb> args)
-{
- // ... existing implementation
-}
+private void AddAltWieldVerb(EntityUid uid, WieldableComponent component, GetVerbsEvent<AlternativeVerb> args)
+{
+ AddWieldVerb(uid, component, args, true);
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // WD EDIT START | |
| /// <summary> | |
| /// Copypasted <see cref="AddToggleWieldVerb(EntityUid, WieldableComponent, GetVerbsEvent{InteractionVerb})"/> | |
| /// </summary> | |
| private void AddAltWieldVerb(EntityUid uid, WieldableComponent component, GetVerbsEvent<AlternativeVerb> args) | |
| { | |
| if (!component.AltUseInHand) | |
| return; | |
| if (args.Hands == null || !args.CanAccess || !args.CanInteract) | |
| return; | |
| if (!_handsSystem.IsHolding(args.User, uid, out _, args.Hands)) | |
| return; | |
| // TODO VERB TOOLTIPS Make CanWield or some other function return string, set as verb tooltip and disable | |
| // verb. Or just don't add it to the list if the action is not executable. | |
| // TODO VERBS ICON | |
| AlternativeVerb verb = new() | |
| { | |
| Text = component.Wielded ? Loc.GetString("wieldable-verb-text-unwield") : Loc.GetString("wieldable-verb-text-wield"), | |
| Act = component.Wielded | |
| ? () => TryUnwield(uid, component, args.User) | |
| : () => TryWield(uid, component, args.User), | |
| InActiveHandOnly = true | |
| }; | |
| args.Verbs.Add(verb); | |
| } | |
| // WD EDIT END | |
| // WD EDIT START | |
| private void AddWieldVerb<T>(EntityUid uid, WieldableComponent component, GetVerbsEvent<T> args, bool requireActiveHand = false) where T : BaseVerb | |
| { | |
| if (requireActiveHand && !component.AltUseInHand) | |
| return; | |
| if (args.Hands == null || !args.CanAccess || !args.CanInteract) | |
| return; | |
| if (!_handsSystem.IsHolding(args.User, uid, out _, args.Hands)) | |
| return; | |
| var verb = Activator.CreateInstance<T>(); | |
| verb.Text = component.Wielded ? Loc.GetString("wieldable-verb-text-unwield") : Loc.GetString("wieldable-verb-text-wield"); | |
| verb.Act = component.Wielded | |
| ? () => TryUnwield(uid, component, args.User) | |
| : () => TryWield(uid, component, args.User); | |
| if (verb is AlternativeVerb altVerb) | |
| altVerb.InActiveHandOnly = true; | |
| args.Verbs.Add(verb); | |
| } | |
| private void AddToggleWieldVerb(EntityUid uid, WieldableComponent component, GetVerbsEvent<InteractionVerb> args) | |
| { | |
| AddWieldVerb(uid, component, args); | |
| } | |
| private void AddAltWieldVerb(EntityUid uid, WieldableComponent component, GetVerbsEvent<AlternativeVerb> args) | |
| { | |
| AddWieldVerb(uid, component, args, true); | |
| } | |
| // WD EDIT END |
There was a problem hiding this comment.
Шило на мыло, тем более в там, куда почти не ходят и что (почти) никогда не лапают.
Не вижу смысла.
There was a problem hiding this comment.
@RedFoxIV, понимаю. Спасибо за ваше мнение.
|
нихуя какие технологии |
|
|
|
ладно посмотрим |
Описание PR
Альт-клик по предмету в активной руке теперь отличается от альт-клика по предмету в неактивной руке или на полу.
Медиа
ass.webm
Взаимодействие с предметом в мире у меня назначено на F, если что. (дефолт E)
Матчасть
В
AlternativeVerbтеперь есть флажокInActiveHandOnly, дефолтfalse. Если он выставлен наtrue, то данный верб можно прокнуть только через контекстное меню (как и все альтвербы), либо через альт. использование в руке, см. описание ПРа.InActiveHandOnly==true.В
WieldableComponentтеперь есть флажокAltUseInHand, дефолтfalse. Если он выставлен наtrue, то чтобы взять предмет в обе руки, нужно нажать альт-z. Если онfalse, то всё работает, как раньше. Для топоров и прочего можно оставить, как есть, для огнестрела фичу лучше включить.В прототипах:
Зачем
Мне не понравилось, что у оружия два разных действия (дёрнуть затвор и взять в две руки) были назначены на одну кнопку. Настолько сильно, что мне сказали это фиксить.
Изменения
🆑