-
Notifications
You must be signed in to change notification settings - Fork 15
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] Snatchprod / Хваталка #65
Conversation
WalkthroughThe changes involve multiple updates across several files, primarily focusing on the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
RSI Diff Bot; head commit 5e6b9f1 merging into f1cc259 Resources/Textures/Objects/Weapons/Melee/stunprod.rsi
Resources/Textures/_White/Objects/Weapons/Melee/snatcherprod.rsi
|
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: 3
Outside diff range and nitpick comments (1)
Resources/Prototypes/Entities/Objects/Weapons/Melee/stunprod.yml (1)
51-67
: Review the added components.The addition of the
shape
,DisarmMalus
,StaticPrice
,PowerCellSlot
,ItemSlots
, andContainerContainer
components looks good. They provide necessary functionality for the stun prod, such as collision detection, disarm effect, pricing, and power cell management.However, the
Appearance
component seems unnecessary since it has no properties. Consider removing it if it serves no purpose.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (12)
Resources/Textures/Objects/Weapons/Melee/stunprod.rsi/stunprod.png
is excluded by!**/*.png
Resources/Textures/Objects/Weapons/Melee/stunprod.rsi/stunprod_cell.png
is excluded by!**/*.png
Resources/Textures/Objects/Weapons/Melee/stunprod.rsi/stunprod_off.png
is excluded by!**/*.png
Resources/Textures/Objects/Weapons/Melee/stunprod.rsi/stunprod_on.png
is excluded by!**/*.png
Resources/Textures/_White/Objects/Weapons/Melee/prod.rsi/prod_unfinished.png
is excluded by!**/*.png
Resources/Textures/_White/Objects/Weapons/Melee/snatcherprod.rsi/off-inhand-left.png
is excluded by!**/*.png
Resources/Textures/_White/Objects/Weapons/Melee/snatcherprod.rsi/off-inhand-right.png
is excluded by!**/*.png
Resources/Textures/_White/Objects/Weapons/Melee/snatcherprod.rsi/on-inhand-left.png
is excluded by!**/*.png
Resources/Textures/_White/Objects/Weapons/Melee/snatcherprod.rsi/on-inhand-right.png
is excluded by!**/*.png
Resources/Textures/_White/Objects/Weapons/Melee/snatcherprod.rsi/snatcherprod.png
is excluded by!**/*.png
Resources/Textures/_White/Objects/Weapons/Melee/snatcherprod.rsi/snatcherprod_cell.png
is excluded by!**/*.png
Resources/Textures/_White/Objects/Weapons/Melee/snatcherprod.rsi/snatcherprod_on.png
is excluded by!**/*.png
Files selected for processing (12)
- Content.Server/Stunnable/Systems/StunbatonSystem.cs (7 hunks)
- Content.Server/_White/Melee/Snatch/SnatchOnMeleeHitComponent.cs (1 hunks)
- Content.Server/_White/Melee/Snatch/SnatchOnMeleeHitSystem.cs (1 hunks)
- Resources/Prototypes/Entities/Objects/Misc/handcuffs.yml (1 hunks)
- Resources/Prototypes/Entities/Objects/Weapons/Melee/stunprod.yml (2 hunks)
- Resources/Prototypes/Recipes/Crafting/Graphs/improvised/makeshiftstunprod.yml (0 hunks)
- Resources/Prototypes/Recipes/Crafting/improvised.yml (0 hunks)
- Resources/Prototypes/_White/Entities/Objects/Weapons/Melee/snatcherprod.yml (1 hunks)
- Resources/Prototypes/_White/Recipes/hidden_crafts.yml (1 hunks)
- Resources/Textures/Objects/Weapons/Melee/stunprod.rsi/meta.json (1 hunks)
- Resources/Textures/_White/Objects/Weapons/Melee/prod.rsi/meta.json (1 hunks)
- Resources/Textures/_White/Objects/Weapons/Melee/snatcherprod.rsi/meta.json (1 hunks)
Files not reviewed due to no reviewable changes (2)
- Resources/Prototypes/Recipes/Crafting/Graphs/improvised/makeshiftstunprod.yml
- Resources/Prototypes/Recipes/Crafting/improvised.yml
Additional context used
yamllint
Resources/Prototypes/_White/Entities/Objects/Weapons/Melee/snatcherprod.yml
[error] 33-33: no new line character at the end of file
(new-line-at-end-of-file)
Resources/Prototypes/_White/Recipes/hidden_crafts.yml
[warning] 19-19: wrong indentation: expected 4 but found 6
(indentation)
[warning] 21-21: wrong indentation: expected 8 but found 10
(indentation)
[warning] 27-27: wrong indentation: expected 4 but found 6
(indentation)
[warning] 29-29: wrong indentation: expected 8 but found 10
(indentation)
[warning] 32-32: wrong indentation: expected 8 but found 10
(indentation)
[warning] 38-38: wrong indentation: expected 8 but found 2
(indentation)
[warning] 40-40: wrong indentation: expected 4 but found 6
(indentation)
[warning] 42-42: wrong indentation: expected 8 but found 10
(indentation)
[warning] 45-45: wrong indentation: expected 8 but found 10
(indentation)
[warning] 48-48: wrong indentation: expected 8 but found 10
(indentation)
[warning] 56-56: wrong indentation: expected 8 but found 2
(indentation)
[warning] 59-59: wrong indentation: expected 4 but found 6
(indentation)
[warning] 61-61: wrong indentation: expected 8 but found 10
(indentation)
[warning] 72-72: wrong indentation: expected 8 but found 2
(indentation)
[warning] 75-75: wrong indentation: expected 4 but found 6
(indentation)
[warning] 77-77: wrong indentation: expected 8 but found 10
(indentation)
[error] 87-87: no new line character at the end of file
(new-line-at-end-of-file)
Additional comments not posted (26)
Content.Server/_White/Melee/Snatch/SnatchOnMeleeHitComponent.cs (1)
1-4
: Approve the new component, but request additional information.The introduction of the
SnatchOnMeleeHitComponent
appears to be a part of the implementation related to the Snatch melee weapon and its interaction with other entities in the game. The component is properly registered and defined as a sealed partial class.However, without additional context or code, it is difficult to determine the specific functionality or impact of this component on the overall game mechanics.
Could you please provide more information about the purpose and usage of this component? It would be helpful to understand how it fits into the larger implementation of the Snatch melee weapon and its interactions with other entities in the game.
Resources/Textures/_White/Objects/Weapons/Melee/prod.rsi/meta.json (1)
1-14
: LGTM!The JSON file follows the correct syntax and structure for a texture metadata file. The included fields provide relevant information about the texture asset, such as the version, license, copyright, size, and states.
The
states
array correctly defines an object with the nameprod_unfinished
, which likely corresponds to the associated texture file name.Overall, the changes look good and can be approved.
Resources/Prototypes/_White/Entities/Objects/Weapons/Melee/snatcherprod.yml (5)
7-16
: LGTM!The
Sprite
component is correctly defined with appropriate layers and visibility mapping.
17-18
: LGTM!The
Item
component correctly references the same sprite file as theSprite
component for consistency.
19-28
: LGTM!The
GenericVisualizer
component is correctly defined with appropriate visuals and visibility mapping for dynamic visual feedback during gameplay.
29-29
: Clarify the behavior of theSnatchOnMeleeHit
component.The
SnatchOnMeleeHit
component is defined without any additional properties or configuration. Please provide more information on the specific behavior of this component when the weapon makes contact with a target.
30-33
: LGTM!The
Construction
component is correctly defined with the appropriatedeconstructionTarget
,graph
, andnode
properties.Tools
yamllint
[error] 33-33: no new line character at the end of file
(new-line-at-end-of-file)
Resources/Textures/Objects/Weapons/Melee/stunprod.rsi/meta.json (2)
4-4
: LGTM!The updated copyright notice correctly attributes the original author and the editor. This is a good practice to follow.
11-11
: Verify the usage of the renamed states in the codebase.The changes to the state names improve clarity and organization. However, ensure that all references to the old state names (
"stunprod_off"
and"stunprod_nocell"
) have been updated to use the new names ("stunprod"
and"stunprod_cell"
) throughout the codebase.Run the following script to verify the usage of the renamed states:
Also applies to: 14-14
Verification successful
State name changes verified and correctly implemented
The verification process has confirmed that the changes to the state names have been properly implemented across the codebase:
- Old state names ('stunprod_off' and 'stunprod_nocell') are no longer present.
- New state names ('stunprod' and 'stunprod_cell') are being used in the appropriate configuration and asset files.
- No references to these state names were found in C# files, suggesting they are not directly used in game logic code.
The changes appear to be consistent and correctly applied, with no remaining references to the old state names.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to the old state names have been updated. # Test 1: Search for the old state name "stunprod_off". Expect: No occurrences. rg --type csharp --type typescript --type javascript $'stunprod_off' # Test 2: Search for the old state name "stunprod_nocell". Expect: No occurrences. rg --type csharp --type typescript --type javascript $'stunprod_nocell' # Test 3: Search for the new state name "stunprod". Expect: Only occurrences in the relevant code. rg --type csharp --type typescript --type javascript $'stunprod' # Test 4: Search for the new state name "stunprod_cell". Expect: Only occurrences in the relevant code. rg --type csharp --type typescript --type javascript $'stunprod_cell'Length of output: 411
Script:
#!/bin/bash # Description: Verify all references to the old and new state names in C# files and general codebase. # Search for old state names echo "Searching for old state name 'stunprod_off':" rg 'stunprod_off' echo "Searching for old state name 'stunprod_nocell':" rg 'stunprod_nocell' # Search for new state names echo "Searching for new state name 'stunprod':" rg '\bstunprod\b' echo "Searching for new state name 'stunprod_cell':" rg 'stunprod_cell' # Search specifically in C# files echo "Searching in C# files:" find . -name "*.cs" -type f | xargs grep -n -E 'stunprod(_off|_nocell|\b|_cell)'Length of output: 1565
Content.Server/_White/Melee/Snatch/SnatchOnMeleeHitSystem.cs (1)
9-40
: LGTM!The
SnatchOnMeleeHitSystem
class correctly implements the snatch mechanic for melee hits. The logic is sound and follows the intended behavior:
- It subscribes to the
MeleeHitEvent
for entities with theSnatchOnMeleeHitComponent
.- When a melee hit occurs, it checks if the item toggle is activated and if there are any hit entities.
- If the hit entity is not the same as the entity that initiated the hit and has a
HandsComponent
, it enumerates through the held items.- For each held item, it attempts to drop it and allows the user to pick up the dropped item.
- The process stops after the first successful drop and pickup.
The code is well-structured and easy to understand. The dependencies on
HandsSystem
andItemToggleSystem
are properly injected.Great job on implementing this feature!
Resources/Textures/_White/Objects/Weapons/Melee/snatcherprod.rsi/meta.json (2)
1-8
: LGTM!The JSON structure and top-level properties are well-defined and follow the expected format for texture metadata files. The metadata provides important information about the texture, including the version, license, copyright, and size.
9-77
: LGTM!The "states" property is well-structured and organized, providing a clear representation of the different visual states for the texture. The states cover various aspects of the texture, including default appearance, cell attachment, animation, and in-hand variations for both left and right hands. The use of properties like "delays" for animation frames and "directions" for directional sprites is consistent and appropriate.
Resources/Prototypes/_White/Recipes/hidden_crafts.yml (2)
11-12
: LGTM!The addition of the
entity
property to thedesword
node is correct and specifies the prototype to spawn at this node.
13-87
: TheStunprodGraph
construction graph looks good overall.The graph structure and node transitions appear to be logically correct for crafting the stun prod and its components. Nice work!
Tools
yamllint
[warning] 19-19: wrong indentation: expected 4 but found 6
(indentation)
[warning] 21-21: wrong indentation: expected 8 but found 10
(indentation)
[warning] 27-27: wrong indentation: expected 4 but found 6
(indentation)
[warning] 29-29: wrong indentation: expected 8 but found 10
(indentation)
[warning] 32-32: wrong indentation: expected 8 but found 10
(indentation)
[warning] 38-38: wrong indentation: expected 8 but found 2
(indentation)
[warning] 40-40: wrong indentation: expected 4 but found 6
(indentation)
[warning] 42-42: wrong indentation: expected 8 but found 10
(indentation)
[warning] 45-45: wrong indentation: expected 8 but found 10
(indentation)
[warning] 48-48: wrong indentation: expected 8 but found 10
(indentation)
[warning] 56-56: wrong indentation: expected 8 but found 2
(indentation)
[warning] 59-59: wrong indentation: expected 4 but found 6
(indentation)
[warning] 61-61: wrong indentation: expected 8 but found 10
(indentation)
[warning] 72-72: wrong indentation: expected 8 but found 2
(indentation)
[warning] 75-75: wrong indentation: expected 4 but found 6
(indentation)
[warning] 77-77: wrong indentation: expected 8 but found 10
(indentation)
[error] 87-87: no new line character at the end of file
(new-line-at-end-of-file)
Resources/Prototypes/Entities/Objects/Weapons/Melee/stunprod.yml (4)
4-6
: LGTM!Defining an abstract base entity for stun prods is a good architectural decision. It promotes code reuse and maintainability.
69-105
: LGTM!The
Stunprod
entity builds upon theStunprodBase
entity, adding specific functionality and visual representation. The changes look good:
- The multiple sprite layers allow for different visual states based on the stun prod's toggle state and power cell presence, enhancing the visual feedback.
- The
GenericVisualizer
component effectively manages the visibility of the sprite layers, providing clear visual cues to the player.- The
Construction
component enables the construction and deconstruction of the stun prod, adding gameplay depth.Overall, the added components enhance the functionality, visual representation, and interactivity of the stun prod.
107-124
: LGTM!The addition of the
ProdUnfinished
entity is a nice touch. It represents an intermediate stage in the construction of the stun prod, adding depth to the crafting process.
- The specific sprite and visual representation clearly distinguish the unfinished prod from the completed stun prod.
- The
Construction
component effectively links the unfinished prod to the stun prod's construction graph, enabling a multi-stage construction process.This addition enhances the gameplay experience by providing a more immersive and realistic crafting system for the stun prod.
125-125
: Comment acknowledged.The presence of the comment indicating the end of the WD edit is appreciated. It helps in understanding the scope of the modifications made to the file and improves the maintainability of the code.
Resources/Prototypes/Entities/Objects/Misc/handcuffs.yml (1)
57-60
: Clarify the reasoning behind associatingCablecuffs
withStunprodGraph
.The change in the construction properties of the
Cablecuffs
entity raises a concern about the appropriateness of associating it with theStunprodGraph
. Given thatCablecuffs
are related to handcuffs, it seems inconsistent to associate them with the stunprod's construction graph.Please provide clarification on the reasoning behind this change and ensure that it aligns with the intended purpose and functionality of the
Cablecuffs
entity within the game's crafting system.Content.Server/Stunnable/Systems/StunbatonSystem.cs (7)
43-48
: Updated hit attempt logic correctly handles activation and battery chargeYour changes to
OnStaminaHitAttempt
ensure that the stunbaton only applies stamina damage when it is activated, has a battery component, and sufficient charge. This prevents unintended usage when the stunbaton lacks power.
58-61
: Examined event now provides accurate battery statusBy using
TryGetBatteryComponent
inOnExamined
, the examination message now accurately reflects the remaining uses based on the current battery charge. This enhances player feedback.
72-81
: Activation prevented when battery is insufficientThe modifications in
TryTurnOn
correctly prevent the stunbaton from being activated if there's no battery or if the charge is insufficient, and provide a popup message to the user. This improves usability and prevents confusion.
112-113
: Charge change now properly updates stunbaton stateCalling
CheckCharge
withinOnChargeChanged
ensures that the stunbaton deactivates when the battery charge drops below the required threshold. This maintains consistent behavior.
115-119
: Power cell changes trigger charge validationThe addition of
OnPowerCellChanged
ensures that any changes to the power cell promptly update the stunbaton's active state viaCheckCharge
. This keeps the item's state in sync with its power source.
121-126
: Centralized charge checking improves maintainabilityImplementing the
CheckCharge
method centralizes the logic for checking battery status and deactivating the stunbaton if necessary. This reduces code duplication and enhances readability.
128-151
: Robust battery component retrievalThe
TryGetBatteryComponent
method efficiently retrieves the battery component, checking both the entity itself and itscell_slot
container. This ensures compatibility with both built-in and replaceable power cells.
Resources/Prototypes/_White/Entities/Objects/Weapons/Melee/snatcherprod.yml
Show resolved
Hide resolved
- !type:SpawnPrototype | ||
prototype: Igniter | ||
- !type:EmptyAllContainers | ||
- !type:DeleteEntity |
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.
Add a newline at the end of the file.
The yamllint static analysis tool has identified a missing newline at the end of the file. Please add a newline to adhere to common conventions and avoid potential issues with some tools.
Add a newline at the end of the file.
Tools
yamllint
[error] 87-87: no new line character at the end of file
(new-line-at-end-of-file)
Описание PR
Порт Хваталки и скрытого крафта шок-палки
Медиа
Видео
Изменения
🆑 Spatison