[Port] Surgery fixes: Diamond Fish #926
Conversation
хирургия на рыбе Co-Authored-By: deltanedas <@deltanedas:kde.org>
WalkthroughЗапрос на изменение вводит несколько улучшений и новых компонентов, связанных с респираторными и органными системами в игре. Ключевые изменения включают добавление Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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 (
|
github-actions
bot
added
the
Changes: Sprites
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (19)
Content.Shared/Body/Systems/SharedBodySystem.Parts.cs (2)
Line range hint
87-90: Refactor to includeRemovePartEffect()inRecursiveBodyUpdate()The comment indicates that
RemovePartEffect()should be implemented withinRecursiveBodyUpdate(), but it's currently omitted due to issues with debug assertions and existing tests. Refactoring the code to includeRemovePartEffect()here would enhance code maintainability and ensure that part removal effects are properly handled during recursive updates.
Line range hint
141-143: Remove unprofessional language from code commentsThe comment contains unprofessional language: "though I really just want the compiler to shut the fuck up." It's important to maintain a professional tone in code comments. Please rephrase or remove this comment to uphold codebase professionalism.
Apply this diff to address the issue:
- // I don't know if this can cause issues, since any part that's being detached HAS to have a Body. - // though I really just want the compiler to shut the fuck up. + // Ensure that any part being detached has a Body component to avoid potential issues. + // This is required to prevent compiler warnings and ensure correct behavior.Resources/Prototypes/_Backmen/status_effects.yml (1)
1-3: Consider improving the documentation commentThe current comment "#Praise him" doesn't provide meaningful information about the status effect's purpose or behavior.
Consider adding a descriptive comment explaining the effect's functionality:
- id: ZombieDisease #Praise him + id: ZombieDisease # Status effect for zombie infection mechanicsContent.Shared/Backmen/Surgery/Body/BreathingImmunityComponent.cs (1)
3-6: Consider enhancing the documentationWhile the current documentation clearly states the purpose, it could be more comprehensive by including:
- Usage examples (e.g., space carps, shadovkins)
- Related systems that interact with this component
- Whether temporary application is supported
Example enhancement:
/// <summary> -/// Backmen: Disables a mobs need for air when this component is added. -/// It will neither breathe nor take airloss damage. +/// Backmen: Disables an entity's need for air when this component is added. +/// Entities with this component will neither breathe nor take airloss damage, +/// making them suitable for space-dwelling creatures like space carps. +/// This component is typically used in conjunction with the RespiratorSystem. /// </summary>Resources/Prototypes/_Goobstation/Body/Organs/Animal/space.yml (2)
4-4: Consider using more specific organ namesGiven that these organs are specifically designed for space carps according to the PR objectives, consider renaming them to be more specific:
- "space animal lungs" → "space carp lungs"
- "space animal heart" → "space carp heart"
- name: space animal lungs + name: space carp lungs- name: space animal heart + name: space carp heartAlso applies to: 13-13
1-9: Add documentation for special organ propertiesConsider adding comments to document the special properties of these organs, making it clear to other developers why they grant immunity effects.
- type: entity parent: OrganAnimalLungs id: OrganSpaceAnimalLungs name: space animal lungs + description: Special lungs that enable breathing in space through unique biological adaptations. components: - type: StatusEffectOrgan refresh: BreathingImmunity: BreathingImmunityResources/Prototypes/_Goobstation/Body/Prototypes/Animal/carp.yml (2)
16-17: Consider adding behavioral attributes for tail.The tail is defined as a basic part, but given this is a space-dwelling creature, consider whether the tail should have special attributes or components for movement in zero gravity.
1-17: Consider adding documentation for surgery interactions.Since this PR introduces surgery features for space carps, it would be helpful to add comments documenting which organs can be operated on and any special surgical considerations.
- type: body id: Carp name: carp root: torso + # Surgery Information: + # - All organs can be surgically manipulated + # - Special care needed when operating on space-adapted organs + # - Lungs and heart require specialized surgical tools slots:Content.Server/_Shitmed/Body/Organ/StatusEffectOrganComponent.cs (3)
7-9: Consider documenting the component's purposeWhile the attributes are correctly applied, adding XML documentation to describe the component's purpose and its relationship with
StatusEffectOrganSystemwould improve maintainability.+/// <summary> +/// Component that manages status effects for organs, refreshing them at regular intervals +/// while the organ is installed in a body. +/// </summary> [RegisterComponent, Access(typeof(StatusEffectOrganSystem))] [AutoGenerateComponentPause] public sealed partial class StatusEffectOrganComponent : Component
21-22: Consider making the delay configurable per status effectThe current implementation uses a single delay for all status effects. Consider allowing per-effect delays for more granular control.
-public Dictionary<ProtoId<StatusEffectPrototype>, string> Refresh = new(); +public Dictionary<ProtoId<StatusEffectPrototype>, (string Component, TimeSpan? Delay)> Refresh = new();
1-26: Consider documenting integration with space carp surgery systemThis component plays a crucial role in the space carp surgery feature by managing organ-specific status effects. Consider adding integration tests to verify the interaction between this component and the breathing immunity system for space carps.
Would you like me to help create integration tests for the space carp surgery system?
Resources/Prototypes/_Backmen/Body/Prototypes/shadowkin.yml (1)
26-26: Consider maintaining organ naming consistencyWhile the change to
OrganSpaceAnimalLungsis functionally correct, it breaks the naming pattern where other organs use Shadowkin-specific types (e.g.,OrganShadowkinHeart,OrganShadowkinBrain). Consider either:
- Creating a Shadowkin-specific lung type that inherits from
OrganSpaceAnimalLungs- Document why this inconsistency is intentional
Content.Server/_Shitmed/Body/Organ/StatusEffectOrganSystem.cs (3)
9-10: Add initialization checks for dependencies.Consider adding null checks in Initialize() to ensure dependencies are properly injected.
+public override void Initialize() +{ + base.Initialize(); + DebugTools.Assert(_timing != null, "Game timing was not properly injected"); + DebugTools.Assert(_effects != null, "Status effects system was not properly injected"); +}
20-21: Consider adding debug logging for skipped updates.Adding debug logs when updates are skipped could help with troubleshooting.
if (now < comp.NextUpdate || organ.Body is not {} body) +{ + _sawmill.Debug($"Skipping update for organ {uid}: NextUpdate={comp.NextUpdate}, HasBody={organ.Body != null}"); continue; +}
7-7: Add documentation about supported organ types.Given that this system is part of the space carp surgery feature, consider adding documentation about the specific organ types and their associated status effects (e.g., space breathing lungs, pressure-immune hearts).
+/// <summary> +/// Manages status effects for specialized organs, including: +/// - Space carp lungs (space breathing capability) +/// - Pressure-immune hearts +/// - Other biological modifications +/// </summary> public sealed class StatusEffectOrganSystem : EntitySystemResources/Prototypes/Entities/Mobs/NPCs/carp.yml (1)
102-110: Maintain consistent comment styleThe new section uses
# start-backmen: surgerystyle comments, while other sections in the file use different styles. Consider standardizing the comment style across the file for better maintainability.Content.Server/Body/Systems/RespiratorSystem.cs (2)
77-78: Improve inline comment documentationThe breathing immunity check is well-implemented, but the comment "backmen: surgery" could be more descriptive. Consider documenting the purpose of this immunity check and its relationship to the surgery system.
Consider updating the comment to something more descriptive like:
- if (_mobState.IsDead(uid) || HasComp<BreathingImmunityComponent>(uid)) // backmen: surgery + if (_mobState.IsDead(uid) || HasComp<BreathingImmunityComponent>(uid)) // Skip respiratory processing for dead entities or those with breathing immunity (e.g., space carps)
Line range hint
99-111: Improve code organization and documentation for immunity handlingThe immunity handling logic is correct but could benefit from better organization and documentation:
- The "blob zombie" comments seem unrelated to the space carp surgery feature described in the PR objectives
- The immunity check block could be more clearly separated from the regular suffocation logic
Consider refactoring to improve clarity:
- // start-backmen: blob zombie - if (respirator.HasImmunity) + // Handle breathing immunity (e.g., space carps, special entities) + if (HasComp<BreathingImmunityComponent>(uid) || respirator.HasImmunity) { if (respirator.SuffocationCycles > 0) { StopSuffocation((uid, respirator)); respirator.SuffocationCycles = 0; } continue; } - else - // end-backmen: blob zombie + + // Handle normal breathing entitiesResources/Prototypes/_Backmen/Entities/Surgery/surgeries.yml (1)
285-301: LGTM with minor suggestions.The
SurgeryAttachTailentity follows the established pattern for attachment surgeries and is well-structured. However, consider the following points:
- The commented out
#symmetry: Noneproperty should be either removed or uncommented for consistency- Given that this is for space carps with unique biological traits, consider adding specific conditions or components to handle their special characteristics
- type: SurgeryPartRemovedCondition connection: tail part: Tail - #symmetry: None + symmetry: None # Tails are typically symmetric
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
Resources/Textures/_Goobstation/Mobs/Aliens/Carps/carp_parts.rsi/tail.pngis excluded by!**/*.pngResources/Textures/_Goobstation/Mobs/Aliens/Carps/carp_parts.rsi/torso.pngis excluded by!**/*.png
📒 Files selected for processing (14)
Content.Server/Body/Systems/RespiratorSystem.cs(1 hunks)Content.Server/_Shitmed/Body/Organ/StatusEffectOrganComponent.cs(1 hunks)Content.Server/_Shitmed/Body/Organ/StatusEffectOrganSystem.cs(1 hunks)Content.Shared/Backmen/Surgery/Body/BreathingImmunityComponent.cs(1 hunks)Content.Shared/Body/Systems/SharedBodySystem.Parts.cs(1 hunks)Resources/Prototypes/Entities/Mobs/NPCs/carp.yml(1 hunks)Resources/Prototypes/_Backmen/Body/Prototypes/carp.yml(1 hunks)Resources/Prototypes/_Backmen/Body/Prototypes/shadowkin.yml(1 hunks)Resources/Prototypes/_Backmen/Entities/Surgery/surgeries.yml(1 hunks)Resources/Prototypes/_Backmen/status_effects.yml(1 hunks)Resources/Prototypes/_Goobstation/Body/Organs/Animal/space.yml(1 hunks)Resources/Prototypes/_Goobstation/Body/Prototypes/Animal/carp.yml(1 hunks)Resources/Prototypes/status_effects.yml(1 hunks)Resources/Textures/_Goobstation/Mobs/Aliens/Carps/carp_parts.rsi/meta.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- Resources/Textures/_Goobstation/Mobs/Aliens/Carps/carp_parts.rsi/meta.json
🔇 Additional comments (15)
Resources/Prototypes/_Backmen/status_effects.yml (2)
4-7: Verify security implications of alwaysAllowed: true
While the flag is necessary for space animal lungs functionality, unrestricted application of breathing immunity could potentially be exploited.
Let's verify how this status effect is controlled:
Also, consider enhancing the documentation:
- alwaysAllowed: true # Used by space animal lungs to work on anything
+ alwaysAllowed: true # Required for space animal lungs to bypass suffocation checks. This effect should only be applied through the BreathingImmunityComponent.✅ Verification successful
The alwaysAllowed flag usage is properly controlled and safe
The implementation shows that BreathingImmunity is tightly controlled and used appropriately:
- It's only applied through space animal lungs via the StatusEffectOrgan component
- The RespiratorSystem checks for this component specifically to bypass suffocation
- The component is a simple marker with no public methods that could be exploited
The current comment is sufficient since the implementation is straightforward and the usage is limited to the intended case.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for references to BreathingImmunity to ensure it's properly controlled
echo "Searching for BreathingImmunity usage..."
rg -l "BreathingImmunity"
# Look for the component that manages this effect
echo "Checking BreathingImmunityComponent implementation..."
ast-grep --pattern 'class BreathingImmunityComponent {
$$$
}'
Length of output: 525
Script:
#!/bin/bash
# Let's examine the implementation details and usage
# Check BreathingImmunityComponent implementation
echo "=== BreathingImmunityComponent Implementation ==="
rg -p "BreathingImmunity" "Content.Shared/Backmen/Surgery/Body/BreathingImmunityComponent.cs"
# Check how it's used in RespiratorSystem
echo -e "\n=== RespiratorSystem Usage ==="
rg -p "BreathingImmunity" "Content.Server/Body/Systems/RespiratorSystem.cs"
# Check how it's defined in space.yml
echo -e "\n=== Space Animal Configuration ==="
cat "Resources/Prototypes/_Goobstation/Body/Organs/Animal/space.yml"
Length of output: 1070
8-9: Add documentation and verify implementation completeness
The Brainrotted status effect lacks documentation and configuration. Additionally, this effect isn't mentioned in the PR objectives.
Let's verify its implementation:
Consider adding documentation and necessary configuration:
- id: Brainrotted
+ id: Brainrotted
+ # Status effect for brain deterioration
+ # Add relevant configuration parameters hereContent.Shared/Backmen/Surgery/Body/BreathingImmunityComponent.cs (1)
1-8: Implementation looks good!
The component is well-structured as a marker component, following best practices with proper registration and sealing. The use of a marker component pattern is appropriate for this functionality.
Resources/Prototypes/_Goobstation/Body/Organs/Animal/space.yml (1)
6-8: Verify status effect definitions
The organs reference BreathingImmunity and PressureImmunity status effects. Let's verify these effects exist and are properly configured.
Also applies to: 15-17
✅ Verification successful
Based on the code review and verification results, I can provide a final response:
Status effect organ implementation verified
The organ references BreathingImmunity status effect and the implementation is correct. The StatusEffectOrganSystem periodically refreshes configured status effects on the body entity while the organ is installed. The organ will grant breathing immunity through this mechanism.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the required status effects are defined
# Search for BreathingImmunity status effect
echo "Checking BreathingImmunity status effect:"
rg -A 5 "BreathingImmunity:" "Resources/Prototypes"
echo -e "\nChecking PressureImmunity status effect:"
rg -A 5 "PressureImmunity:" "Resources/Prototypes"
# Check if these effects are used in the StatusEffectOrganSystem
echo -e "\nChecking StatusEffectOrganSystem implementation:"
ast-grep --pattern 'class StatusEffectOrganSystem {
$$$
}'
Length of output: 1087
Script:
#!/bin/bash
# Let's search for status effect definitions in a broader context
# Search for any status effect definitions
echo "Searching for status effect definitions:"
fd -e yml -e yaml . Resources/Prototypes --exec grep -l "StatusEffect" {} \; | xargs cat
# Search for StatusEffectOrganSystem in source files
echo -e "\nSearching for StatusEffectOrganSystem implementation:"
rg -g "*.cs" "StatusEffectOrgan" -A 10
Length of output: 321764
Resources/Prototypes/_Goobstation/Body/Prototypes/Animal/carp.yml (1)
1-4: LGTM! Entity definition looks correct.
The basic entity structure follows the expected YAML prototype format with proper indentation and required fields.
Resources/Prototypes/_Backmen/Body/Prototypes/carp.yml (2)
6-7: Verify sprite asset path consistency
The sprite path references _Goobstation directory while the file is in _Backmen. This path inconsistency might cause asset loading issues.
21-23: Review movement speed configuration
The movement speeds are defined but might not work as intended due to the hardcoded leg requirement mentioned in the TODO. Consider either:
- Implementing a custom movement system for tails
- Modifying the base movement system to support non-leg body parts
Content.Server/_Shitmed/Body/Organ/StatusEffectOrganComponent.cs (3)
1-4: LGTM: Imports are appropriate and minimal
The imports are well-organized and include only the necessary dependencies for the component's functionality.
24-25: LGTM: Proper time tracking implementation
The NextUpdate field is correctly implemented with the TimeOffsetSerializer and AutoPausedField attribute, ensuring proper serialization and pause behavior.
14-15: Verify the status effect prototypes exist
The required: true attribute ensures the dictionary is provided, but we should verify that the referenced status effect prototypes are properly defined in the game's prototype system.
Resources/Prototypes/_Backmen/Body/Prototypes/shadowkin.yml (1)
26-26: Verify the OrganSpaceAnimalLungs implementation
The change aligns with the PR objective of making Shadowkins breathe like space carps. However, we should verify that the OrganSpaceAnimalLungs prototype is properly defined and implements the required functionality.
✅ Verification successful
OrganSpaceAnimalLungs implementation is properly defined and integrated
The verification confirms that:
OrganSpaceAnimalLungsis properly defined inResources/Prototypes/_Goobstation/Body/Organs/Animal/space.yml- It grants breathing immunity through the
StatusEffectOrgancomponent withBreathingImmunityeffect - The organ is consistently used by space carps, which aligns with the PR objective
- The
BreathingImmunityComponentis properly integrated into the respiration system
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if OrganSpaceAnimalLungs is defined and implements required components
# Search for OrganSpaceAnimalLungs definition
echo "Searching for OrganSpaceAnimalLungs definition:"
rg "OrganSpaceAnimalLungs" -A 10
# Search for any references to BreathingImmunityComponent
echo -e "\nVerifying integration with BreathingImmunityComponent:"
rg "BreathingImmunityComponent"
Length of output: 3148
Resources/Prototypes/status_effects.yml (1)
39-39: Verify security implications of alwaysAllowed: true
The alwaysAllowed flag allows this status effect to be applied to any entity without restrictions. Please confirm if this is the intended behavior, as it could potentially be exploited if not properly controlled at the system level.
Let's check how other status effects with alwaysAllowed: true are used:
✅ Verification successful
alwaysAllowed: true usage appears safe and consistent with similar effects
The verification shows that alwaysAllowed: true is used consistently across similar space-survival related status effects:
PressureImmunityfor space animal heartsBreathingImmunityfor space animal lungsJitteras a general effect
The status effect system (StatusEffectsSystem.cs) handles these effects through proper API calls (TryRemoveStatusEffect), suggesting the presence of controlled application mechanisms. The usage pattern indicates this is a deliberate design choice for entity-agnostic effects rather than a security concern.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all status effects with alwaysAllowed and their usage
echo "Status effects with alwaysAllowed:"
rg "alwaysAllowed: true" -B 2
echo "\nUsage of these effects in systems:"
rg "ApplyStatus|RemoveStatus" -A 2
Length of output: 3166
Resources/Prototypes/Entities/Mobs/NPCs/carp.yml (1)
102-110: Verify required dependencies for surgery components
The surgery-related components look well-structured, but we need to verify the existence of their dependencies.
Let's verify the required components and definitions:
✅ Verification successful
Let's verify the actual content of these dependencies to ensure they're properly implemented.
Let's try one more search to find the body prototype definition and UI-related code:
Dependencies for surgery components are properly implemented
The verification shows that all required dependencies exist and are properly implemented:
- The Carp body prototype is defined in
Resources/Prototypes/_Goobstation/Body/Prototypes/Animal/carp.yml - The SurgeryBui interface is implemented in
Content.Client/Backmen/Surgery/SurgeryBui.cs - The SurgeryUIKey enum is defined in
Content.Shared/Backmen/Surgery/SurgeryUI.cs - The surgery system implementation exists in both server and shared code
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for required surgery-related definitions
# Check for Carp body prototype definition
echo "Checking for Carp body prototype..."
rg -l "prototype: Carp" --type yaml
# Check for SurgeryBui interface implementation
echo "Checking for SurgeryBui interface..."
rg -l "SurgeryBui" --type cs
# Check for SurgeryUIKey enum definition
echo "Checking for SurgeryUIKey enum..."
rg -l "enum SurgeryUIKey" --type cs
# Check for breathing immunity component mentioned in AI summary
echo "Checking for BreathingImmunityComponent..."
rg -l "BreathingImmunityComponent" --type cs
Length of output: 884
Script:
#!/bin/bash
# Check the Carp body prototype definition in carp.yml
echo "=== Carp Body Prototype Definition ==="
rg "prototype: Carp" -B 5 -A 5 --type yaml
# Check SurgeryBui implementation
echo -e "\n=== SurgeryBui Implementation ==="
ast-grep --pattern 'class SurgeryBui {
$$$
}'
# Check SurgeryUIKey enum definition
echo -e "\n=== SurgeryUIKey Enum Definition ==="
ast-grep --pattern 'enum SurgeryUIKey {
$$$
}'
# Check if the Body component with Carp prototype is properly referenced
echo -e "\n=== Body Component References ==="
rg "type: Body.*prototype: Carp" --type yaml -C 2
Length of output: 9722
Script:
#!/bin/bash
# Look for Carp body prototype definition in a different way
echo "=== Searching for Carp Body Definition ==="
rg "^- type: body" -B 2 -A 5 --type yaml -i
# Check for the SurgeryBui class definition with a different pattern
echo -e "\n=== Searching for SurgeryBui Class ==="
rg "class SurgeryBui" -A 5 --type cs
# Look for SurgeryUIKey enum definition
echo -e "\n=== Searching for SurgeryUIKey Definition ==="
rg "SurgeryUIKey" -A 5 --type cs
# Look for body prototypes directory
echo -e "\n=== Checking Body Prototypes Location ==="
fd "body" --type directory
Length of output: 24339
Content.Server/Body/Systems/RespiratorSystem.cs (1)
Line range hint 77-111: Verify integration with related systems
The breathing immunity implementation looks sound, but please verify its interaction with:
- Damage systems (ensure immune entities don't take unexpected damage)
- Status effect systems (confirm proper handling of status effects)
- UI/Alert systems (verify proper display of breathing status)
✅ Verification successful
Let me gather more specific information about the breathing-related systems.
Implementation verified and properly integrated
The breathing immunity implementation is correctly integrated with all related systems:
-
Damage System: The code properly handles immunity by checking
HasComp<BreathingImmunityComponent>before any suffocation processing, and explicitly stops suffocation (StopSuffocation) when immunity is present. -
Status Effects: The implementation correctly maintains the breathing cycle states (Inhaling/Exhaling) and properly manages the
SuffocationCyclescounter, resetting it to 0 when immunity is active. -
UI/Alerts: The alerts system (from alerts.yml) has proper breathing status indicators that will naturally work with the immunity system since they rely on the same underlying respiratory state.
The implementation follows existing patterns and has appropriate test coverage as seen in LungTest.cs and SpecForceTest.cs.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential interactions with other systems
# Look for damage system interactions
rg -l "TakeDamage.*Suffocation"
# Check for breathing-related status effects
rg -l "StatusEffect.*[Bb]reath"
# Search for alert UI components related to breathing
rg -l "Alert.*[Bb]reath"
Length of output: 133
Script:
#!/bin/bash
# Check the alerts.yml content for breathing-related alerts
cat Resources/Prototypes/Alerts/alerts.yml | grep -A 5 "[Bb]reath"
# Look for suffocation damage implementation
rg -g "*.cs" "Suffocation" -C 2
# Check for breathing immunity component usage
ast-grep --pattern 'HasComp<BreathingImmunityComponent>'
# Look for other respiratory status checks
rg -g "*.cs" "RespiratorStatus" -C 2
Length of output: 11133
Resources/Prototypes/_Backmen/Entities/Surgery/surgeries.yml (1)
285-301: Consider architectural improvements for space carp specifics.
The current implementation handles basic tail attachment, but doesn't account for the unique biological traits mentioned in the PR objectives (special lungs for space breathing, pressure-immune heart). Consider:
- Adding specific organ conditions for space carp organs
- Implementing validation checks for compatibility with space-breathing capabilities
- Adding specific steps for handling the unique biological connections
| torso: | ||
| part: TorsoAnimal | ||
| connections: | ||
| - tail | ||
| organs: | ||
| lungs: OrganSpaceAnimalLungs # Immunity to airloss | ||
| stomach: OrganAnimalStomach | ||
| liver: OrganAnimalLiver | ||
| heart: OrganSpaceAnimalHeart # Immunity to cold | ||
| kidneys: OrganAnimalKidneys |
There was a problem hiding this comment.
💡 Codebase verification
Organ implementations and immunities are correctly defined
The verification shows that:
OrganSpaceAnimalHeartcorrectly providesPressureImmunity(not cold immunity as mentioned in the comment)OrganSpaceAnimalLungscorrectly providesBreathingImmunity
Both organs are properly implemented in Resources/Prototypes/_Goobstation/Body/Organs/Animal/space.yml with their respective immunity components. The comment about "Immunity to cold" in the carp prototype is incorrect and should be updated to "Immunity to pressure changes" to match the actual implementation.
🔗 Analysis chain
Verify organ implementations and immunities.
The organ definitions align with the PR objectives for space carps, but there are some points to verify:
- The comment mentions "Immunity to cold" for the heart, but PR objectives mentioned "immunity to pressure changes"
- Need to verify if
OrganSpaceAnimalLungsandOrganSpaceAnimalHeartare properly implemented
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the organ implementations exist and check their properties
# Check for organ definitions
echo "Checking for organ prototype definitions..."
rg -l "OrganSpaceAnimalLungs|OrganSpaceAnimalHeart" --type yaml
# Check for immunity implementations
echo "Checking for immunity implementations..."
ast-grep --pattern 'class $name : IComponent {
$$$
Immunity$_
$$$
}'
Length of output: 519
Script:
#!/bin/bash
# Let's check the content of the organ definitions file and any related immunity implementations
# Check the space organ definitions
echo "=== Space Animal Organs Definitions ==="
cat "Resources/Prototypes/_Goobstation/Body/Organs/Animal/space.yml"
# Search for pressure and cold immunity related code
echo -e "\n=== Pressure and Cold Immunity Related Code ==="
rg -i "pressure|cold|temperature" "Resources/Prototypes/_Goobstation/Body/Organs/Animal/space.yml" -C 2
# Check for any related component definitions
echo -e "\n=== Related Component Definitions ==="
rg -i "class.*organ.*space|immunity" --type cs -C 2
Length of output: 20996
| name: carp tail | ||
| description: Unique glands in this tail let space carp fly in a vacuum. |
There was a problem hiding this comment.
Implementation needed for space flight capability
The description promises vacuum flight capabilities, but the TODOs indicate this feature is not implemented yet. This could lead to misleading gameplay expectations.
Would you like help implementing the space flight functionality? I can help create a new component for vacuum movement.
Also applies to: 24-25
| id: TorsoCarp | ||
| name: carp torso | ||
| components: | ||
| - type: Sprite | ||
| layers: | ||
| - state: torso | ||
| - type: BodyPart | ||
| partType: Torso |
There was a problem hiding this comment.
Add missing biological components for space carp
According to the PR objectives, space carps should have special lungs for space breathing and a heart providing pressure immunity. Consider adding:
- Breathing immunity component
- Pressure immunity component
- Special organ components for lungs and heart
Example addition:
- type: BodyPart
partType: Torso
+ - type: BreathingImmunity
+ - type: PressureImmunity
+ - type: StatusEffectOrgan
organType: SpaceLungsCommittable suggestion skipped: line range outside the PR's diff.
| public override void Update(float frameTime) | ||
| { | ||
| base.Update(frameTime); | ||
|
|
||
| var query = EntityQueryEnumerator<StatusEffectOrganComponent, OrganComponent>(); | ||
| var now = _timing.CurTime; | ||
| while (query.MoveNext(out var uid, out var comp, out var organ)) | ||
| { | ||
| if (now < comp.NextUpdate || organ.Body is not {} body) | ||
| continue; | ||
|
|
||
| comp.NextUpdate = now + comp.Delay; | ||
| if (!TryComp<StatusEffectsComponent>(body, out var effects)) | ||
| continue; | ||
|
|
||
| foreach (var (key, component) in comp.Refresh) | ||
| { | ||
| _effects.TryAddStatusEffect(body, key, comp.Delay, refresh: true, component, effects); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider performance optimization and error handling improvements.
Several suggestions to improve the implementation:
- Consider adding error handling for status effect application
- Add XML documentation to explain the system's purpose and behavior
- Consider using a timer instead of checking every frame if the delay is significant
Here's a suggested improvement:
+/// <summary>
+/// Handles the application and refresh of status effects for organs installed in bodies.
+/// This system runs on every frame to ensure timely updates of organ-related effects.
+/// </summary>
public sealed class StatusEffectOrganSystem : EntitySystem
{
[Dependency] private readonly IGameTiming _timing = default!;
[Dependency] private readonly StatusEffectsSystem _effects = default!;
+ private readonly ILogger _logger = LogManager.GetCurrentClassLogger();
public override void Update(float frameTime)
{
base.Update(frameTime);
var query = EntityQueryEnumerator<StatusEffectOrganComponent, OrganComponent>();
var now = _timing.CurTime;
while (query.MoveNext(out var uid, out var comp, out var organ))
{
if (now < comp.NextUpdate || organ.Body is not {} body)
continue;
comp.NextUpdate = now + comp.Delay;
if (!TryComp<StatusEffectsComponent>(body, out var effects))
continue;
foreach (var (key, component) in comp.Refresh)
{
- _effects.TryAddStatusEffect(body, key, comp.Delay, refresh: true, component, effects);
+ try
+ {
+ if (!_effects.TryAddStatusEffect(body, key, comp.Delay, refresh: true, component, effects))
+ {
+ _logger.Warning($"Failed to apply status effect {key} to body {body}");
+ }
+ }
+ catch (Exception e)
+ {
+ _logger.Error($"Error applying status effect {key} to body {body}: {e}");
+ }
}
}
}
}📝 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.
| public override void Update(float frameTime) | |
| { | |
| base.Update(frameTime); | |
| var query = EntityQueryEnumerator<StatusEffectOrganComponent, OrganComponent>(); | |
| var now = _timing.CurTime; | |
| while (query.MoveNext(out var uid, out var comp, out var organ)) | |
| { | |
| if (now < comp.NextUpdate || organ.Body is not {} body) | |
| continue; | |
| comp.NextUpdate = now + comp.Delay; | |
| if (!TryComp<StatusEffectsComponent>(body, out var effects)) | |
| continue; | |
| foreach (var (key, component) in comp.Refresh) | |
| { | |
| _effects.TryAddStatusEffect(body, key, comp.Delay, refresh: true, component, effects); | |
| } | |
| } | |
| } | |
| /// <summary> | |
| /// Handles the application and refresh of status effects for organs installed in bodies. | |
| /// This system runs on every frame to ensure timely updates of organ-related effects. | |
| /// </summary> | |
| public sealed class StatusEffectOrganSystem : EntitySystem | |
| { | |
| [Dependency] private readonly IGameTiming _timing = default!; | |
| [Dependency] private readonly StatusEffectsSystem _effects = default!; | |
| private readonly ILogger _logger = LogManager.GetCurrentClassLogger(); | |
| public override void Update(float frameTime) | |
| { | |
| base.Update(frameTime); | |
| var query = EntityQueryEnumerator<StatusEffectOrganComponent, OrganComponent>(); | |
| var now = _timing.CurTime; | |
| while (query.MoveNext(out var uid, out var comp, out var organ)) | |
| { | |
| if (now < comp.NextUpdate || organ.Body is not {} body) | |
| continue; | |
| comp.NextUpdate = now + comp.Delay; | |
| if (!TryComp<StatusEffectsComponent>(body, out var effects)) | |
| continue; | |
| foreach (var (key, component) in comp.Refresh) | |
| { | |
| try | |
| { | |
| if (!_effects.TryAddStatusEffect(body, key, comp.Delay, refresh: true, component, effects)) | |
| { | |
| _logger.Warning($"Failed to apply status effect {key} to body {body}"); | |
| } | |
| } | |
| catch (Exception e) | |
| { | |
| _logger.Error($"Error applying status effect {key} to body {body}: {e}"); | |
| } | |
| } | |
| } | |
| } | |
| } |
| using Content.Shared.StatusEffect; | ||
| using Robust.Shared.Timing; | ||
|
|
||
| namespace Content.Server._Shitmed.Body.Organ; |
There was a problem hiding this comment.
💡 Codebase verification
The namespace convention needs to be aligned with existing patterns
The codebase follows a clear namespace convention for medical and body-related functionality:
- Medical features use
Content.Server.Medical - Body systems use
Content.Server.Body.Systems - Body components use
Content.Server.Body.Components
The current _Shitmed namespace deviates from these established patterns. This code should be moved to either:
Content.Server.Medical.Organsfor medical-specific organ functionalityContent.Server.Body.Systemsif it's a core body system
🔗 Analysis chain
Consider revising the namespace convention.
The underscore prefix in _Shitmed appears non-standard. Consider using a more descriptive and conventional namespace name that better reflects the module's purpose (e.g., Surgery, Medical, or Organs).
Let me gather more information about the namespace conventions in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for similar namespace patterns in the codebase
rg "namespace Content\.Server\._" -A 1
Length of output: 391
Script:
#!/bin/bash
# Check for other medical/surgery related namespaces in the codebase
rg "namespace Content\.Server\.(Medical|Surgery|Body)" -A 1
# Check if there are any other directories with underscore prefix
fd -t d "^_" --base-directory Content.Server
Length of output: 7394
| # start-backmen: surgery | ||
| - type: Body | ||
| prototype: Carp | ||
| - type: SurgeryTarget | ||
| - type: UserInterface | ||
| interfaces: | ||
| enum.SurgeryUIKey.Key: | ||
| type: SurgeryBui | ||
| # end-backmen: surgery |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding components for unique biological traits
The PR objectives mention that space carps have special lungs for breathing in space and hearts providing immunity to pressure changes. Consider adding components to represent these unique biological traits:
Add these components after the surgery section:
# end-backmen: surgery
+ # start-backmen: biological-traits
+ - type: BreathingImmunity
+ reason: SpaceLungs
+ - type: PressureImmunity
+ reason: SpaceHeart
+ # end-backmen: biological-traitsThis will better reflect the unique biological characteristics described in the PR objectives and provide proper gameplay mechanics for these traits.
Committable suggestion skipped: line range outside the PR's diff.
|
Рыбоебы реальны 💀 |
|
Надо ещё перевести некоторые штуки и сбалансировать космическое сердце, мне кажется порезать карпа чтобы больше никогда не носить скафы слишком имбово |
github-actions
bot
added
the
Changes: Localization
|
@Rxup ревью пжлст |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
Resources/Locale/ru-RU/backmen/surgery/surgery-items.ftl (1)
38-39: Consider adding descriptions for carp body parts.While the translations are correct, consider adding
.descfields to maintain consistency with other entries and provide players with additional context about these unique space carp parts.Example additions:
ent-TorsoCarp = туловище карпа + .desc = Туловище космического карпа, обладающее уникальными биологическими свойствами. ent-TailCarp = плавник карпа + .desc = Плавник космического карпа, приспособленный для движения в космическом пространстве.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
Resources/Locale/ru-RU/backmen/surgery/surgery-items.ftl(1 hunks)Resources/Locale/ru-RU/backmen/surgery/surgery-operations.ftl(1 hunks)
🔇 Additional comments (3)
Resources/Locale/ru-RU/backmen/surgery/surgery-items.ftl (1)
35-36: LGTM! Translations for space animal organs are well-formed.
The translations maintain consistency with existing organ terminology while accurately representing the special nature of these organs as described in the PR objectives.
Resources/Locale/ru-RU/backmen/surgery/surgery-operations.ftl (2)
46-46: LGTM! The new tail attachment localization entry is well-integrated.
The format and placement are consistent with other attachment operations in the file.
46-46: Consider adding a corresponding tail removal operation.
For consistency with other body parts that have both attachment and removal operations, consider adding:
ent-SurgeryAttachTail = Прикрепить хвост
+ent-SurgeryRemoveTail = Удалить хвостLet's verify if a removal operation exists in the codebase:
BreathingImmunityComponent - у нас есть респератор имунити, дубликат функционала? |
Видимо так, поправлю это |
фиксед |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
Content.Server/Backmen/Blob/ZombieBlobSystem.cs (3)
Line range hint
73-81: Consider documenting special respiratory behaviorThe implementation overrides normal breathing mechanics with custom gas mixtures. This appears to implement the special breathing capabilities mentioned in the PR objectives.
Consider adding XML documentation to explain the special respiratory behavior:
+/// <summary> +/// Overrides inhaled gas mixture with normal atmosphere regardless of environment. +/// This implements special breathing capabilities for blob entities. +/// </summary> private void OnInhale(Entity<ZombieBlobComponent> ent, ref InhaleLocationEvent args) { args.Gas = _normalAtmos; }
Line range hint
83-98: Add error handling for respirator component operationsThe respirator immunity handlers should include error handling for cases where the respirator component might be missing.
Consider this safer implementation:
private void OnPressureImmuneInit(EntityUid uid, RespiratorImmunityComponent pressureImmunity, ComponentInit args) { - if (TryComp<RespiratorComponent>(uid, out var respirator)) + if (!TryComp<RespiratorComponent>(uid, out var respirator)) + { + Log.Warning($"Entity {ToPrettyString(uid)} has RespiratorImmunityComponent but no RespiratorComponent"); + return; + } { respirator.HasImmunity = true; } }
Line range hint
122-196: Refactor OnStartup method to improve maintainabilityThe OnStartup method is handling too many responsibilities, making it harder to maintain and test. Consider breaking it down into smaller, focused methods.
Consider this restructuring:
private void OnStartup(EntityUid uid, ZombieBlobComponent component, ComponentStartup args) { + InitializeInventory(uid); + InitializeComponents(uid); + HandleFactions(uid, component); + SetupTemperature(uid, component); + SetupFixtures(uid, component); + InitializeMindOrNPC(uid, component); } +private void InitializeInventory(EntityUid uid) +{ + _ui.CloseUis(uid); + _inventory.TryUnequip(uid, "underpants", true, true); + // ... other unequip operations +} +private void InitializeComponents(EntityUid uid) +{ + EnsureComp<BlobMobComponent>(uid); + EnsureComp<BlobSpeakComponent>(uid); + // ... other component operations +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
Content.Server/Backmen/Blob/ZombieBlobSystem.cs(1 hunks)Content.Server/Body/Systems/RespiratorSystem.cs(0 hunks)Resources/Prototypes/_Goobstation/Body/Organs/Animal/space.yml(1 hunks)
💤 Files with no reviewable changes (1)
- Content.Server/Body/Systems/RespiratorSystem.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- Resources/Prototypes/_Goobstation/Body/Organs/Animal/space.yml
🔇 Additional comments (2)
Content.Server/Backmen/Blob/ZombieBlobSystem.cs (2)
Line range hint 198-270: Clean implementation of shutdown and state change handling
The cleanup logic is thorough and properly handles component removal, state restoration, and entity death.
20-20: Verify integration with the new surgery system
The addition of respiratory event handlers and surgery-related imports aligns with the PR objectives. However, we should ensure proper integration with the new space carp surgery features.
Also applies to: 67-71
✅ Verification successful
Import is correctly used and integrated
The import Content.Shared.Backmen.Surgery.Body is properly integrated with the respiratory system implementation in ZombieBlobSystem. The file correctly handles inhale/exhale events and shows proper integration with both the surgery and respiratory systems. The extensive presence of surgery-related files in the codebase and the implementation of respiratory events in this specific file confirms the proper usage of the imported namespace.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for related surgery system implementations
ast-grep --pattern 'class $_ {
$$$
OnInhale($_, ref InhaleLocationEvent $_) {
$$$
}
$$$
}'
Length of output: 104
Script:
#!/bin/bash
# Let's check for any usage of surgery-related components or systems
rg -l "Surgery" --type cs
# Check for any respiratory system implementations
rg -l "Inhale|Respiratory|Breathing" --type cs
# Check the specific file for surgery-related method implementations
rg -A 5 "Surgery|Inhale|Respiratory|Breathing" "Content.Server/Backmen/Blob/ZombieBlobSystem.cs"
Length of output: 7062
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai update Summary |
|
@Rxup: I will update the summary. ✅ Actions performedSummary regeneration triggered. |
Они сделали хирургию на рыбе...
Теперь вы можете делать хирургию на карпах! Они носят в себе крутые лёгкие и сердца, которые дают необычные способности.
🆑 Deltanedas & Rouden
- tweak: Шадовкины снова перестали дышать (но теперь не задыхаются) - вместо человеческих лёгких они имеют лёгкие как у карпов.Summary by CodeRabbit
Новые функции
BreathingImmunityиBrainrotted.StatusEffectOrganComponentдля управления статусными эффектами, связанными с органами.SurgeryAttachTailдля прикрепления хвостов в ходе хирургии.RespiratorSystemдля предотвращения удушья у сущностей с иммунитетом.space carpновыми компонентами и механикой хирургии.OrganSpaceAnimalLungsиOrganSpaceAnimalHeartдля улучшения функциональности органов животных.Исправления ошибок
Документация