[Fix] Borg and Golem LayDown #906
Conversation
KayzelW
added
Require Testing
|
@trest100 ты тестил борга после удара молнии?(от аномалии) |
Нет, после учебы протещю |
WalkthroughThe changes introduce enhancements to the mechanics of standing and lying down for entities, particularly those with a Changes
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 (
|
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
KayzelW
removed
DONT MERGE
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (6)
Resources/Prototypes/Entities/Mobs/Cyborgs/base_borg_chassis.yml (2)
Line range hint
241-244: Consider separating unrelated changes.The addition of heat damage protection appears unrelated to the PR's primary objective of implementing lying down functionality. While the feature is well-documented and potentially useful, consider moving it to a separate PR to maintain focused changes.
Line range hint
236-252: Recommend splitting changes into separate PRs.While the LayingDown component addition successfully implements the PR's primary objective, this PR includes several unrelated changes:
- Heat damage protection
- Language capabilities
- Step trigger protection
Consider splitting these features into separate PRs to maintain focused changes and simplify the review process.
Resources/Prototypes/_Backmen/Entities/Mobs/Species/golem.yml (2)
8-8: LGTM! Consider standardizing the comment style.The LayingDown component addition successfully implements the lying down functionality for golems. However, the comment style
# backmen: LayingDownseems non-standard.Consider removing or standardizing the comment as it appears to be a temporary note:
- - type: LayingDown # backmen: LayingDown + - type: LayingDown
Line range hint
1-524: Consider documenting the golem variants' gameplay balance.The implementation introduces three distinct golem variants with unique characteristics:
- Cult Golem: Basic variant with repair mechanics
- Wood Golem: Nature-themed with unique vulnerabilities
- Silver Golem: High-mobility combat variant
Consider documenting the intended gameplay balance and interactions between these variants in a design document.
Content.Shared/Backmen/Standing/SharedLayingDownSystem.cs (1)
87-88: Improve code documentationThe comment "no legs bro" could be more professional. Consider:
- args.Cancel(); // no legs bro + args.Cancel(); // Entity lacks required number of legs for standingContent.Shared/Body/Systems/SharedBodySystem.Body.cs (1)
18-19: Consider removing unused importThe
Content.Shared.Rejuvenateimport appears to be unused in the visible changes. Consider removing it if it's not needed elsewhere in the file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
Content.Shared/Backmen/Standing/SharedLayingDownSystem.cs(2 hunks)Content.Shared/Body/Systems/SharedBodySystem.Body.cs(2 hunks)Resources/Prototypes/Entities/Mobs/Cyborgs/base_borg_chassis.yml(1 hunks)Resources/Prototypes/_Backmen/Entities/Mobs/Species/golem.yml(1 hunks)
🔇 Additional comments (11)
Resources/Prototypes/Entities/Mobs/Cyborgs/base_borg_chassis.yml (4)
236-236: LGTM: LayingDown component addition matches PR objectives.
The addition of the LayingDown component to BaseBorgChassis enables the lying down functionality as intended.
237-239: LGTM: GuideHelp component improves documentation.
The addition of GuideHelp with relevant guide references enhances user documentation accessibility.
Line range hint 240-240: Verify the necessity of ProtectedFromStepTriggers component.
Please clarify why this component is needed. Is it related to preventing trigger effects while the Borg is lying down?
✅ Verification successful
Based on the search results, I can generate the final response as I have enough context about the usage of ProtectedFromStepTriggers component across the codebase. It's commonly used in:
- Silicon-based entities (robots, cyborgs, etc.)
- Various protective suits and armor
- Certain special mob types (animals, elementals)
The ProtectedFromStepTriggers component is appropriate for cyborgs.
This component is consistently used across silicon-based entities in the codebase, including the base silicon entity and other robotic entities. It's a standard protection mechanism for synthetic beings, likely preventing them from triggering certain floor-based effects that would normally affect organic entities.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other entities using ProtectedFromStepTriggers to understand its typical usage
rg "type: ProtectedFromStepTriggers" -A 5 -B 5
Length of output: 22102
Line range hint 245-252: Consider separating language-related changes.
The addition of language components appears unrelated to the PR's primary objective. While the language configuration is well-structured, consider:
- Moving these changes to a separate PR
- Verifying if these capabilities already exist through other components
✅ Verification successful
Based on the search results, I can now generate the final response:
Language components are consistent with existing patterns
The added language configuration (LanguageSpeaker and LanguageKnowledge) follows the established pattern used across multiple mob entities in the codebase, including other robotic/silicon entities like MMIs, PAIs, and other cyborgs. The Binary and TauCetiBasic language combination is specifically standard for silicon-based entities.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for existing language-related components in Borg entities
rg "type: (LanguageSpeaker|LanguageKnowledge)" -A 5 -B 5
# Search for Speech and UnblockableSpeech components to understand existing communication capabilities
ast-grep --pattern 'type: Speech
$$$
type: UnblockableSpeech'
Length of output: 81670
Resources/Prototypes/_Backmen/Entities/Mobs/Species/golem.yml (2)
Line range hint 391-394: Verify the wood golem's fire resistance mechanics.
The wood golem is configured as both fire-spreading and fire-resistant, which seems contradictory. Additionally, the heat damage multiplier might make the golem too vulnerable to fire.
✅ Verification successful
Based on the search results, I can now generate the final response since I have enough information about fire mechanics in wooden entities across the codebase:
The wood golem's fire configuration is consistent with game mechanics
The wood golem's configuration of having both fireSpread: true and canResistFire: true is actually a valid combination in the codebase's design. The heat damage multiplier of 1.2 is also reasonable because:
- The
fireSpreadproperty indicates the entity can propagate fire, which is consistent with its wooden nature - The
canResistFireproperty doesn't make it immune, but rather gives it some resistance while still taking the configured heat damage - The heat damage multiplier of 1.2 is relatively modest compared to other flammable entities, balancing its fire-spreading nature with some durability
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other wooden entities to compare fire resistance values
rg -l "damageModifierSet: Wood" | xargs rg "canResistFire|fireSpread"
# Check for fire-related damage modifiers
rg "damageModifierSet: Wood" -A 10
Length of output: 44899
Line range hint 451-453: Review the silver golem's movement speed values.
The movement speed values (walk: 4, sprint: 5.5) are significantly higher than the base golem's values (walk: 1.8, sprint: 3). This might create balance issues.
Content.Shared/Backmen/Standing/SharedLayingDownSystem.cs (3)
19-19: LGTM: Required dependency for Borg functionality
The addition of the Borg components namespace aligns with the PR objective to implement lying down for Borgs.
87-88: Verify standing state transitions for Borgs
The leg check bypass for Borgs should be tested in various scenarios to ensure proper state transitions:
- Standing up from lying down
- Forced state changes (e.g., from external effects)
- Interaction with buckled state
Let's check for standing state handling:
✅ Verification successful
Standing state transitions for Borgs are correctly handled
The code review shows that Borg standing transitions are properly implemented:
- Borgs are explicitly exempted from leg requirements in
OnCheckLegs - Standing state system handles all transitions through proper events and validations
- No Borg-specific restrictions are placed on standing/lying transitions beyond the leg check bypass
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for standing state transitions in Borg-related code
ast-grep --pattern 'class $_ {
$$$
BorgChassisComponent $_ {
$$$
StandingState $$$
}
}'
Length of output: 103
Script:
#!/bin/bash
# Let's search for standing state transitions and Borg-related code more broadly
rg -l "StandingState" .
# Then search for BorgChassis usage
rg -l "BorgChassis" .
# And specifically look for standing-related methods that might affect Borgs
rg "Stand.*Borg" -A 3
Length of output: 3587
Script:
#!/bin/bash
# Let's examine the SharedLayingDownSystem.cs more closely
cat "./Content.Shared/Backmen/Standing/SharedLayingDownSystem.cs"
# And check the BorgChassisComponent for any standing-related logic
cat "./Content.Shared/Silicons/Borgs/Components/BorgChassisComponent.cs"
# Also check StandingStateSystem for any Borg-specific handling
cat "./Content.Shared/Standing/StandingStateSystem.cs"
Length of output: 22306
87-87: Verify edge cases for Borg standing mechanics
While the bypass for Borg leg requirements is logical, we should consider:
- How does this interact with damaged Borgs?
- Should there be similar handling for Golem entities (mentioned in PR objectives)?
Let's check for any existing damage-related conditions for Borgs:
✅ Verification successful
Borg standing mechanics implementation is appropriate
The current implementation is correct and well-considered:
- Damage handling for Borgs is properly managed through a separate relay system that doesn't interfere with standing mechanics
- No special standing mechanics exist for Golems, so no additional checks are needed
- The leg bypass for Borgs is a clean solution that doesn't introduce edge cases, as Borgs use a different damage system altogether
The informal comment can be addressed separately, but the core logic is sound and doesn't require modifications.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for damage handling in Borg-related code
rg -l "BorgChassisComponent.*Damage" || rg -l "Damage.*BorgChassisComponent"
Length of output: 94
Script:
#!/bin/bash
# Let's check the damage handling implementation in the Borg relay file
rg -A 10 "BorgChassisComponent.*Damage|Damage.*BorgChassisComponent" Content.Shared/Silicons/Borgs/SharedBorgSystem.Relay.cs
# Also search for any Golem-related standing mechanics
rg -l "Golem.*Stand|Stand.*Golem"
# Let's also check the broader context of standing mechanics
ast-grep --pattern 'class SharedLayingDownSystem {
$$$
private void OnCheckLegs($$$) {
$$$
}
$$$
}'
Length of output: 677
Content.Shared/Body/Systems/SharedBodySystem.Body.cs (2)
144-144: Implementation for Golems is missing
While the changes successfully implement the lying down functionality for Borgs, the PR objectives also mention Golems ("Големы могут лежать!"). Consider adding similar handling for Golem entities or clarify if this will be addressed in a separate PR.
Let's check if there's any existing Golem-related code that might need similar changes:
144-144: Verify impact on related systems
The logic to allow Borgs to stand without legs looks correct. However, we should verify that this change doesn't have unintended consequences on other systems that might depend on the standing mechanics.
Let's check for other systems that might be affected by this change:
✅ Verification successful
Let me check the implementation details of the standing mechanics to ensure there are no unintended consequences.
Based on the search results, I can now generate the final response as I have enough information about the standing mechanics and their interactions:
Standing mechanics change for Borgs is properly implemented
The change to allow Borgs to stand without legs is consistent with the codebase's standing mechanics implementation. The verification shows:
- The standing attempt event is properly handled across multiple systems (Stunnable, Buckle, Body)
- The same BorgChassisComponent check is consistently implemented in SharedLayingDownSystem
- No other systems impose leg requirements that would conflict with this change
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for systems that might interact with standing mechanics
rg -l "StandAttemptEvent|LayingDownComponent|BorgChassisComponent" --type cs
# Search for tests related to standing mechanics
rg -l "StandAttempt.*Test" --type cs
# Look for other methods that check leg count
ast-grep --pattern 'LegEntities.Count'
Length of output: 1269
Script:
#!/bin/bash
# Check standing mechanics implementation
rg -A 10 "OnStandAttempt" Content.Shared/Standing/StandingStateSystem.cs
# Check if there are any other conditions related to standing in other systems
rg -A 5 "StandAttemptEvent" --type cs
# Check borg-specific standing logic
rg -A 5 "Stand" Content.Shared/Silicons/Borgs/SharedBorgSystem.cs
Length of output: 6625
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
Content.Shared/Body/Systems/SharedBodySystem.Body.cs (1)
18-19: Consider removing unused importThe
Content.Shared.Rejuvenatenamespace appears to be unused in the visible changes.-using Content.Shared.Rejuvenate;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
Content.Shared/Backmen/Standing/SharedLayingDownSystem.cs(2 hunks)Content.Shared/Body/Systems/SharedBodySystem.Body.cs(2 hunks)Resources/Prototypes/Entities/Mobs/Cyborgs/base_borg_chassis.yml(1 hunks)Resources/Prototypes/_Backmen/Entities/Mobs/Species/golem.yml(1 hunks)
🔇 Additional comments (11)
Resources/Prototypes/Entities/Mobs/Cyborgs/base_borg_chassis.yml (3)
Line range hint 242-245: Verify heat protection values through testing
The flat heat reduction of 10 allows interaction with hot objects. Please verify this value through testing with:
- Light bulbs (typical heat damage)
- Stoves/ovens (higher heat damage)
- Other heat sources in the game
This ensures the protection is sufficient but not overpowered.
236-236: LGTM: LayingDown component properly integrated
The addition of the LayingDown component aligns with the PR objective and works in conjunction with the existing StandingState component.
Let's verify the integration with the standing system:
✅ Verification successful
LayingDown component properly integrated with StandingState
The verification shows that both LayingDown and StandingState components are present in the base_borg_chassis.yml file, and this combination is consistently used across other mob entities like base species, golems, and NPCs. This pattern indicates proper integration with the standing/lying system.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for proper integration with standing/laying system
rg -l "LayingDown.*StandingState|StandingState.*LayingDown" --type yaml
Length of output: 73
Script:
#!/bin/bash
# Let's check for files containing both components separately
echo "=== Files with LayingDown ==="
rg -l "LayingDown" --type yaml
echo -e "\n=== Files with StandingState ==="
rg -l "StandingState" --type yaml
# Also check the content to see how they're used
echo -e "\n=== LayingDown usage context ==="
rg "LayingDown" --type yaml -C 2
echo -e "\n=== StandingState usage context ==="
rg "StandingState" --type yaml -C 2
Length of output: 9095
Line range hint 236-250: Verify security implications for Syndicate borg lying down
Since Syndicate borgs have MovementAlwaysTouching and different faction membership, please verify that the lying down mechanic:
- Cannot be exploited for unintended movement
- Works correctly with their unique movement system
- Doesn't interfere with their special abilities
Also applies to: 317-341
✅ Verification successful
No security concerns with Syndicate borg lying down mechanics
Based on the LayingDown system implementation, there are no security implications specific to Syndicate borgs:
- The lying down system properly handles movement speed modifications through
RefreshMovementSpeedModifiersEvent - Standing up has proper safety checks including collision detection and mob state validation
- The system works independently of the MovementAlwaysTouching component
- Borgs have special handling to ignore leg requirements for standing via the
OnCheckLegsmethod
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential movement system conflicts
rg -A 5 "MovementAlwaysTouching.*LayingDown|LayingDown.*MovementAlwaysTouching" --type yaml
# Check for any special handling of Syndicate borgs in movement systems
ast-grep --pattern 'class $_ {
$$$
MovementAlwaysTouching($_) {
$$$
}
$$$
}'
Length of output: 184
Script:
#!/bin/bash
# Check for LayingDown component implementation and interactions
rg -l "LayingDown" --type cs
# Check for movement-related components in the Syndicate borg yaml
rg -A 5 "MovementAlwaysTouching|LayingDown" Resources/Prototypes/Entities/Mobs/Cyborgs/base_borg_chassis.yml
# Look for any special movement handling in the codebase
ast-grep --pattern 'class $_ {
$$$
public void Update($_) {
$$$
LayingDown
$$$
}
$$$
}'
Length of output: 1044
Script:
#!/bin/bash
# Check LayingDown system implementation for any movement-related code
cat Content.Shared/Backmen/Standing/SharedLayingDownSystem.cs
# Check if there are any special movement restrictions or handlers
rg -A 5 "MovementAlwaysTouching" Content.Shared/Backmen/Standing/SharedLayingDownSystem.cs Content.Server/Backmen/Standing/LayingDownSystem.cs
# Look for any movement speed modifiers
ast-grep --pattern 'class SharedLayingDownSystem {
$$$
public void $_($_) {
$$$
Movement
$$$
}
$$$
}'
Length of output: 13642
Resources/Prototypes/_Backmen/Entities/Mobs/Species/golem.yml (3)
Line range hint 271-272: Consider edge cases for flammable wooden golem
The Flammable component on MobGolemWood might need special handling when the golem is lying down. Consider if the fire spread mechanics should behave differently in this state.
Let's check for any existing fire-related state handling:
Also applies to: 273-274, 275-276
8-8: LGTM! Verify interaction with StandingState component
The addition of the LayingDown component to MobGolemBase is correct and aligns with the PR objective to allow golems to lie down.
Let's verify the interaction between LayingDown and StandingState components:
✅ Verification successful
✓ LayingDown component can safely coexist with StandingState
Based on the search results, the LayingDown and StandingState components are commonly used together in various entity prototypes:
- Both components are present in base mob entities (base.yml)
- Used together in cyborg chassis (base_borg_chassis.yml)
- Integration tests (BuckleTest.cs) explicitly test their interaction
- The components are consistently added across different mob types (golems, cyborgs, base mobs)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any potential conflicts or dependencies between LayingDown and StandingState components
# Search for other entities using both components
ast-grep --pattern 'components:
$$$
- type: LayingDown
$$$
- type: StandingState
$$$'
# Search for any system that handles both components
rg -l "LayingDown.*StandingState|StandingState.*LayingDown"
Length of output: 97
Script:
#!/bin/bash
# Let's try a different approach to verify the components interaction
# Search for LayingDown component usage
echo "=== LayingDown Component Usage ==="
rg "type:\s*LayingDown" -A 3 -B 3
# Search for StandingState component usage
echo -e "\n=== StandingState Component Usage ==="
rg "type:\s*StandingState" -A 3 -B 3
# Search for systems handling these states
echo -e "\n=== Systems Handling States ==="
rg -l "LayingDown|StandingState" Content/Systems/
Length of output: 9968
Line range hint 401-407: Verify flying mechanics while lying down
The MobGolemSilver has both MovementIgnoreGravity and LayingDown components. Ensure that the flying mechanics work correctly when the golem transitions between standing and lying states.
Let's check for any potential conflicts:
✅ Verification successful
No conflict between flying and lying mechanics detected
After analyzing the implementation, there's no conflict between the MovementIgnoreGravity and LayingDown components for the silver golem:
- The LayingDownSystem handles state transitions between standing and lying independently of the movement system
- The MovementIgnoreGravitySystem only affects movement capabilities and doesn't interfere with the lying down state
- The silver golem's fixture is specifically configured with FlyingMobMask/FlyingMobLayer, which remains consistent regardless of lying state
- The lying down state only affects movement speed modifiers and doesn't disable the gravity-ignoring capability
The systems are properly decoupled and work independently, allowing the silver golem to maintain its flying capabilities even when transitioning between standing and lying states.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for flying-related state handling
# Search for movement gravity ignore handling with laying state
rg -A 5 "MovementIgnoreGravity.*LayingDown|LayingDown.*MovementIgnoreGravity"
# Check for any flying-related state changes
ast-grep --pattern 'fixtures:
$$$
- FlyingMobLayer
$$$
- type: LayingDown'
Length of output: 164
Script:
#!/bin/bash
# Let's try a different approach to verify the components and their interactions
# Check for MovementIgnoreGravity component implementations
rg -l "MovementIgnoreGravity"
# Search for LayingDown component implementations and state changes
rg -l "LayingDown"
# Look for flying-related state handling in movement system
ast-grep --pattern 'class $_ : ISystem {
$$$
MovementIgnoreGravity $$$
$$$
}'
# Check for golem-specific movement handling
rg -l "golem.*Movement|Movement.*golem"
Length of output: 2055
Script:
#!/bin/bash
# Let's examine the specific systems and their interactions
# Check MovementIgnoreGravitySystem implementation
cat Content.Shared/Movement/Systems/MovementIgnoreGravitySystem.cs
# Check LayingDown system implementation for any movement-related handling
cat Content.Shared/Backmen/Standing/SharedLayingDownSystem.cs
# Look at the golem prototype to see component configuration
cat Resources/Prototypes/_Backmen/Entities/Mobs/Species/golem.yml
Length of output: 26999
Content.Shared/Backmen/Standing/SharedLayingDownSystem.cs (3)
19-19: LGTM: Import aligns with new Borg functionality
The added import for Borg components is necessary for the new chassis checks.
87-87: 🛠️ Refactor suggestion
Missing Golem handling in leg check logic
The PR objectives mention implementing lying down for both Borgs and Golems, but the code only handles Borgs. Additionally, there's a redundant condition in the leg check.
- Add Golem handling:
- if (!HasComp<BorgChassisComponent>(ent) && (body.LegEntities.Count < body.RequiredLegs || body.LegEntities.Count == 0))
+ if (!HasComp<BorgChassisComponent>(ent) && !HasComp<GolemComponent>(ent) && body.LegEntities.Count < body.RequiredLegs)- Let's verify the impact of these changes:
✅ Verification successful
Let me generate additional scripts to check for standing-related systems since the previous command didn't show all the results we need.
Let me try one final verification to check for any existing Golem-specific state handling or restrictions.
Golem leg check exemption should be added
Based on the verification results, I can confirm that:
- Golems are a distinct entity type with their own component (
GolemComponent) - The current leg check logic only handles Borgs but not Golems
- The redundant condition
body.LegEntities.Count == 0is indeed covered by the first check
The original review comment is correct. The suggested fix should be implemented:
- if (!HasComp<BorgChassisComponent>(ent) && (body.LegEntities.Count < body.RequiredLegs || body.LegEntities.Count == 0))
+ if (!HasComp<BorgChassisComponent>(ent) && !HasComp<GolemComponent>(ent) && body.LegEntities.Count < body.RequiredLegs)🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential edge cases and related systems
# Check for Golem component definition
echo "Checking for Golem component..."
fd -e cs -x rg "GolemComponent"
# Check for other systems that might be affected by leg requirements
echo "Checking for other systems using leg requirements..."
rg "LegEntities|RequiredLegs" -A 5
# Check for other standing-related systems that might need similar updates
echo "Checking for other standing systems..."
rg "StandAttemptEvent|StandingState" -A 5
Length of output: 6959
Script:
#!/bin/bash
# Check for standing-related systems and their interactions
echo "Checking for standing system implementations..."
rg "StandingSystem|LayingDownSystem" -l
# Check for Borg-specific standing logic
echo "Checking for Borg-related standing code..."
rg "BorgChassisComponent.*Stand|Stand.*BorgChassisComponent" -A 3
# Check for any other mob-specific standing exceptions
echo "Checking for other mob-specific standing exceptions..."
rg "HasComp.*Stand|Stand.*HasComp" -A 3
# Check for Golem-specific movement or state handling
echo "Checking for Golem movement handling..."
rg "GolemComponent.*Move|Move.*GolemComponent" -A 3
Length of output: 820
Script:
#!/bin/bash
# Check for any Golem-specific state handling
echo "Checking for Golem state handling..."
rg "class GolemComponent|interface IGolem" -A 10
# Check for any existing Golem restrictions or special cases
echo "Checking for Golem restrictions..."
rg "GolemComponent.*HasComp|HasComp.*GolemComponent" -A 3
# Check the body system for any special mob handling
echo "Checking body system for special cases..."
fd SharedBodySystem -x cat {} | rg "HasComp.*Component.*body|body.*HasComp.*Component" -A 3
Length of output: 1761
Line range hint 1-400: Verify Borg standing behavior after lightning strikes
Based on the PR comments, we should verify how Borgs handle standing up after being struck by lightning from anomalies.
✅ Verification successful
Let me generate another script to check for Borg-specific damage handling and lightning interactions.
This script will help us verify:
- How Borgs handle damage events
- If there are any specific lightning damage handlers for Borgs
- The BorgChassisComponent implementation to understand its damage-related properties
Let me check one more aspect to understand how Borgs handle standing and damage interactions.
This will help us understand:
- How standing checks interact with Borgs
- How damage is relayed through Borg modules
- Any special standing state handling for Borgs
Based on the code analysis, I can now provide a final response about the Borg standing behavior verification:
Borgs are correctly handled in standing mechanics
The code shows that Borgs have special handling in standing mechanics that bypasses the leg requirement checks:
- In
SharedLayingDownSystem.cs, the leg check explicitly excludes Borgs:if (!HasComp<BorgChassisComponent>(ent) && (body.LegEntities.Count < body.RequiredLegs...) - Similarly in
SharedBodySystem.Body.cs, Borgs are excluded from leg requirements for standing - Damage is properly relayed through the Borg module system via
BorgModuleRelayedEvent<DamageModifyEvent>
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for lightning and anomaly interactions with Borgs
# Check for lightning/anomaly damage handlers
echo "Checking for lightning/anomaly damage systems..."
rg "lightning|anomaly" -A 5 --type cs
# Check for Borg-specific damage handlers
echo "Checking for Borg damage handlers..."
ast-grep --pattern 'class $_ {
$$$
$_(EntityUid<BorgChassisComponent> $_, DamageChangedEvent $_) {
$$$
}
$$$
}'
Length of output: 147915
Script:
#!/bin/bash
# Check for Borg damage handling and lightning interactions
# Look for Borg damage handling
echo "Checking Borg damage handling..."
rg "BorgChassisComponent.*Damage" -A 5 --type cs
# Look for lightning damage handlers
echo "Checking lightning damage handlers..."
rg "HitByLightningEvent.*Borg" -A 5 --type cs
# Look for Borg chassis component
echo "Checking BorgChassisComponent..."
rg "class BorgChassisComponent" -B 2 -A 10 --type cs
Length of output: 2260
Script:
#!/bin/bash
# Check for Borg standing and damage interactions
# Look for Borg standing checks
echo "Checking Borg standing system interactions..."
rg "HasComp<BorgChassisComponent>" -B 2 -A 5 --type cs
# Look for Borg damage relay implementation
echo "Checking Borg damage relay system..."
rg "BorgModuleRelayedEvent.*Damage" -B 2 -A 5 --type cs
# Look for standing state checks with Borgs
echo "Checking standing state system..."
rg "StandingStateComponent.*Borg" -B 2 -A 5 --type cs
Length of output: 5619
Content.Shared/Body/Systems/SharedBodySystem.Body.cs (2)
144-144: LGTM for Borg standing logic!
The condition correctly allows Borgs to stand regardless of leg count, while maintaining the leg requirement for other entities.
144-144: Verify Golem standing mechanics
According to the PR objectives, both Borgs and Golems should be able to lie down. However, the current implementation only handles Borgs. Consider if Golems need similar treatment.
✅ Verification successful
Let me gather more information about the Golem implementation to better understand their mechanics.
Let me check one more aspect to understand how Golems are implemented in relation to the body system.
Based on the analysis of the codebase, I can now provide a final response:
Golems already have lying down mechanics implemented
Looking at the MobGolemBase entity definition in the codebase, Golems already have the LayingDown component included in their base prototype. This means they already have the ability to lie down, similar to Borgs. The check in the body system is correct as is.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for Golem-related components and their standing/lying mechanics
echo "Searching for Golem components..."
rg -l "Golem.*Component"
echo "Searching for Golem standing/lying references..."
rg -i "golem.*stand|golem.*lay|golem.*lie"
Length of output: 727
Script:
#!/bin/bash
# Check GolemComponent and related systems for movement/body mechanics
echo "=== GolemComponent content ==="
cat Content.Shared/Backmen/Soul/GolemComponent.cs
echo -e "\n=== GolemSystem content ==="
cat Content.Server/Backmen/Soul/GolemSystem.cs
echo -e "\n=== Checking for body-related references in Golem files ==="
rg -i "body|stand|lying|prone" Content.Shared/Backmen/Soul/GolemComponent.cs Content.Server/Backmen/Soul/GolemSystem.cs Content.Shared/Backmen/Soul/SharedGolemSystem.cs
Length of output: 11437
Script:
#!/bin/bash
# Check if Golems use the body system and how they're spawned
echo "=== Checking for body system usage with Golems ==="
rg -A 5 "Body.*Component.*Golem|Golem.*Body"
echo -e "\n=== Checking Golem prototype definition ==="
fd -e yml -e yaml -x grep -l "Golem" {} \; -x cat {}
Length of output: 77923
github-actions
bot
added
the
Changes: Localization
KayzelW
added
Feature
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
Content.Server/Silicons/Laws/SiliconLawSystem.cs (1)
272-272: Remove non-standard commentsThe
backmen-startandbackmen-endcomments don't follow the codebase's standard commenting style and should be removed.- //backmen-start public void SetLaws(EntityUid uid, SiliconLawsetPrototype prototype) { // ... method implementation ... } - //backmen-endAlso applies to: 289-289
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
Content.Server/Backmen/Administration/Commands/Toolshed/ChangeLaws.cs(1 hunks)Content.Server/Silicons/Laws/SiliconLawSystem.cs(1 hunks)Resources/Locale/ru-RU/backmen/toolshed.ftl(1 hunks)
🔇 Additional comments (2)
Resources/Locale/ru-RU/backmen/toolshed.ftl (1)
40-42: LGTM! The command descriptions are well-structured and properly translated.
The new command descriptions for law management are clear and align with their intended functionality. The empty line provides good readability separation from other command groups.
Let's verify the integration of these commands with their implementation:
✅ Verification successful
Command descriptions correctly match their implementations
The translations in the localization file accurately reflect the implemented functionality:
command-description-laws-setmatches theSetLawsimplementation that accepts a prototype parametercommand-description-laws-overridecorresponds to theOverrideLawimplementation that takes an index and law string
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the command implementations exist and match these descriptions
# Check for the command implementations
ast-grep --pattern 'class LawsCommand {
$$$
public $_ SetLaws($_) {
$$$
}
$$$
public $_ OverrideLaw($_) {
$$$
}
$$$
}'
# Check for the prototype definitions that would be used by these commands
rg -g '*.yml' -g '*.yaml' '"laws":|^ - type: Laws'
Length of output: 196
Script:
#!/bin/bash
# Let's try a different approach to find the command implementations
# Search for command implementations using more flexible patterns
ast-grep --pattern 'SetLaws($_) {
$$$
}'
ast-grep --pattern 'OverrideLaw($_) {
$$$
}'
# Search for command registration patterns
rg -l "laws-set|laws-override" --type cs
# Look for law-related files and implementations
fd "Law" --type f --extension cs
Length of output: 1313
Script:
#!/bin/bash
# Let's check the specific command implementation file
cat "Content.Server/Backmen/Administration/Commands/Toolshed/ChangeLaws.cs"
# Also check for command registration in the system
rg -A 5 "laws-set|laws-override" "Content.Server/Backmen/Administration/Commands/Toolshed/ChangeLaws.cs"
Length of output: 3278
Content.Server/Silicons/Laws/SiliconLawSystem.cs (1)
273-288: Verify the relationship with PR objectives
The changes to SiliconLawSystem appear unrelated to the PR's stated objective of allowing Borgs and Golems to lie down. Please clarify how these law system changes contribute to the PR's goals.
Let's check for related changes:
| if (index < 0) | ||
| return null; |
There was a problem hiding this comment.
Validate Index Before Inserting New Law
When inserting a new law into laws.Laws, the index should be validated to prevent an ArgumentOutOfRangeException. Currently, only negative indices are checked. If the index is greater than the count of existing laws, it will cause an exception.
Apply this diff to fix the issue:
-if (index < 0)
+if (index < 0 || index > laws.Laws.Count)
return null;This ensures that the index is within the valid range before insertion.
Also applies to: 77-84
| [ToolshedCommand, AdminCommand(AdminFlags.Admin)] | ||
| public sealed class LawsCommand : ToolshedCommand | ||
| { | ||
| private SiliconLawSystem? _law; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential Thread-Safety Issue with Lazy Initialization of _law
The _law field is lazily initialized in multiple methods without synchronization. In a multithreaded context, this could lead to race conditions. Consider initializing _law once in the constructor or at declaration time to ensure thread safety.
Apply this diff to fix the issue:
-private SiliconLawSystem? _law;
+private readonly SiliconLawSystem _law;
+public LawsCommand()
+{
+ _law = GetSys<SiliconLawSystem>();
+}Committable suggestion skipped: line range outside the PR's diff.
| public void SetLaws(EntityUid uid, SiliconLawsetPrototype prototype) | ||
| { | ||
| if (_prototype.TryIndex<SiliconLawsetPrototype>(prototype, out var lawSet)) | ||
| { | ||
| var laws = lawSet.Laws | ||
| .Select(x => _prototype.Index<SiliconLawPrototype>(x)) | ||
| .Select(x => new SiliconLaw() | ||
| { | ||
| Order = x.Order, | ||
| LawString = x.LawString, | ||
| LawIdentifierOverride = x.LawIdentifierOverride, | ||
| }) | ||
| .ToList(); | ||
| SetLaws(laws, uid); | ||
| } | ||
| } |
There was a problem hiding this comment.
Add error handling and improve consistency with existing patterns
The current implementation has several issues that need to be addressed:
- Silent failure when prototype lookup fails
- Inconsistent parameter type compared to other methods
- Multiple LINQ operations that could be combined
- Missing documentation
Apply these improvements:
- public void SetLaws(EntityUid uid, SiliconLawsetPrototype prototype)
+ /// <summary>
+ /// Sets the laws for a silicon entity using a lawset prototype.
+ /// </summary>
+ /// <param name="uid">The entity to set laws for</param>
+ /// <param name="protoId">The ID of the lawset prototype</param>
+ /// <exception cref="ArgumentException">Thrown when the prototype is not found</exception>
+ public void SetLaws(EntityUid uid, ProtoId<SiliconLawsetPrototype> protoId)
{
- if (_prototype.TryIndex<SiliconLawsetPrototype>(prototype, out var lawSet))
- {
- var laws = lawSet.Laws
- .Select(x => _prototype.Index<SiliconLawPrototype>(x))
- .Select(x => new SiliconLaw()
- {
- Order = x.Order,
- LawString = x.LawString,
- LawIdentifierOverride = x.LawIdentifierOverride,
- })
- .ToList();
- SetLaws(laws, uid);
- }
+ var lawSet = _prototype.Index(protoId);
+ var laws = lawSet.Laws
+ .Select(x => _prototype.Index(x))
+ .Select(x => new SiliconLaw
+ {
+ Order = x.Order,
+ LawString = x.LawString,
+ LawIdentifierOverride = x.LawIdentifierOverride,
+ })
+ .ToList();
+ SetLaws(laws, uid);
}📝 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 void SetLaws(EntityUid uid, SiliconLawsetPrototype prototype) | |
| { | |
| if (_prototype.TryIndex<SiliconLawsetPrototype>(prototype, out var lawSet)) | |
| { | |
| var laws = lawSet.Laws | |
| .Select(x => _prototype.Index<SiliconLawPrototype>(x)) | |
| .Select(x => new SiliconLaw() | |
| { | |
| Order = x.Order, | |
| LawString = x.LawString, | |
| LawIdentifierOverride = x.LawIdentifierOverride, | |
| }) | |
| .ToList(); | |
| SetLaws(laws, uid); | |
| } | |
| } | |
| /// <summary> | |
| /// Sets the laws for a silicon entity using a lawset prototype. | |
| /// </summary> | |
| /// <param name="uid">The entity to set laws for</param> | |
| /// <param name="protoId">The ID of the lawset prototype</param> | |
| /// <exception cref="ArgumentException">Thrown when the prototype is not found</exception> | |
| public void SetLaws(EntityUid uid, ProtoId<SiliconLawsetPrototype> protoId) | |
| { | |
| var lawSet = _prototype.Index(protoId); | |
| var laws = lawSet.Laws | |
| .Select(x => _prototype.Index(x)) | |
| .Select(x => new SiliconLaw | |
| { | |
| Order = x.Order, | |
| LawString = x.LawString, | |
| LawIdentifierOverride = x.LawIdentifierOverride, | |
| }) | |
| .ToList(); | |
| SetLaws(laws, uid); | |
| } |
🆑
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation