-
-
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
Allow customizing aircraft landing/docking direction #1149
Allow customizing aircraft landing/docking direction #1149
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. |
26d4320
to
09d264e
Compare
f018696
to
6d51eec
Compare
it would be nice to be able to set this on a per-dock basis rather than a per-building one. |
6d51eec
to
21bb096
Compare
WalkthroughThe project has seen significant enhancements in aircraft landing and docking functionalities. These updates introduce customizable landing directions for aircraft, both globally and per dock, respecting specific settings in Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
21bb096
to
43f97fc
Compare
This has now been implemented. |
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 (11)
- CREDITS.md (1 hunks)
- docs/Fixed-or-Improved-Logics.md (3 hunks)
- docs/New-or-Enhanced-Logics.md (1 hunks)
- docs/Whats-New.md (2 hunks)
- src/Ext/Aircraft/Body.cpp (2 hunks)
- src/Ext/Aircraft/Body.h (1 hunks)
- src/Ext/Aircraft/Hooks.cpp (2 hunks)
- src/Ext/BuildingType/Body.cpp (2 hunks)
- src/Ext/BuildingType/Body.h (2 hunks)
- src/Ext/TechnoType/Body.cpp (2 hunks)
- src/Ext/TechnoType/Body.h (2 hunks)
Additional comments: 21
src/Ext/Aircraft/Body.h (1)
- 11-11: LGTM!
src/Ext/Aircraft/Body.cpp (2)
- 3-5: Ensure these includes are used within the file to avoid unnecessary dependencies.
- 58-102: - Validate the logic for determining the landing direction, especially the handling of negative values and the default pose direction.
- Check the use of
Math::clamp
for correctness in the context of direction values.- Ensure that the handling of
pDock
andpThis->HasAnyLink()
covers all intended scenarios.src/Ext/Aircraft/Hooks.cpp (4)
- 117-133: Verify the correctness of the landing direction calculation in
FlyLocomotionClass_FlightUpdate_LandingDir
hook and its impact on gameplay.- 136-143: Ensure
BuildingClass_GrandOpening_PoseDir
hook correctly overrides the aircraft's landing direction with the building's setting.- 146-151: Check
BuildingClass_ExitObject_PoseDir1
for correct application of landing direction upon aircraft exit.- 156-165: Confirm that
BuildingClass_ExitObject_PoseDir2
correctly adjusts aircraft facing based on the landing direction.src/Ext/BuildingType/Body.h (1)
- 64-64: Ensure
AircraftDockingDirs
is correctly initialized and used where necessary to manage aircraft docking directions.src/Ext/BuildingType/Body.cpp (2)
- 150-170: Review the logic for initializing
AircraftDockingDirs
from the INI file for correctness, especially handling of individual dock settings.- 259-259: Ensure serialization of
AircraftDockingDirs
correctly handles all scenarios, including empty and default values.src/Ext/TechnoType/Body.h (1)
- 184-184: Confirm that
LandingDir
is correctly utilized in the context of aircraft landing direction customization.CREDITS.md (1)
- 222-222: LGTM!
src/Ext/TechnoType/Body.cpp (2)
- 275-275: LGTM!
- 574-574: LGTM!
docs/Whats-New.md (2)
- 359-359: The documentation clearly explains the new feature allowing customization of aircraft landing direction per aircraft or per dock. Ensure that examples or references to how to use this feature (e.g., code snippets or property names) are provided elsewhere in the documentation for clarity.
- 413-413: The documentation correctly addresses the change in default behavior for aircraft docking on buildings, now respecting the
[AudioVisual]
->PoseDir
setting. This change aligns with the feature's goal to enhance customization and should be clearly communicated to modders.docs/Fixed-or-Improved-Logics.md (4)
- 149-149: The addition of customizable aircraft landing direction based on
[AudioVisual]
->PoseDir
is consistent with the PR objectives. Ensure that the implementation details are thoroughly documented to guide developers and modders on how to use this feature effectively.- 172-179: The introduction of the
LandingDir
property for customizable aircraft landing direction per AircraftType, including support for negative values forAirportBound=false
aircraft, aligns with the PR objectives. This enhancement significantly increases the flexibility and realism of aircraft behavior in the game.- 255-261: The implementation of
AircraftDockingDir(N)
for customizing the landing direction of docking aircraft at buildings is a significant enhancement. It provides game developers and modders with finer control over aircraft behavior, fulfilling the PR objectives. Ensure that the documentation clearly explains how to use this feature, including the use of the optional indexN
for differentDockingOffset
s.- 167-183: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-262]
The comprehensive list of fixes and improvements across various game mechanics, including but not limited to bug fixes, enhancements to existing features, and the introduction of new properties and behaviors, significantly contributes to the overall quality and flexibility of the game. It's crucial to ensure that these changes are well-documented and that developers and modders are provided with clear guidance on how to utilize the new features effectively.
docs/New-or-Enhanced-Logics.md (1)
- 264-264: The
Facing
parameter in theCreateUnit
declaration is updated to specify it as a "Direction type" with integers ranging from 0 to 255. Ensure this change is reflected in all relevant code whereCreateUnit
is used, and verify that the handling of this parameter is correctly implemented to support the full range of integers.
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/Aircraft/Body.cpp (2 hunks)
Additional comments: 2
src/Ext/Aircraft/Body.cpp (2)
- 3-5: The addition of includes for
BuildingClass.h
and<Ext/BuildingType/Body.h>
is necessary for the new functionality introduced inGetLandingDir
. Ensure these includes are used elsewhere in the file to justify their addition, avoiding unnecessary dependencies.- 58-105: The
GetLandingDir
function introduces customizable landing directions for aircraft, considering various conditions such as the aircraft's owner, the dock it's landing at, and whether it's airport-bound. A few observations and suggestions:
Error Handling: The function begins by checking if
pThis
isnullptr
, returning a default direction if so. This is good practice for robustness. However, consider logging or handling this scenario more explicitly if it's unexpected forpThis
to benullptr
.Use of
SpawnOwner
: The addition based on the past comment to return theSpawnOwner
's direction if present is a nice touch for aligning spawned aircraft with their owner's facing. This aligns with the feedback and should be tested thoroughly to ensure it behaves as expected in all scenarios.Dock and Link Handling: The logic to determine the landing direction based on the dock or linked building is well-structured. It accounts for multiple docks and defaults to the first dock's direction if the specific link index isn't found. This is a flexible approach that supports the customization goals of the PR.
Airport-Bound Check: The check for whether the aircraft is airport-bound and the subsequent logic to determine the landing direction based on
LandingDir
or the aircraft's primary facing is clear and aligns with the PR's objectives.Performance Considerations: The function involves several conditional checks and potentially multiple lookups in extension maps. While this is necessary for the feature's flexibility, consider the performance impact in scenarios with many aircraft and docks. Optimizations or caching strategies might be beneficial if performance becomes an issue.
Code Clarity: The function is generally well-commented and structured, making it relatively easy to follow the logic. Additional comments explaining the rationale behind certain decisions (e.g., why negative
LandingDir
values lead to using the aircraft's primary facing) could further improve clarity.Overall, the implementation appears to align well with the PR's objectives, enhancing aircraft landing and docking mechanics with customizable directions.
a4139d5
to
669391c
Compare
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: 3
Configuration used: CodeRabbit UI
Files selected for processing (11)
- CREDITS.md (1 hunks)
- docs/Fixed-or-Improved-Logics.md (3 hunks)
- docs/New-or-Enhanced-Logics.md (1 hunks)
- docs/Whats-New.md (2 hunks)
- src/Ext/Aircraft/Body.cpp (2 hunks)
- src/Ext/Aircraft/Body.h (1 hunks)
- src/Ext/Aircraft/Hooks.cpp (2 hunks)
- src/Ext/BuildingType/Body.cpp (2 hunks)
- src/Ext/BuildingType/Body.h (2 hunks)
- src/Ext/TechnoType/Body.cpp (2 hunks)
- src/Ext/TechnoType/Body.h (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/Ext/Aircraft/Body.cpp
Additional comments: 13
src/Ext/Aircraft/Body.h (1)
- 11-11: The addition of
GetLandingDir
inAircraftExt
introduces customizable landing directions. Ensure the implementation gracefully handles nullpDock
pointers, given its optional nature. This design choice enhances flexibility in determining aircraft landing behavior, aligning with the PR's objectives.src/Ext/Aircraft/Hooks.cpp (1)
- 117-166: The integration of
GetLandingDir
through various hooks inHooks.cpp
is well-executed, strategically enhancing aircraft landing direction logic. It's crucial to thoroughly test these changes to ensure the new customizable directions behave as expected, especially in edge cases or with complex docking configurations.src/Ext/BuildingType/Body.h (1)
- 64-64: The introduction of
AircraftDockingDirs
inBuildingTypeExt::ExtData
is a well-thought-out design choice, enabling customizable aircraft docking directions. Ensure this new variable is properly handled in serialization and deserialization processes to maintain data integrity across game sessions.src/Ext/BuildingType/Body.cpp (2)
- 150-170: The updates to
LoadFromINIFile
for handlingAircraftDockingDirs
are correctly implemented, ensuring custom docking directions are loaded from INI files as intended. It's important to thoroughly test these changes to confirm that docking directions are accurately loaded and applied in-game.- 259-259: The inclusion of
AircraftDockingDirs
in theSerialize
method ensures proper serialization and deserialization of custom docking directions. This is crucial for maintaining data integrity across game sessions. Ensure comprehensive testing to verify that serialization processes work as expected.src/Ext/TechnoType/Body.h (2)
- 184-184: The addition of
Nullable<int> LandingDir;
toTechnoTypeExt::ExtData
introduces customizable landing directions for aircraft, aligning with the PR objectives. This change allows for greater flexibility in game mechanics and modding capabilities. Ensure that the corresponding logic to utilize this property effectively in aircraft landing behavior is implemented elsewhere in the codebase.- 363-363: The initialization of
LandingDir
within theExtData
constructor is correctly set to an empty state, indicating its optional nature. This approach is consistent with the use ofNullable
types in the class, allowing for optional customization of aircraft landing directions without mandating a default value.CREDITS.md (1)
- 224-224: The addition of "Aircraft landing / docking direction" under Starkku's contributions in the
CREDITS.md
file appropriately acknowledges the enhancement to aircraft behavior customization. This update is crucial for maintaining transparency and giving credit where it's due, fostering a collaborative and appreciative development environment.src/Ext/TechnoType/Body.cpp (1)
- 573-573: The inclusion of
LandingDir
in theSerialize
template method ensures that the property is correctly processed during serialization and deserialization. It's important to verify that all relevant serialization tests have been updated or added to cover this new property, ensuring its correct behavior across game saves and loads.docs/Fixed-or-Improved-Logics.md (3)
- 148-149: The addition of customizable aircraft docking and landing directions based on
[AudioVisual]
->PoseDir
is a significant improvement. It enhances gameplay customization and realism by allowing aircraft to land facing the direction specified by the game's visual settings or the spawner's facing. This change aligns with the PR objectives and should positively impact the game's dynamics.- 172-179: The implementation of customizable landing directions for aircraft via the
LandingDir
property inrulesmd.ini
is a thoughtful addition. Allowing negative values forAirportBound=false
aircraft is a clever way to handle non-airport-bound aircraft, providing flexibility in gameplay design. This feature should be well-received by developers and modders looking to fine-tune aircraft behavior.- 257-263: Introducing the
AircraftDockingDir(N)
property for dock buildings to customize aircraft docking directions is a valuable feature. It enables more precise control over how aircraft interact with docking buildings, enhancing strategic gameplay elements. This customization option, which overrides the aircraft's own landing direction setting, is a welcome addition to the game's mechanics.docs/New-or-Enhanced-Logics.md (1)
- 269-269: The
CreateUnit.Facing
parameter's type has been updated to aDirection
type, which is represented by integers from 0 to 255. This change allows for more precise control over the facing direction of units created by animations, enhancing gameplay and visual consistency.
[AudioVisual]
->PoseDir
as the default setting and do not always land facing north or in case of pre-placed buildings, the building's direction.Landing direction
[AudioVisual]
->PoseDir
. This can now be customized per AircraftType viaLandingDir
, defaults to[AudioVisual]
->PoseDir
. If the building the aircraft is docking to has aircraft docking direction set, that setting takes priority over this.AirportBound=false
aircraft which makes them land facing their current direction.In
rulesmd.ini
:Aircraft docking direction
Aircraft docking direction
AircraftDockingDir(N)
(N
optionally replaced by 0-based index for eachDockingOffset
separately,AircraftDockingDir
andAircraftDockingDir0
are synonymous and will be used if direction is not set for a specific offset) on the dock building. This overrides the aircraft's own landing direction setting and defaults to[AudioVisual]
->PoseDir
.In
rulesmd.ini
:Summary by CodeRabbit
AmbientDamage
warhead & main target ignore customization.CreateUnit
declaration to specifyFacing
parameter as a "Direction type".