Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Insignias drawing customization. Animations on techno promotion. #1195

Merged
merged 24 commits into from
Jul 10, 2024

Conversation

Fryone
Copy link
Contributor

@Fryone Fryone commented Jan 21, 2024

[AudioVisual]
DrawInsignia.OnlyOnSelected=true
DrawInsignia.AdjustPos.Infantry=5, 2  ;position adjustment from infantry center
DrawInsignia.AdjustPos.Buildings=10, 6  ;position adjustment from building center
DrawInsignia.AdjustPos.Units=10, 6  ;position adjustment from other units center

Promote.VeteranAnimation= ;animation, played when techno is getting veteran (or elite if next is not defined)
Promote.EliteAnimation= ;animation, played when techno is getting elite

Copy link

coderabbitai bot commented Jan 21, 2024

Walkthrough

The recent update brings a new promotion animation feature for unit or structure promotions, offering distinct animations for different veterancy levels. It enhances control over insignia display, allowing customization for selected units. Additionally, the update standardizes the process of reading INI files across various modules, potentially improving performance and simplifying code.

Changes

Files Change Summary
CREDITS.md
docs/Whats-New.md
Added details and documentation for the promotion animation feature.
docs/Fixed-or-Improved-Logics.md
src/Ext/Rules/Body.h
Expanded options for insignia display with a new setting.
docs/New-or-Enhanced-Logics.md Introduced customization for promotion animations and visibility settings.
src/Ext/Rules/Body.cpp
src/Ext/Techno/Hooks.cpp
Implemented visual effects for promotions and control over insignia display.
YRpp Updated the subproject commit reference.
src/Ext/AnimType/Body.cpp
.../BuildingType/Body.cpp
.../BulletType/Body.cpp
.../SWType/Body.cpp
.../TechnoType/Body.cpp
.../WarheadType/Body.cpp
.../WeaponType/Body.cpp
Standardized the process of reading INI files.
src/Ext/Bullet/Hooks.DetonateLogics.cpp Simplified code and potentially improved performance.

Poem

In the realm of code, where changes gleam bright,
Rabbits dance in the glow of the digital light.
Promotions and animations, a symphony they play,
Standardizing the dance, in an elegant array.
Through lines of code, they prance and they flip,
Crafting a world with each elegant script.
In this game of updates, where features entwine,
The rabbits march on, in a dance so divine.

🐇💻🎶


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

‼️ IMPORTANT
Auto-reply has been disabled for this repository in the CodeRabbit settings. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged.

  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

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

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

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

Documentation and Community

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 5c46132 and 66e66c7.
Files selected for processing (4)
  • docs/Fixed-or-Improved-Logics.md (1 hunks)
  • src/Ext/Rules/Body.cpp (2 hunks)
  • src/Ext/Rules/Body.h (2 hunks)
  • src/Ext/Techno/Hooks.cpp (1 hunks)
Additional comments: 6
src/Ext/Rules/Body.h (2)
  • 107-107: The new property DrawInsigniaOnlyOnSelected has been added to the RulesExt class without any issues.
  • 167-167: The initialization of DrawInsigniaOnlyOnSelected to false in the constructor is consistent with the declaration and the intended default behavior as per the PR objectives.
src/Ext/Techno/Hooks.cpp (1)
  • 400-406: The conditional logic to draw insignias only when units are selected is implemented correctly. It respects the new DrawInsigniaOnlyOnSelected setting and maintains the existing behavior when the setting is not enabled.
src/Ext/Rules/Body.cpp (2)
  • 135-135: The LoadBeforeTypeData method has been updated to read the DrawInsigniaOnlyOnSelected property from the INI file correctly.
  • 266-266: The Serialize method correctly processes the DrawInsigniaOnlyOnSelected property, ensuring it is included in save and load operations.
docs/Fixed-or-Improved-Logics.md (1)
  • 409-419: The addition of DrawInsigniaOnlyOnSelected under [AudioVisual] is consistent with the PR objectives. However, the summary mentions that the default value for this setting is false, which should be reflected in the documentation to avoid confusion.

Copy link

github-actions bot commented Jan 21, 2024

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.

src/Ext/Techno/Hooks.cpp Outdated Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 66e66c7 and d472f3c.
Files selected for processing (1)
  • src/Ext/Techno/Hooks.cpp (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • src/Ext/Techno/Hooks.cpp

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 4

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between d472f3c and 8699731.
Files selected for processing (19)
  • YRpp (1 hunks)
  • src/Ext/AnimType/Body.cpp (3 hunks)
  • src/Ext/BuildingType/Body.cpp (1 hunks)
  • src/Ext/Bullet/Hooks.DetonateLogics.cpp (1 hunks)
  • src/Ext/BulletType/Body.cpp (1 hunks)
  • src/Ext/SWType/Body.cpp (1 hunks)
  • src/Ext/TechnoType/Body.cpp (2 hunks)
  • src/Ext/WarheadType/Body.cpp (1 hunks)
  • src/Ext/WeaponType/Body.cpp (2 hunks)
  • src/Misc/Hooks.INIInheritance.cpp (3 hunks)
  • src/New/Type/Affiliated/InterceptorTypeClass.cpp (1 hunks)
  • src/New/Type/RadTypeClass.cpp (1 hunks)
  • src/New/Type/ShieldTypeClass.cpp (1 hunks)
  • src/Phobos.cpp (2 hunks)
  • src/Utilities/Macro.h (4 hunks)
  • src/Utilities/Patch.cpp (3 hunks)
  • src/Utilities/Patch.h (2 hunks)
  • src/Utilities/Template.h (2 hunks)
  • src/Utilities/TemplateDef.h (43 hunks)
Additional comments: 35
YRpp (1)
  • 1-1: The change in the subproject commit reference is noted. Ensure that the new commit 646099b78e03ad354912b2bd2ccc9fff913c76b5 has been tested and is stable.
src/Utilities/Patch.h (2)
  • 23-23: The addition of a new Apply_RAW method is noted. Ensure that the method is used consistently and that any previous raw patch applications have been refactored to use this new method.
Verification successful

Observations

The shell script searched for instances of the Patch::Apply_RAW method in the codebase. The output shows multiple occurrences of this method being used in various files.

Actions

The script output indicates that the Patch::Apply_RAW method is indeed being used in several places within the codebase. This suggests that the new Apply_RAW method added to the Patch struct in Patch.h is being utilized in the code. Therefore, the method appears to be used consistently, as requested in the review comment.

Given this information, the response to the review comment can be formulated.

Response

The Patch::Apply_RAW method is being used consistently in the codebase, indicating that the new Apply_RAW method added to the Patch struct in Patch.h is being utilized as expected. The review comment has been successfully verified.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Search for any raw patch applications that should now use Apply_RAW
rg --type cpp "Patch::Apply_RAW"

Length of output: 2353

* 43-46: The `Apply_VTABLE` method has been added. Verify that all virtual table patching now uses this method and that it is correctly applied in all instances.
src/New/Type/RadTypeClass.cpp (1)
  • 24-24: The RadWarhead.Read method now includes an additional boolean argument. Confirm that this change is reflected everywhere this method is called and that the boolean argument is correctly set.
src/New/Type/Affiliated/InterceptorTypeClass.cpp (1)
  • 27-27: The Read method call for the WeaponOverride attribute now includes an additional boolean argument. Verify that this change is consistent with the intended logic and that the boolean argument is correctly set.
src/Utilities/Patch.cpp (3)
  • 7-9: Initialization of buf and retrieval of the module file name in GetSection function is noted. Ensure that the buffer size is adequate and that there are no buffer overflows.
  • 52-56: The new Apply_RAW method implementation is correct and follows the existing pattern for similar methods.
  • 79-84: The Apply_VTABLE method is implemented correctly and follows the existing pattern for similar methods.
src/Utilities/Macro.h (2)
  • 100-103: The DEFINE_PATCH macro has been modified to use byte instead of a specific type. Confirm that this change does not affect the expected behavior where this macro is used.
  • 123-126: The DEFINE_DYNAMIC_PATCH macro has been modified similarly to DEFINE_PATCH. Verify that this change is correctly applied wherever the macro is used.
src/New/Type/ShieldTypeClass.cpp (1)
  • 61-61: The Read method call for BreakWeapon has been modified to include the boolean argument at the end. Verify that this change is consistent with the intended logic and that the boolean argument is correctly set.
src/Ext/BulletType/Body.cpp (1)
  • 38-38: The Interceptable_WeaponOverride.Read method call has been altered to include an additional boolean argument. Confirm that this change is consistent with the intended logic and that the boolean argument is correctly set.
src/Phobos.cpp (1)
  • 163-169: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [166-180]

The hook for _ExeTerminate has been changed to a naked function with a jump to the new location. Ensure that this change does not introduce any issues with the program's termination sequence.

Verification successful

The review comment requests verification of the changes made to the _ExeTerminate hook in the Phobos.cpp file. The change involves converting the hook to a naked function and adding a jump to a new location. The goal is to ensure that this change does not introduce any issues with the program's termination sequence. The provided shell script uses ripgrep to search for _ExeTerminate in the C++ files. The output confirms that the _ExeTerminate hook is now defined as a naked function in Phobos.cpp.

The verification script has successfully located the _ExeTerminate hook in the Phobos.cpp file, confirming that the changes have been correctly implemented. Therefore, the changes to the _ExeTerminate hook appear to be consistent with the review comment.

Given this information, the changes to the _ExeTerminate hook in the Phobos.cpp file have been verified and do not appear to introduce any issues with the program's termination sequence.

Conclusion

The changes to the _ExeTerminate hook in the Phobos.cpp file have been successfully verified and do not appear to introduce any issues with the program's termination sequence.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify that the new _ExeTerminate hook is correctly implemented and does not cause issues
rg --type cpp "_ExeTerminate"

Length of output: 87

src/Ext/WeaponType/Body.cpp (2)
  • 47-47: The RadType.Read method call now includes an additional boolean argument. Confirm that this change is reflected everywhere this method is called and that the boolean argument is correctly set.
  • 56-56: The FeedbackWeapon.Read method call has been updated to include an additional boolean argument. Verify that this change is consistent with the intended logic and that the boolean argument is correctly set.
src/Ext/AnimType/Body.cpp (3)
  • 87-87: The CreateUnit.Read method call now includes an additional boolean argument. Confirm that this change is reflected everywhere this method is called and that the boolean argument is correctly set.
  • 102-102: The Weapon.Read method call has been updated to include an additional boolean argument. Verify that this change is consistent with the intended logic and that the boolean argument is correctly set.
  • 111-111: The AttachedSystem.Read method call now includes an additional boolean argument. Confirm that this change is reflected everywhere this method is called and that the boolean argument is correctly set.
src/Ext/Bullet/Hooks.DetonateLogics.cpp (4)
  • 85-85: The direct instantiation of DynamicVectorClass<AircraftClass*> is a change from the previous lambda function approach. Ensure that this change does not introduce any unintended side effects, such as memory leaks or improper copying of the array elements.
  • 93-93: Similar to the previous comment, verify that the direct instantiation of DynamicVectorClass<BuildingClass*> does not cause any issues with memory management or element copying.
  • 101-101: Again, verify that the direct instantiation of DynamicVectorClass<InfantryClass*> is safe and does not introduce any memory or copying issues.
  • 109-109: Lastly, ensure that the direct instantiation of DynamicVectorClass<UnitClass*> is handled correctly without causing memory leaks or improper copying.
src/Ext/SWType/Body.cpp (1)
  • 138-138: The change to the Read method call for Detonate_Weapon now includes a boolean parameter. Confirm that this parameter is being used correctly within the method and that all calls to this method have been updated to include the new parameter.
src/Misc/Hooks.INIInheritance.cpp (3)
  • 32-32: The addition of the false parameter to the detail::read function call within ReadTemplate may affect the control flow. Verify that this default value is intended and does not alter the expected behavior.
  • 47-47: Similar to the previous comment, ensure that the false parameter added to the detail::read function call within ReadTemplatePtr is correct and does not change the intended functionality.
  • 63-63: Again, verify that the false parameter added to the detail::read function call within ReadTemplatePtr<CLSID> is appropriate and does not introduce any issues.
src/Ext/BuildingType/Body.cpp (1)
  • 141-141: The Read method for Grinding_Weapon now includes a boolean parameter. Confirm that this parameter is being used correctly and that all calls to this method have been updated accordingly.
src/Utilities/Template.h (2)
  • 114-114: The Read method in the Valueable class now includes an additional boolean parameter named Allocate. Ensure that this parameter is being used correctly within the method and that all calls to this method have been updated to include the new parameter.
  • 220-220: Similarly, the Read method in the Nullable class now includes the Allocate boolean parameter. Verify that this parameter is being used correctly and that all calls to this method have been updated.
src/Ext/WarheadType/Body.cpp (1)
  • 151-151: The Read method call for Shield_BreakWeapon has been modified to include a boolean parameter at the end of the argument list. This change is consistent with the PR objectives and the AI-generated summary, which do not mention this particular property. However, the addition of the boolean parameter suggests a change in the behavior of the Read method, possibly to control whether the property is mandatory or not in the INI file.
src/Ext/TechnoType/Body.cpp (2)
  • 154-154: The Read method call for ShieldType has been modified to include a boolean parameter at the end of the argument list. This change is consistent with the PR objectives and the AI-generated summary, which do not mention this particular property. However, the addition of the boolean parameter suggests a change in the behavior of the Read method, possibly to control whether the property is mandatory or not in the INI file.
  • 194-196: The Read method calls for WarpInWeapon, WarpInMinRangeWeapon, and WarpOutWeapon have been modified to include a boolean parameter at the end of the argument list. This change is consistent with the PR objectives and the AI-generated summary, which do not mention these particular properties. However, the addition of the boolean parameter suggests a change in the behavior of the Read method, possibly to control whether the property is mandatory or not in the INI file.
src/Utilities/TemplateDef.h (3)
  • 64-73: The read function template has been modified to include an allocate parameter. Ensure that all calls to this function have been reviewed to pass the correct allocate argument where necessary.
  • 1273-1275: The Valueable<T>::Read method has been updated to pass the Allocate parameter to the detail::read function. Verify that all usages of Valueable<T>::Read have been updated to pass this parameter correctly.
  • 1312-1314: The Nullable<T>::Read method has been updated to pass the Allocate parameter to the detail::read function. Verify that all usages of Nullable<T>::Read have been updated to pass this parameter correctly.

src/Utilities/TemplateDef.h Outdated Show resolved Hide resolved
src/Utilities/TemplateDef.h Outdated Show resolved Hide resolved
src/Utilities/TemplateDef.h Outdated Show resolved Hide resolved
src/Utilities/TemplateDef.h Outdated Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 8699731 and b1a637d.
Files selected for processing (1)
  • docs/Fixed-or-Improved-Logics.md (1 hunks)
Additional comments: 1
docs/Fixed-or-Improved-Logics.md (1)
  • 409-419: The documentation correctly introduces the DrawInsigniaOnlyOnSelected feature under the [AudioVisual] section, setting its default value to false. This aligns with the PR's objective to allow insignias to be displayed only on selected units, enhancing the game's visual customization. The documentation is clear and provides the necessary information for users to understand and utilize this new feature.

global audiovisual params
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 1

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between b1a637d and 827fd06.
Files selected for processing (5)
  • CREDITS.md (1 hunks)
  • docs/New-or-Enhanced-Logics.md (1 hunks)
  • src/Ext/Rules/Body.cpp (2 hunks)
  • src/Ext/Rules/Body.h (2 hunks)
  • src/Ext/Techno/Hooks.cpp (2 hunks)
Additional comments: 7
src/Ext/Rules/Body.h (1)
  • 169-171: Initialization of the new properties in the constructor follows the expected pattern. However, verify that the default values align with the intended functionality, especially for feature toggles and nullable types.
src/Ext/Techno/Hooks.cpp (2)
  • 400-403: The conditional check for DrawInsigniaOnlyOnSelected is implemented correctly. However, ensure that IsSelected property accurately reflects the unit's selection state in all scenarios.
  • 462-474: The logic for handling promotion animations based on veterancy levels is implemented. Verify that Promote_VeteranAnimation and Promote_EliteAnimation are correctly triggered and that the animations are properly loaded and displayed.
CREDITS.md (1)
  • 311-311: The addition of Fryone and their contributions, including the promotion animation feature, is correctly documented.
src/Ext/Rules/Body.cpp (2)
  • 135-137: The loading of DrawInsigniaOnlyOnSelected, Promote_VeteranAnimation, and Promote_EliteAnimation from the INI file is implemented. Ensure that these properties are correctly read from the INI file and that the section and keys exist in the INI files.
  • 268-270: Serialization of the new properties is correctly implemented. Verify that serialization and deserialization work as expected, especially for nullable types.
docs/New-or-Enhanced-Logics.md (1)
  • 969-982: The documentation introduces the ability to specify distinct animations for veteran and elite promotions, enhancing visual feedback for promotions. Ensure the implementation aligns with this documentation.

src/Ext/Rules/Body.h Outdated Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 827fd06 and 4881187.
Files selected for processing (6)
  • CREDITS.md (1 hunks)
  • docs/Fixed-or-Improved-Logics.md (1 hunks)
  • docs/Whats-New.md (1 hunks)
  • src/Ext/Rules/Body.cpp (2 hunks)
  • src/Ext/Rules/Body.h (2 hunks)
  • src/Ext/Techno/Hooks.cpp (2 hunks)
Files skipped from review as they are similar to previous changes (4)
  • CREDITS.md
  • src/Ext/Rules/Body.cpp
  • src/Ext/Rules/Body.h
  • src/Ext/Techno/Hooks.cpp
Additional comments: 3
docs/Whats-New.md (1)
  • 359-359: The addition of the promotion animation feature is clear and concise. However, ensure that the documentation elsewhere is updated to include detailed implementation guidance for this feature if necessary.
docs/Fixed-or-Improved-Logics.md (2)
  • 409-410: The documentation for DrawInsigniaOnlyOnSelected is clear and accurately describes the new feature's functionality. However, ensure that this feature is consistently implemented across all relevant game entities and thoroughly tested in various gameplay scenarios.
  • 406-422: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-408]

The documentation thoroughly covers a wide range of fixes and improvements, providing clear and detailed explanations. It's important to ensure that all changes are accurately reflected in the game's behavior and that any potential side effects or interactions with other game features are considered and documented if necessary.

I hope, that this is working as its should.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 4881187 and f2e670d.
Files selected for processing (3)
  • docs/New-or-Enhanced-Logics.md (1 hunks)
  • src/Ext/Rules/Body.cpp (2 hunks)
  • src/Ext/Techno/Hooks.cpp (2 hunks)
Files skipped from review as they are similar to previous changes (2)
  • src/Ext/Rules/Body.cpp
  • src/Ext/Techno/Hooks.cpp
Additional comments: 1
docs/New-or-Enhanced-Logics.md (1)
  • 969-983: The documentation for promotion animations is clear and correctly specifies the new properties Promote.VeteranAnimation, Promote.EliteAnimation, and Promote.AnimationVisibility. The fallback mechanism for elite promotions and visibility customization are well-explained.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between f2e670d and cfa1b5e.
Files selected for processing (3)
  • CREDITS.md (1 hunks)
  • docs/Fixed-or-Improved-Logics.md (1 hunks)
  • docs/Whats-New.md (1 hunks)
Files skipped from review as they are similar to previous changes (3)
  • CREDITS.md
  • docs/Fixed-or-Improved-Logics.md
  • docs/Whats-New.md

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between cfa1b5e and 02111e6.
Files selected for processing (7)
  • CREDITS.md (1 hunks)
  • docs/Fixed-or-Improved-Logics.md (1 hunks)
  • docs/New-or-Enhanced-Logics.md (1 hunks)
  • docs/Whats-New.md (1 hunks)
  • src/Ext/Rules/Body.cpp (2 hunks)
  • src/Ext/Rules/Body.h (2 hunks)
  • src/Ext/Techno/Hooks.cpp (2 hunks)
Additional comments: 9
src/Ext/Rules/Body.h (2)
  • 113-116: The addition of new properties DrawInsigniaOnlyOnSelected, Promote_VeteranAnimation, Promote_EliteAnimation, and Promote_AnimationVisibility to the RulesExt::ExtData class is correctly implemented. These properties are appropriately declared and initialized in the constructor. It's good practice to ensure that the default values align with the expected behavior of these features in the game. The use of Valueable<bool> for boolean flags and Nullable<AnimTypeClass*> for animation types is appropriate, providing flexibility for optional configurations.
  • 180-183: The initialization of the newly added properties in the ExtData constructor is correctly done, with sensible default values (false for DrawInsigniaOnlyOnSelected and AffectedHouse::All for Promote_AnimationVisibility). This ensures that the new features are disabled by default and animations are visible to all houses unless specified otherwise in the game's configuration files. This approach maintains backward compatibility and allows for easy customization.
src/Ext/Techno/Hooks.cpp (2)
  • 383-386: The conditional logic implemented in the TechnoClass_DrawExtras_Insignia function correctly checks the DrawInsigniaOnlyOnSelected global rule and the selection status of the unit. This ensures that insignias are drawn only when a unit is selected if the rule is enabled, aligning with the PR objectives. The implementation is clear and follows best practices for conditional checks.
  • 459-477: The new hook TechnoClass_Update_TurretAndVeterancy correctly implements conditional animations based on the unit's veterancy level and the global visibility settings for promotion animations. The logic checks whether the unit's current ranking has changed and plays the appropriate animation if the conditions are met. This addition enhances the game's visual feedback for unit promotions, aligning with the PR objectives. However, ensure that the Promote_EliteAnimation and Promote_VeteranAnimation properties are correctly handled when null, avoiding potential null dereferences.
CREDITS.md (1)
  • 317-317: The addition of "Promotion animation" under Fryone's contributions is correctly formatted and accurately represents a contribution to the project.
src/Ext/Rules/Body.cpp (1)
  • 139-142: The addition of new properties DrawInsigniaOnlyOnSelected, Promote_VeteranAnimation, Promote_EliteAnimation, and Promote_AnimationVisibility to RulesExt::ExtData is correctly implemented. These properties follow the existing pattern for loading configuration values from INI files, ensuring consistency with the rest of the codebase.
docs/Whats-New.md (1)
  • 362-362: The addition of the "Promotion animation (by Fryone)" feature under the "New" section for the "Version TBD" changelog is clear and consistent with the document's format. This update provides valuable information about the new feature and its contributor.
docs/Fixed-or-Improved-Logics.md (1)
  • 435-435: The addition of the ability to control the appearance of insignias on selected units using DrawInsigniaOnlyOnSelected is a significant enhancement. It allows for greater customization and can improve the player's visual experience by reducing clutter. This change aligns well with the PR's objectives of enhancing visual customization options for units.
docs/New-or-Enhanced-Logics.md (1)
  • 988-1002: The documentation for promotion animations is clear and provides necessary details about the new feature. It explains how to specify animations for veteran and elite promotions and introduces a property for customizing animation visibility from other houses. This addition should enhance the game's visual feedback for unit promotions, making it more engaging for players.

@Fryone Fryone changed the title Option to draw insiginas only when unit is selected. Insignias drawing customization. Animations on techno promotion. Mar 2, 2024
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 02111e6 and 27c241a.
Files selected for processing (5)
  • docs/Fixed-or-Improved-Logics.md (1 hunks)
  • src/Ext/Rules/Body.cpp (2 hunks)
  • src/Ext/Rules/Body.h (2 hunks)
  • src/Ext/Techno/Body.Visuals.cpp (1 hunks)
  • src/Ext/Techno/Hooks.cpp (2 hunks)
Files skipped from review as they are similar to previous changes (3)
  • docs/Fixed-or-Improved-Logics.md
  • src/Ext/Rules/Body.cpp
  • src/Ext/Techno/Hooks.cpp
Additional comments: 3
src/Ext/Rules/Body.h (2)
  • 113-118: The addition of new properties related to insignia display and promotion animations (DrawInsignia_OnlyOnSelected, DrawInsignia_AdjustPos_Infantry, DrawInsignia_AdjustPos_Techno, Promote_VeteranAnimation, Promote_EliteAnimation, and Promote_AnimationVisibility) in the RulesExt::ExtData class is correctly implemented. These properties are well-defined and follow the naming conventions and data types consistent with the rest of the class. The use of Valueable and Nullable types for these properties is appropriate, allowing for default values and the ability to specify null values where applicable.
  • 182-187: The initialization of the newly added properties in the ExtData constructor is correctly implemented. Default values are set appropriately for each property, ensuring that the game behaves as expected when these properties are not explicitly defined in the INI files. This approach maintains consistency with the rest of the ExtData constructor's initialization logic and ensures that the new features can be optionally used without breaking existing functionality.
src/Ext/Techno/Body.Visuals.cpp (1)
  • 215-217: The logic for adjusting the offset position for drawing insignias based on the type of pThis (Techno or Infantry) is correctly implemented. The use of RulesExt::Global()->DrawInsignia_AdjustPos_Techno.Get() and RulesExt::Global()->DrawInsignia_AdjustPos_Infantry.Get() simplifies the code by directly accessing the relevant properties from the global RulesExt instance. This change enhances code readability and maintainability by utilizing centralized configuration values instead of hard-coded values. The conditional check (pThis->WhatAmI() != AbstractType::Infantry) is correctly used to distinguish between Techno and Infantry types.

Fryone added 3 commits May 6, 2024 21:59
- separated offset for buildings
- some refactoring for Promoting Animation
- fix docs
@Fryone Fryone marked this pull request as ready for review July 7, 2024 10:31
@Metadorius Metadorius self-assigned this Jul 8, 2024
@Metadorius Metadorius self-requested a review July 8, 2024 09:58
src/Ext/Techno/Hooks.cpp Outdated Show resolved Hide resolved
docs/New-or-Enhanced-Logics.md Show resolved Hide resolved
Metadorius and others added 2 commits July 8, 2024 22:08
- excluded vet promotion anim from using for elite promotion
- building insignias anchoring
docs/Whats-New.md Outdated Show resolved Hide resolved
@Metadorius Metadorius merged commit a94131e into Phobos-developers:develop Jul 10, 2024
3 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants