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

[Port] Snatchprod / Хваталка #65

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

Spatison
Copy link
Member

Описание PR

Порт Хваталки и скрытого крафта шок-палки


Медиа

Видео


Изменения

🆑 Spatison

  • add: Added snatchprod / Добавлена хваталка
  • tweak: You can now change the battery in the stunprod / В шок-палке теперь можно менять батарейку
  • tweak: The creation of the stunprod is now hidden / Создание шок-палке теперь скрытое

@Spatison Spatison self-assigned this Sep 21, 2024
Copy link

coderabbitai bot commented Sep 21, 2024

Walkthrough

The changes involve multiple updates across several files, primarily focusing on the StunbatonSystem class and the introduction of new components and systems related to melee interactions. Key modifications include enhancements to battery component handling, the addition of the SnatchOnMeleeHitComponent and its corresponding system, and structural changes to the stun prod and snatcher prod entities. Additionally, crafting graphs and metadata for textures have been updated or introduced, reflecting a comprehensive overhaul of melee weapon functionality and related assets.

Changes

File Path Change Summary
Content.Server/Stunnable/Systems/StunbatonSystem.cs Modifications to improve battery component handling, including the addition of TryGetBatteryComponent, refactoring of charge checking logic, and the introduction of an event handler for power cell changes.
Content.Server/_White/Melee/Snatch/SnatchOnMeleeHitComponent.cs Introduction of a new component SnatchOnMeleeHitComponent to handle melee hit interactions.
Content.Server/_White/Melee/Snatch/SnatchOnMeleeHitSystem.cs New system SnatchOnMeleeHitSystem added to manage interactions during melee hits, allowing for item snatching from hit entities.
Resources/Prototypes/Entities/Objects/Misc/handcuffs.yml Update to the handcuffs construction entry, changing the construction graph and deconstruction target.
Resources/Prototypes/Entities/Objects/Weapons/Melee/stunprod.yml Significant structural changes to the stun prod entity, creating an abstract base entity and introducing a new entity for the stun prod with updated components and visuals.
Resources/Prototypes/Recipes/Crafting/Graphs/improvised/makeshiftstunprod.yml Deletion of the makeshift stun prod crafting graph.
Resources/Prototypes/Recipes/Crafting/improvised.yml Removal of the makeshift stun prod entry from crafting recipes.
Resources/Prototypes/_White/Entities/Objects/Weapons/Melee/snatcherprod.yml New entity configuration for the snatcher prod weapon, inheriting from StunprodBase with defined components and behaviors.
Resources/Prototypes/_White/Recipes/hidden_crafts.yml Introduction of a new construction graph called StunprodGraph for crafting stun prod-related entities.
Resources/Textures/Objects/Weapons/Melee/stunprod.rsi/meta.json Updates to metadata for the stun prod sprite, including copyright changes and renaming of state names.
Resources/Textures/_White/Objects/Weapons/Melee/prod.rsi/meta.json New metadata file for a texture asset related to a melee weapon, defining version, license, copyright, size, and states.
Resources/Textures/_White/Objects/Weapons/Melee/snatcherprod.rsi/meta.json New metadata file for the snatcher prod texture, including version, license, copyright, size, and multiple animation states.

Possibly related PRs

Suggested reviewers

  • Remuchi

Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e421365 and 7def0ac.

Files selected for processing (1)
  • Content.Server/Stunnable/Systems/StunbatonSystem.cs (7 hunks)
Files skipped from review as they are similar to previous changes (1)
  • Content.Server/Stunnable/Systems/StunbatonSystem.cs

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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    -- I pushed a fix in commit <commit_id>, please review it.
    -- Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    -- @coderabbitai generate unit testing code for this file.
    -- @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    -- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    -- @coderabbitai read src/utils.ts and generate unit testing code.
    -- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    -- @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

github-actions bot commented Sep 21, 2024

RSI Diff Bot; head commit 7def0ac merging into f1cc259
This PR makes changes to 1 or more RSIs. Here is a summary of all changes:

Resources/Textures/Objects/Weapons/Melee/stunprod.rsi

State Old New Status
stunprod_on Modified
stunprod_off Removed
stunprod_cell Added

Resources/Textures/_White/Objects/Weapons/Melee/prod.rsi

State Old New Status
prod_unfinished Added

Resources/Textures/_White/Objects/Weapons/Melee/snatcherprod.rsi

State Old New Status
off-inhand-left Added
off-inhand-right Added
on-inhand-left Added
on-inhand-right Added
snatcherprod Added
snatcherprod_cell Added
snatcherprod_on Added

Edit: diff updated after 7def0ac

Copy link

@coderabbitai coderabbitai bot left a 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, and ContainerContainer 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

Commits

Files that changed from the base of the PR and between f1cc259 and e421365.

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 name prod_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 the Sprite 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 the SnatchOnMeleeHit 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 appropriate deconstructionTarget, graph, and node 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:

  1. It subscribes to the MeleeHitEvent for entities with the SnatchOnMeleeHitComponent.
  2. When a melee hit occurs, it checks if the item toggle is activated and if there are any hit entities.
  3. 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.
  4. For each held item, it attempts to drop it and allows the user to pick up the dropped item.
  5. The process stops after the first successful drop and pickup.

The code is well-structured and easy to understand. The dependencies on HandsSystem and ItemToggleSystem 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 the desword node is correct and specifies the prototype to spawn at this node.


13-87: The StunprodGraph 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 the StunprodBase 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 associating Cablecuffs with StunprodGraph.

The change in the construction properties of the Cablecuffs entity raises a concern about the appropriateness of associating it with the StunprodGraph. Given that Cablecuffs 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 charge

Your 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 status

By using TryGetBatteryComponent in OnExamined, 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 insufficient

The 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 state

Calling CheckCharge within OnChargeChanged 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 validation

The addition of OnPowerCellChanged ensures that any changes to the power cell promptly update the stunbaton's active state via CheckCharge. This keeps the item's state in sync with its power source.


121-126: Centralized charge checking improves maintainability

Implementing 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 retrieval

The TryGetBatteryComponent method efficiently retrieves the battery component, checking both the entity itself and its cell_slot container. This ensures compatibility with both built-in and replaceable power cells.

- !type:SpawnPrototype
prototype: Igniter
- !type:EmptyAllContainers
- !type:DeleteEntity
Copy link

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants