-
-
Notifications
You must be signed in to change notification settings - Fork 97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Web logic against infantry #1152
base: develop
Are you sure you want to change the base?
Conversation
Nightly build for this pull request:
This comment is automatic and is meant to allow guests to get latest nightly builds for this pull request without registering. It is updated on every successful build. |
as suggested by Otamaa
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ImmuneToWeb
should be described in the documentation. I think this PR is OK aside from a few review comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 5
Configuration used: CodeRabbit UI
Files selected for processing (12)
- CREDITS.md (1 hunks)
- docs/New-or-Enhanced-Logics.md (1 hunks)
- docs/Whats-New.md (1 hunks)
- src/Ext/Techno/Body.Update.cpp (1 hunks)
- src/Ext/Techno/Body.cpp (1 hunks)
- src/Ext/Techno/Body.h (4 hunks)
- src/Ext/Techno/Hooks.cpp (2 hunks)
- src/Ext/TechnoType/Body.cpp (2 hunks)
- src/Ext/TechnoType/Body.h (2 hunks)
- src/Ext/Unit/Hooks.DisallowMoving.cpp (2 hunks)
- src/Ext/WarheadType/Body.cpp (2 hunks)
- src/Ext/WarheadType/Body.h (2 hunks)
Additional comments: 23
src/Ext/Unit/Hooks.DisallowMoving.cpp (3)
- 5-5: Including
InfantryClass.h
is necessary for accessing infantry-related properties and methods, which is relevant for the new feature being implemented.- 9-9: Including
<Ext/Techno/Body.h>
is essential for accessing extended properties and methods related to theTechnoClass
, which includes infantry. This is crucial for implementing the new web effect logic.- 87-109: The new hook
InfantryClass_Assign_Destination_DisallowMoving
effectively prevents webbed infantry from being assigned new destinations. This logic checks if the infantry is currently webbed (WebbyDurationCountDown > 0
) and, if so, clears their target and destination, and sets their mission toSleep
. This approach ensures that webbed infantry units remain stationary until the effect wears off.
- The use of
ParalysisTimer.HasTimeLeft()
to check for any existing paralysis effect is a good practice, ensuring that the code respects other potential sources of paralysis.- The retrieval of the
TechnoExt
extension data (pExt
) is necessary to access custom properties related to the web effect.- The condition to check if the infantry is webbed (
WebbyDurationCountDown > 0
) is straightforward and effective.- The actions taken when an infantry unit is webbed (clearing the target and destination, setting the mission to
Sleep
) are appropriate for the intended behavior of temporarily paralyzing the unit.Overall, this implementation aligns well with the PR's objectives of introducing a non-damaging, temporary paralysis effect for infantry units.
src/Ext/Techno/Body.h (5)
- 46-50: The addition of new members (
WebbyDurationCountDown
,WebbyDurationTimer
,WebbyAnim
,WebbyLastTarget
, andWebbyLastMission
) to theTechnoExt::ExtData
class is crucial for implementing the web effect logic. These members will store the duration of the effect, manage timers, track animations, and remember the last target and mission of the webbed unit.
WebbyDurationCountDown
is used to count down the duration of the web effect.WebbyDurationTimer
is a timer that likely manages the actual timing of the effect.WebbyAnim
holds the animation associated with the web effect.WebbyLastTarget
andWebbyLastMission
are used to store the last target and mission of the unit before being webbed, which is important for restoring the unit's state after the effect wears off.These additions are well thought out and align with the objectives of the PR.
- 70-74: Initializing the new web-related members in the
TechnoExt::ExtData
constructor ensures that they are set to sensible default values upon creation of an instance. This is a good practice for avoiding uninitialized state issues.
- Setting
WebbyDurationCountDown
to-1
by default indicates that, by default, units are not under the web effect.- Initializing
WebbyAnim
andWebbyLastTarget
tonullptr
andWebbyLastMission
toMission::Sleep
are appropriate defaults that reflect a "no effect" state.This initialization is crucial for the correct functioning of the web effect logic.
- 90-90: The addition of the
WebbyUpdate()
method to theTechnoExt::ExtData
class is essential for updating the state of the web effect on units. This method likely contains the logic for decrementing the duration countdown, handling the expiration of the effect, and possibly updating animations or restoring the unit's previous state.Without seeing the implementation details, it's clear that this method plays a key role in the web effect's lifecycle management.
- 97-97: Modifying the
InvalidatePointer()
method to includeWebbyLastTarget
ensures that if the target object is removed from the game, the pointer is safely invalidated. This prevents potential crashes due to dereferencing a dangling pointer.This change is a direct response to a previous review comment and demonstrates attention to detail and a commitment to code safety.
- 160-160: The static
WebbyUpdate()
method in theTechnoExt
class is likely responsible for updating the web effect across all instances ofTechnoClass
. This method's presence at the class level suggests it's intended to be called periodically, perhaps from a game tick or similar mechanism, to ensure the web effect is processed for all affected units in a centralized manner.This approach is efficient for managing game-wide effects and aligns with the objectives of introducing a non-damaging, temporary paralysis effect.
src/Ext/WarheadType/Body.h (2)
- 118-122: The addition of new properties (
Webby
,Webby_Anims
,Webby_Duration
,Webby_DurationVariation
, andWebby_Cap
) to theWarheadTypeExt::ExtData
class is essential for configuring the web effect at the warhead level. These properties allow for a high degree of customization of the web effect, including:
- Enabling the web effect (
Webby
).- Specifying animations to display when the effect is applied (
Webby_Anims
).- Setting the duration and variation of the effect (
Webby_Duration
andWebby_DurationVariation
).- Controlling the stacking behavior of the effect (
Webby_Cap
).These properties are crucial for the flexible implementation of the web effect, allowing game designers to tailor the effect to their needs.
- 247-251: Initializing the new web-related properties in the
WarheadTypeExt::ExtData
constructor ensures that they are set to sensible default values. This is a good practice for avoiding uninitialized state issues and provides a clear baseline for how the web effect behaves by default.
- Setting
Webby
tofalse
by default ensures that the web effect is opt-in.- Initializing
Webby_Anims
as an empty vector,Webby_Duration
andWebby_DurationVariation
to0
, andWebby_Cap
to-1
are appropriate defaults that reflect a "no effect" state.This initialization is crucial for the correct functioning of the web effect logic at the warhead level.
src/Ext/TechnoType/Body.h (2)
- 188-191: The addition of new properties (
Webby_Anims
,ImmuneToWeb
,Webby_Duration
, andWebby_DurationVariation
) to theTechnoTypeExt::ExtData
class is essential for configuring the web effect at the techno type level. These properties allow for:
- Specifying animations to display when the effect is applied (
Webby_Anims
).- Making certain units immune to the web effect (
ImmuneToWeb
).- Setting the duration and variation of the effect (
Webby_Duration
andWebby_DurationVariation
).These properties are crucial for the flexible implementation of the web effect, allowing game designers to tailor the effect to specific units or classes of units.
- 371-374: Initializing the new web-related properties in the
TechnoTypeExt::ExtData
constructor ensures that they are set to sensible default values. This is a good practice for avoiding uninitialized state issues and provides a clear baseline for how the web effect behaves by default for different techno types.
- Initializing
Webby_Anims
as an empty vector and settingImmuneToWeb
tofalse
ensures that, by default, units are not immune and have no animations associated with the web effect.- Setting
Webby_Duration
andWebby_DurationVariation
to0
are appropriate defaults that reflect a "no effect" state unless explicitly configured.This initialization is crucial for the correct functioning of the web effect logic at the techno type level.
src/Ext/Techno/Body.cpp (1)
- 387-391: The serialization for
WebbyDurationCountDown
,WebbyDurationTimer
,WebbyAnim
,WebbyLastTarget
, andWebbyLastMission
has been correctly added to theTechnoExt::ExtData::Serialize
method. Ensure that these properties are properly initialized and used safely throughout the codebase.Verification successful
The initialization and usage of the properties
WebbyDurationCountDown
,WebbyDurationTimer
,WebbyAnim
,WebbyLastTarget
, andWebbyLastMission
are properly handled in the codebase. Their states are correctly managed across different scenarios, and the serialization process inBody.cpp
accurately includes these properties. The review comment's recommendation to verify the initialization and usage of these properties has been confirmed through the provided context.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify initialization of new properties rg --type cpp "WebbyDurationCountDown|WebbyDurationTimer|WebbyAnim|WebbyLastTarget|WebbyLastMission" src/Length of output: 2728
CREDITS.md (1)
- 133-133: The addition of the web logic contribution is well-formatted and clearly described, accurately acknowledging the work done on the project.
src/Ext/Techno/Hooks.cpp (3)
- 8-8: Including
<Ext/TechnoType/Body.h>
ensures that the necessary definitions from the TechnoType extension are available for use in this file. This change is necessary for the new functionality introduced in this pull request.- 444-465: The
InfantryClass_DrawIt_DontRenderSHP
hook is designed to prevent the rendering of infantry units when they are affected by the web effect. This is achieved by checking theWebbyDurationCountDown
property of the infantry's extension. If the countdown is greater than 0, indicating that the unit is currently webbed, the drawing code is skipped.This implementation correctly handles the condition to not render the SHP (Shape File) of an infantry unit when it is under the effect of a web. However, it's important to ensure that all necessary visual feedback is provided to the player to indicate that the unit is webbed, such as playing an animation or changing the unit's appearance in some other way.
- 467-555: The
InfantryClass_TakeDamage_Webby
hook manages the application of the web effect to infantry units. It checks for the presence of theWebby
property on the warhead and applies the effect accordingly. This includes selecting an animation from either the warhead or the unit type, creating the animation, and adjusting the unit's paralysis timer and web effect duration.This implementation appears to correctly handle the application of the web effect, including the selection of animations, calculation of the effect duration with variation, and handling of the effect stacking behavior based on the
Webby_Cap
property. It's important to ensure that the effect is visually clear to the player and that the gameplay implications of the web effect (e.g., temporary immobilization) are well communicated.One potential improvement could be to add comments explaining the logic behind the calculation of the web effect duration and its variation, as well as the handling of different
Webby_Cap
values, to make the code more understandable for future maintainers.Consider adding comments to explain the logic behind the duration calculation and handling of
Webby_Cap
values for better code maintainability.src/Ext/WarheadType/Body.cpp (2)
- 200-205: The addition of
Webby
properties to theWarheadTypeExt::ExtData
class allows for the customization of the web effect through INI files. This includes specifying whether a warhead has the web effect (Webby
), the animations to play (Webby_Anims
), the duration of the effect (Webby_Duration
), the variation in duration (Webby_DurationVariation
), and how the effect stacks (Webby_Cap
).This implementation correctly extends the warhead type with new properties to support the web effect, providing a flexible way to define how the effect behaves. It's important to ensure that these properties are well-documented in the mod's documentation to help mod creators understand how to use them effectively.
- 350-355: The serialization of the
Webby
properties ensures that these settings are correctly saved and loaded, preserving the state of the web effect across game sessions. This is crucial for maintaining consistency in the game's behavior and for debugging purposes.This implementation correctly handles the serialization of the new
Webby
properties, ensuring that the game can correctly save and load these settings. It's important to test this functionality thoroughly to ensure that there are no issues with saving or loading the game state related to these new properties.src/Ext/Techno/Body.Update.cpp (1)
- 96-134: The
WebbyUpdate
function introduces the logic for handling the web effect on infantry units. Here are some observations and suggestions:
Correctness & Logic: The function correctly checks if the unit is active and an infantry unit before proceeding with the web effect logic. This ensures that the web effect is only applied to valid targets.
Performance: The function is called during the
OnEarlyUpdate
phase, which is appropriate as it allows for timely updates to the web effect's state. However, ensure that the performance impact is minimal, especially in large-scale battles with many units.Error Handling: The function assumes that the
TechnoExt::ExtMap.Find(pThis)
call will always succeed. Consider adding error handling for cases where the extension data might not be found for some reason.Animation Handling: The use of
pExt->WebbyAnim->Limbo()
to handle the animation is appropriate. Ensure that the animation is correctly initialized elsewhere in the codebase and that it's properly cleaned up when no longer needed.Restoring Previous Action: The logic to restore the unit's previous action (destination, target, and mission) is a good touch. It ensures that units return to their intended behavior after the web effect wears off.
Maintainability: The function is well-structured and follows a logical flow, making it easy to understand and maintain. Consider adding comments to explain the purpose of each major block within the function for future maintainability.
Overall, the implementation of the
WebbyUpdate
function appears solid. Ensure thorough testing, especially around the edges of the web effect's duration and interaction with other game mechanics.src/Ext/TechnoType/Body.cpp (2)
- 277-280: The implementation for reading the new web-related properties from the INI file appears correct and follows the established pattern for loading properties. Ensure that the INI sections and keys are properly documented to guide modders or developers in configuring these properties.
- 589-592: The serialization of the new web-related properties (
Webby_Anims
,ImmuneToWeb
,Webby_Duration
,Webby_DurationVariation
) is handled correctly. This ensures that these settings are preserved across game sessions. Good job following the existing serialization pattern.docs/Whats-New.md (1)
- 350-350: The term "Web logic against infantry" is introduced without context. It might be helpful to briefly describe what "Web logic" entails for clarity.
Consider adding a brief description or reference to what "Web logic" entails for readers unfamiliar with the term.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/Ext/Techno/Body.Update.cpp (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/Ext/Techno/Body.Update.cpp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/Ext/Techno/Body.Update.cpp (2 hunks)
Additional comments: 1
src/Ext/Techno/Body.Update.cpp (1)
- 99-133: The
WebbyUpdate
method introduces several key functionalities for managing the web effect on infantry units. Here are some observations and suggestions:
Active State and Type Checks (Lines 103-108): The method correctly checks if the unit is active and is of type Infantry before proceeding. This ensures that the web effect is applied only to the intended targets.
Duration Timer Check (Lines 110-132): The logic for checking if the web effect's duration has expired is clear and straightforward. When the duration expires, the method proceeds to clean up the web effect by stopping the timer, detaching the animation, and restoring the unit's previous actions.
Animation Handling (Lines 115-119): The method checks if the
WebbyAnim
has a type before marking it for removal. This is a good practice as it prevents potential null pointer exceptions.Restoration of Previous Actions (Lines 124-131): Restoring the unit's last target, mission, and destination is crucial for a smooth transition back to normal behavior. This part of the method ensures that units can resume their previous state seamlessly after the web effect wears off.
Overall, the implementation of the
WebbyUpdate
method appears to be solid and well-thought-out. It effectively manages the new web effect, ensuring that it is applied and removed correctly for infantry units. Great job on this addition!
Added a new tag ForceWeapon.Webby=-1 that points the weapon that should use the unit against victims of the web net...
Added a new tag ForceWeapon.Webby=-1 that points the weapon that should use the unit against victims of the web net... |
Webby=yes
.Webby.Duration
specifies the duration, in frames, of the warhead's web effect.Webby.DurationVariation
allows for random variance to the duration of the warhead's web effect.Webby.Anims
contains more than 1 animation then the new animation will be picked randomly.Webby.Cap
works like in EMP logic developed by Ares.Webby.Cap=-1
case: The target’s web counter is set to this absolute number of frames specified byWeb.Duration
, unless the target’s web counter is already greater than this.Webby.Cap=0
case: Makes this web effect stackable, but uncapped.Webby.Cap >0
case: Makes this web effect stackable, but maximum value capped toWebby.Cap
value.In
rulesmd.ini
:[SOMEWARHEAD] ; Warhead
Webby=no ; boolean
Webby.Anims= ; list of animations
Webby.Duration=0 ; integer, game frames
Webby.DurationVariation=0 ; integer
Webby.Cap=-1 ; integer
In
rulesmd.ini
:[SOMETECHNO] ; InfantryType
ImmuneToWeb=no ; boolean
Webby.Anims= ; list of animations
Webby.Duration=0 ; integer, game frames
Webby.DurationVariation=0 ; integer
Summary by CodeRabbit