[Port] Small Energy Crossbow / Маленький Энергетический Арбалет #60
Conversation
github-actions
bot
added
Changes: C#
Changes: Localization
Changes: Sprite
Changes: YML
labels
WalkthroughThis pull request introduces several new components and systems related to visual effects and collision mechanics in the game. Key additions include the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 2f488ed merging into 6fb8e63 Resources/Textures/_White/Objects/Weapons/Guns/Battery/mini-ebow.rsi
Resources/Textures/_White/Objects/Weapons/Guns/Projectiles/ebolt.rsi
|
Spatison
changed the title
![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 (3)
Resources/Locale/ru-RU/_white/prototypes/entities/objects/weapons/guns/battery/ebow.ftl (1)
1-2: LGTM! The localized entries for the new weapon entity are clear and descriptive.The addition of the "Energy Crossbow Mini" weapon entity and its corresponding localized entries in Russian is a great enhancement to the game's weapon variety. The name and description effectively convey the weapon's functionality and its impact on gameplay.
A few suggestions for improvement:
- Consider adding localized entries for any additional properties or attributes specific to this weapon entity, such as ammo type, reload speed, or unique visual effects.
- Ensure that the localized entries are consistent with the naming conventions and writing style used for other weapon entities in the game.
- Double-check the accuracy of the Russian translation to ensure that it effectively communicates the intended meaning to Russian-speaking players.
Content.Shared/_White/Collision/Blur/BlurOnCollideComponent.cs (1)
1-8: LGTM! Minor suggestions for improvement.The
BlurOnCollideComponentclass is well-structured and follows good practices like using the[RegisterComponent]attribute and being marked assealed.A few suggestions for improvement:
- Consider adding XML documentation comments to describe the purpose and usage of the class and its properties. This can help other developers understand how to use the component correctly.
- If the class definition is not expected to be split across multiple files, consider removing the
partialkeyword to keep the code simpler.Resources/Prototypes/_White/Entities/Objects/Weapons/Guns/Projectiles/bolts..yml (1)
31-31: Add a newline character at the end of the file.The static analysis tool
yamllinthas flagged an issue with a missing newline character at the end of the file. While this is a minor formatting issue and does not affect the functionality of the YAML configuration, it is generally considered good practice to include a newline character at the end of files for consistency and to comply with common coding style guidelines.Please add a newline character at the end of the file to resolve this issue.
Tools
yamllint
[error] 31-31: no new line character at the end of file
(new-line-at-end-of-file)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (6)
Resources/Textures/_White/Objects/Weapons/Guns/Battery/mini-ebow.rsi/bolt.pngis excluded by!**/*.pngResources/Textures/_White/Objects/Weapons/Guns/Battery/mini-ebow.rsi/empty.pngis excluded by!**/*.pngResources/Textures/_White/Objects/Weapons/Guns/Battery/mini-ebow.rsi/icon.pngis excluded by!**/*.pngResources/Textures/_White/Objects/Weapons/Guns/Battery/mini-ebow.rsi/inhand-left.pngis excluded by!**/*.pngResources/Textures/_White/Objects/Weapons/Guns/Battery/mini-ebow.rsi/inhand-right.pngis excluded by!**/*.pngResources/Textures/_White/Objects/Weapons/Guns/Projectiles/ebolt.rsi/bullet.pngis excluded by!**/*.png
Files selected for processing (14)
- Content.Shared/Eye/Blinding/Components/BlurryVisionComponent.cs (1 hunks)
- Content.Shared/_White/Collision/Blur/BlurOnCollideComponent.cs (1 hunks)
- Content.Shared/_White/Collision/Blur/BlurOnCollideSystem.cs (1 hunks)
- Content.Shared/_White/Collision/Knockdown/KnockdownOnCollideComponent.cs (1 hunks)
- Content.Shared/_White/Collision/Knockdown/KnockdownOnCollideSystem.cs (1 hunks)
- Resources/Locale/en-US/_white/store/uplink-catalog.ftl (1 hunks)
- Resources/Locale/ru-RU/_white/prototypes/entities/objects/weapons/guns/battery/ebow.ftl (1 hunks)
- Resources/Locale/ru-RU/_white/store/uplink-catalog.ftl (1 hunks)
- Resources/Prototypes/_White/Catalog/uplink_catalog.yml (1 hunks)
- Resources/Prototypes/_White/Entities/Objects/Weapons/Guns/Battery/mini_ebow.yml (1 hunks)
- Resources/Prototypes/_White/Entities/Objects/Weapons/Guns/Projectiles/bolts..yml (1 hunks)
- Resources/Prototypes/status_effects.yml (1 hunks)
- Resources/Textures/_White/Objects/Weapons/Guns/Battery/mini-ebow.rsi/meta.json (1 hunks)
- Resources/Textures/_White/Objects/Weapons/Guns/Projectiles/ebolt.rsi/meta.json (1 hunks)
Files skipped from review due to trivial changes (2)
- Content.Shared/Eye/Blinding/Components/BlurryVisionComponent.cs
- Resources/Textures/_White/Objects/Weapons/Guns/Projectiles/ebolt.rsi/meta.json
Additional context used
yamllint
Resources/Prototypes/_White/Entities/Objects/Weapons/Guns/Projectiles/bolts..yml
[error] 31-31: no new line character at the end of file
(new-line-at-end-of-file)
Resources/Prototypes/_White/Catalog/uplink_catalog.yml
[warning] 32-32: too many spaces after colon
(colons)
[warning] 37-37: wrong indentation: expected 4 but found 2
(indentation)
[warning] 39-39: wrong indentation: expected 4 but found 2
(indentation)
[warning] 42-42: wrong indentation: expected 8 but found 6
(indentation)
[error] 43-43: no new line character at the end of file
(new-line-at-end-of-file)
Additional comments not posted (29)
Content.Shared/_White/Collision/Blur/BlurOnCollideComponent.cs (1)
6-7: Verify the defaultBlurTimevalue and its impact on gameplay.The default value of 20 seconds for the
BlurTimefield seems reasonable, but it's important to ensure that this value provides the desired visual effect and doesn't negatively impact gameplay.Consider testing different values for
BlurTimeto find the optimal duration for the blur effect. Also, think about whether this value should be configurable for different use cases or entities.To verify the impact of the
BlurTimevalue, you can use the following script to search for any other references to theBlurOnCollideComponentand analyze how theBlurTimevalue is used:Verification successful
To gather more information and fully verify the impact of the BlurTime value, I'll run additional scripts to search for related information in the codebase.
BlurTime value is consistent, but lacks configurability and documentation
The default
BlurTimevalue of 20 seconds is consistently used throughout the codebase. However, there are a few points to consider:
- The value is not configurable through external files, which may limit flexibility in adjusting the gameplay experience.
- There's no documentation explaining the reasoning behind the 20-second duration or its intended impact on gameplay.
To improve this:
- Consider making
BlurTimeconfigurable through a settings file for easier tuning.- Add comments or documentation explaining the intended effect of the 20-second blur duration on gameplay.
- Conduct manual testing to ensure the 20-second duration provides the desired gameplay experience across different scenarios.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for references to BlurOnCollideComponent and analyze BlurTime usage. # Test: Search for BlurOnCollideComponent usage. Expect: Relevant code snippets. rg --type cs -A 5 $'BlurOnCollideComponent'Length of output: 2423
Script:
#!/bin/bash # Search for other references to BlurTime rg --type cs -i 'blurtime' # Search for configuration files that might override BlurTime fd -e yaml -e json | xargs rg -i 'blurtime' # Search for documentation or comments related to blur effect or BlurTime rg -i 'blur.*effect|blurtime' -g '*.md' -g '*.txt'Length of output: 333
Content.Shared/_White/Collision/Knockdown/KnockdownOnCollideComponent.cs (1)
5-10: LGTM!The
KnockdownOnCollideComponentclass is well-structured and introduces a useful mechanic for handling knockdowns upon collision. TheBehaviorfield provides flexibility in defining how held items are handled during knockdowns, with a sensible default value.Some additional insights:
- The
DropHeldItemsBehaviorenum is imported from another namespace, ensuring proper encapsulation and reusability of the behavior definitions.- Declaring the class as
sealedprevents unintended inheritance, which can help maintain the integrity of the component's behavior.- The
partialdeclaration allows for potential extension of the class in other files, promoting modularity and maintainability.Overall, this component adds value to the game's collision system and enhances gameplay interactions.
Resources/Locale/ru-RU/_white/store/uplink-catalog.ftl (2)
5-6: LGTM!The description accurately conveys the functionality of the betrayal knife weapon. The translation seems appropriate.
7-8: LGTM!The name and description accurately convey the characteristics of the small energy crossbow weapon. The translations seem appropriate.
Resources/Locale/en-US/_white/store/uplink-catalog.ftl (1)
7-8: LGTM!The addition of the "Small energy crossbow" weapon and its description aligns with the PR objectives and enhances the game's weapon catalog. The description provides relevant details about the weapon's characteristics and suggests tactical options for players.
Resources/Textures/_White/Objects/Weapons/Guns/Battery/mini-ebow.rsi/meta.json (1)
1-28: LGTM!The
meta.jsonfile follows a well-structured format for defining metadata related to the mini electronic bow weapon asset. The inclusion of licensing and copyright information ensures proper attribution. The specified asset states and directions provide flexibility for rendering the asset in different contexts within the game.The metadata structure is clear, concise, and follows best practices.
Resources/Prototypes/_White/Entities/Objects/Weapons/Guns/Projectiles/bolts..yml (7)
1-5: LGTM!The entity type and metadata are configured correctly. Deriving from the
BaseBulletparent entity is a good practice for reusing common functionality, and the uniqueidallows the entity to be referenced and spawned programmatically. SettingnoSpawntotrueis appropriate for a projectile entity that should only be spawned when fired from a weapon.
7-9: Verify the "bullet" state in the sprite file.The
Spritecomponent is correctly defined and specifies the visual representation of the energy crossbow bolt. The file path for the sprite resource follows the expected convention.Please verify that the "bullet" state exists in the
_White/Objects/Weapons/Guns/Projectiles/ebolt.rsisprite file to ensure the correct animation is displayed.
10-11: LGTM!The
Ammocomponent is correctly defined for the energy crossbow bolt entity. Setting themuzzleFlashproperty tonullis a valid configuration option to disable the muzzle flash effect, which makes sense for a crossbow bolt as it is not a typical visual effect associated with this type of projectile.
12-16: Ensure the damage amount is balanced with other weapons.The
Projectilecomponent is correctly defined for the energy crossbow bolt entity. Setting theimpactEffecttonullis a valid configuration option to disable any visual effects upon impact.The damage configuration specifies that the bolt deals
Poisondamage, which aligns with the concept of an energy weapon. The damage amount of 15 points seems reasonable, but please ensure that it is properly balanced with other weapons in the game to maintain fair gameplay.
17-19: LGTM!The
Reflectivecomponent is correctly defined for the energy crossbow bolt entity. Setting thereflectiveproperty to an array allows specifying multiple types of projectiles that the bolt can reflect.Including "Energy" in the
reflectivearray is an interesting gameplay mechanic that enables the bolt to potentially deflect incoming energy attacks. This adds a unique strategic element to the weapon's usage.
20-28: Verify the AABB bounds match the bolt sprite.The
Fixturescomponent is correctly defined for the energy crossbow bolt entity. The "projectile" fixture uses an axis-aligned bounding box (AABB) shape, which is a common choice for simple projectile collisions.Please ensure that the bounds of the AABB shape (
"-0.1,-0.2,0.1,0.2") accurately match the visual representation of the bolt sprite. This will help maintain consistency between the visual and collision aspects of the bolt.Setting the
hardproperty tofalseand specifying themaskproperty to only collide with "Opaque" objects are appropriate configurations for a projectile that should pass through non-solid obstacles.
29-31: Provide more information about theBlurOnCollidecomponent.The
KnockdownOnCollidecomponent is correctly defined for the energy crossbow bolt entity. Setting thebehaviorproperty to "AlwaysDrop" ensures that the bolt consistently causes the target to be knocked down upon collision, which can be a useful gameplay mechanic.Regarding the
BlurOnCollidecomponent, while it is correctly defined, there are no additional properties specified. It would be helpful to provide more information or documentation on the expected behavior and purpose of this component. What kind of blur effect does it apply, and how does it enhance the gameplay experience?Tools
yamllint
[error] 31-31: no new line character at the end of file
(new-line-at-end-of-file)
Resources/Prototypes/_White/Entities/Objects/Weapons/Guns/Battery/mini_ebow.yml (9)
7-9: LGTM!The
Spritecomponent is correctly defined with the appropriate sprite file and default state.
10-12: LGTM!The
Itemcomponent is correctly defined with the appropriate size and sprite file.
13-15: LGTM!The
Clothingcomponent is correctly defined, allowing the item to be equipped in the "Belt" slot.
16-20: LGTM!The
Guncomponent is correctly defined with the appropriate properties such as fire rate and gunshot sound.
21-24: LGTM!The
RechargeBasicEntityAmmocomponent is correctly defined with the appropriate recharge cooldown and sound.
25-28: LGTM!The
BasicEntityAmmoProvidercomponent is correctly defined with the appropriate ammo type, capacity, and initial count.
29-29: LGTM!The
AmmoCountercomponent is correctly defined to track the ammo count for the gun.
30-35: LGTM!The
GenericVisualizercomponent is correctly defined with the appropriate visual states based on the ammo status.
36-36: LGTM!The
Appearancecomponent is correctly defined to handle the overall appearance of the entity.Resources/Prototypes/status_effects.yml (2)
63-68: LGTM!The
RecentlyBlockedstatus effect is defined correctly and follows the established conventions. ThealwaysAllowedproperty and matching alert identifier are used appropriately.
69-72: LGTM!The
BlurryVisionstatus effect is defined correctly and uses thealwaysAllowedproperty appropriately. The absence of an alert identifier is acceptable, as it may not require direct player notification.Content.Shared/_White/Collision/Knockdown/KnockdownOnCollideSystem.cs (4)
11-17: LGTM!The
Initializemethod correctly subscribes to the required events for handling knockdown on collide functionality. The event handlers are appropriately named and associated with theKnockdownOnCollideComponent.
19-22: LGTM!The
OnEntityHitmethod correctly handles theThrowDoHitEventevent by invoking theApplyEffectsmethod with the appropriate arguments.
24-27: LGTM!The
OnProjectileHitmethod correctly handles theProjectileHitEventevent by invoking theApplyEffectsmethod with the appropriate arguments.
29-32: LGTM!The
ApplyEffectsmethod correctly uses theSharedLayingDownSystemto attempt to make the target entity lie down based on the providedKnockdownOnCollideComponent. The arguments passed to theTryLieDownmethod are appropriate.Content.Shared/_White/Collision/Blur/BlurOnCollideSystem.cs (1)
8-37: LGTM!The
BlurOnCollideSystemclass is well-structured and follows the expected pattern for an ECS system. It correctly subscribes to the relevant events and applies the blur effect on the target entity upon collision.Some key observations:
- The class is correctly subscribing to the
ProjectileHitEventandThrowDoHitEventevents in theInitializemethod.- The event handlers
OnEntityHitandOnProjectileHitare invoking theApplyEffectsmethod with the correct arguments.- The
ApplyEffectsmethod is using theStatusEffectsSystemto add theBlurryVisionComponentstatus effect with the correct duration.The class introduces a new gameplay mechanic where entities can be blurred upon collision. While this could potentially impact the balance and difficulty of the game, the class itself is self-contained and does not seem to have any external dependencies or side effects.
Overall, the implementation looks good and can be approved.
Описание PR
Порт маленького энерго арбалета.
Медиа
Видео
Изменения
🆑 Spatison