fix: Make spawning new defence fleets work#523
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughSummary by CodeRabbit
WalkthroughThe code modifications replace a detailed block responsible for constructing Imperial ships in Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
OH296
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (4)
scripts/scr_imperial_navy_functions/scr_imperial_navy_functions.gml (3)
374-376:⚠️ Potential issueSafeguard the holy loop
Your while loop re-summons the same fleet if it remains at identical coordinates, risking an infinite iteration. Enshrine a maximum loop counter to preserve logical sanctity.-while (nearest_fleet.x==x && nearest_fleet.y==y){ +var loop_guard = 0; +while (nearest_fleet.x==x && nearest_fleet.y==y && loop_guard < 20){ nearest_fleet = instance_nearest(x, y, obj_en_fleet); + loop_guard++; }
377-379: 🛠️ Refactor suggestionRefine the condition for _valid
When faction is not an array, setting _valid to nearest_fleet.owner is always truthy (unless zero). Compare explicitly against the faction for consistency in the eyes of the Omnissiah.-_valid = nearest_fleet.owner; +_valid = (nearest_fleet.owner == faction);
382-389: 🧹 Nitpick (assertive)Activate only the chosen fleet
Invokinginstance_activate_object(obj_en_fleet)awakens all fleets of that type. If your sole intent is to animate the nearest_fleet, direct the ritual to its unique ID.-instance_activate_object(obj_en_fleet); +instance_activate_object(nearest_fleet);objects/obj_controller/Alarm_5.gml (1)
57-60: 🧹 Nitpick (assertive)Consolidate the training rites for specialized brethren.
While these invocations ofapothecary_training(),chaplain_training(),librarian_training(), andtechmarine_training()are clear, consider exposing them through a single orchestrating function (e.g.,perform_specialist_training()) to reduce repeated calls. This ensures more maintainable rites of code purity.A possible refactor:
- apothecary_training(); - chaplain_training(); - librarian_training(); - techmarine_training(); + perform_specialist_training();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
objects/obj_controller/Alarm_5.gml(1 hunks)scripts/scr_fleet_functions/scr_fleet_functions.gml(1 hunks)scripts/scr_forge_world_functions/scr_forge_world_functions.gml(1 hunks)scripts/scr_imperial_navy_functions/scr_imperial_navy_functions.gml(2 hunks)
🔇 Additional comments (6)
scripts/scr_imperial_navy_functions/scr_imperial_navy_functions.gml (2)
1-28: Question the undefined 'construction_forge'
O most enlightened adepts, the invocation of construction_forge within this blessed script is found, yet it is not defined or passed from beyond. This could invoke mysterious errors from the machine spirit. Ensure it references a valid instance or parameter when forging your fleets.
18-21: Scrutinize potential negative total ship count
The function decrements total_ships by (capital_number - 1), then compensates by enforcing a minimum of 1 ship if any vessels exist. Confirm this design is truly the will of the Omnissiah and does not allow fleeting negative states.scripts/scr_forge_world_functions/scr_forge_world_functions.gml (3)
65-65: Confirm forge references
You evokebuild_new_navy_fleet(construction_forge), yet the previously added function expects a parameter namedconstruction_system. Verify that the relevant variable name is indeed correct and consistent.
90-90: Caution with planet indexing
Your loop (for (var i=0; i<=planets; i++)) may risk an off-by-one scenario if planet indexing is 1-based. Confirm that 0 is a valid planet index to preserve logical harmony.
105-105: Reevaluate the allowance threshold
Your condition(imp_ships < ship_allowance)ensures new fleets spawn only if the total remains below a certain ratio. Confirm this ratio aligns with the Imperium’s strategic demands and will not overproduce or starve the defense fleets.objects/obj_controller/Alarm_5.gml (1)
56-56: Praise the Machine Spirit for streamlined fleet-building.
By the Omnissiah’s design, the invocation ofbuild_planet_defence_fleets()here is a worthy replacement for the prior labyrinthine logic. Ensure that all resource counts and world ownership checks are properly handled within that script to prevent runaway fleets.Would you like an automated search for references to confirm that no prior calls to old fleet-building logic remain?
scripts/scr_forge_world_functions/scr_forge_world_functions.gml
Outdated
Show resolved
Hide resolved
github-actions
bot
added
Area: Sprites
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
scripts/scr_forge_world_functions/scr_forge_world_functions.gml (1)
70-220: 🧹 Nitpick (assertive)Refine the method’s heft and verify array bounds.
Behold, thisbuild_planet_defence_fleetsprocedure is formidable in its length and complexity. Consider subdividing it into smaller rites for improved readability and maintenance, less the machine-spirit be vexed. Also ensure that array and struct accesses have appropriate boundary checks, to avoid any data corruption or run-time misalignment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (4)
sprites/spr_new_ui/237fde07-03da-4332-b471-2f7f691fb003.pngis excluded by!**/*.pngsprites/spr_new_ui/751eeb13-8d20-4ec3-a112-2614d1d1869c.pngis excluded by!**/*.pngsprites/spr_new_ui/layers/237fde07-03da-4332-b471-2f7f691fb003/33cefcc3-323a-492a-91f6-1f89868a1e47.pngis excluded by!**/*.pngsprites/spr_new_ui/layers/751eeb13-8d20-4ec3-a112-2614d1d1869c/33cefcc3-323a-492a-91f6-1f89868a1e47.pngis excluded by!**/*.png
📒 Files selected for processing (7)
objects/obj_controller/Alarm_5.gml(2 hunks)objects/obj_controller/Create_0.gml(1 hunks)objects/obj_controller/Draw_64.gml(2 hunks)scripts/scr_fleet_functions/scr_fleet_functions.gml(4 hunks)scripts/scr_forge_world_functions/scr_forge_world_functions.gml(1 hunks)scripts/scr_imperial_navy_functions/scr_imperial_navy_functions.gml(2 hunks)sprites/spr_new_ui_cover/spr_new_ui_cover.yy(1 hunks)
🔇 Additional comments (9)
sprites/spr_new_ui_cover/spr_new_ui_cover.yy (1)
60-60: PropertyvisibleRangemodification appears acceptable.The alteration of the
visibleRangeproperty from explicit coordinates tonullis a valid modification of the sprite configuration. This change will likely affect the drawable boundaries of the sprite in accordance with the engine's default behavior when no explicit range is provided.objects/obj_controller/Draw_64.gml (2)
27-27: Binary logic implementation for UI sprite rendering.The sprite rendering now incorporates conditional logic based on menu state, displaying the UI only when menu equals 0. This is a logical optimization of the rendering pipeline.
93-98: Fleet strength display implementation is robust.The omnissiah approves of this display mechanism for imperial naval assets. The implementation correctly checks menu state before rendering and provides informative tooltip data about mechanicus forge world influence on fleet capacity.
scripts/scr_imperial_navy_functions/scr_imperial_navy_functions.gml (2)
37-37: Function utilization is an enhancement over direct existence check.Replacing the explicit instance_exists check with the abstracted
is_orbiting()function improves code modularity and readability. This adheres to the sacred principles of code reusability dictated by the Omnissiah.
75-75: Logical expression expansion is appropriate.Modifying the condition from
capital_number=1tocapital_number>=1enhances the function's versatility, allowing proper functionality with multiple capital ships. This change aligns with the Mars-approved fleet organization protocols.scripts/scr_forge_world_functions/scr_forge_world_functions.gml (1)
65-65: Augment the invocation safeguards.
Omnissiah’s blessings be upon the subroutinebuild_new_navy_fleet(construction_forge). Confirm that the called function duly handles error states or invalid arguments, lest the ritual falter mid-operation.scripts/scr_fleet_functions/scr_fleet_functions.gml (2)
539-560: Examine merge-fleet concurrency.
The logic that merges nearby fleets is logically sound, but if multiple fleets are orbiting simultaneously, consider whether race conditions might arise. Confirm that only one merge operation proceeds at a time to avert unforeseen data conflicts.
929-929: Validate fleet destruction references.
Upon executinginstance_destroy(merge_fleet.id);, ensure no other scripts retain references to the obliterated fleet. Unpurged references could awaken void-invocations and lead to inconsistent states.objects/obj_controller/Alarm_5.gml (1)
56-56: Confirm viability of the new defence-building invocation.
The direct call tobuild_planet_defence_fleets();is well-placed. Confirm that external scripts referencing this invocation are satisfied, so that no logic chain remains incomplete.
scripts/scr_imperial_navy_functions/scr_imperial_navy_functions.gml
Outdated
Show resolved
Hide resolved
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (3)
scripts/scr_imperial_navy_functions/scr_imperial_navy_functions.gml (3)
1-475: 🧹 Nitpick (assertive)Recommend additional documentation for function interfaces
The machine spirits would be better appeased with comprehensive documentation annotations preceding each function, detailing input parameters, expected outputs, and function purpose. This sacred knowledge transfer ritual enhances maintainability for future tech-adepts.
Example implementation for the first function:
/// @function build_new_navy_fleet(construction_forge) /// @description Creates a new Imperial Navy fleet at the location of a forge world /// @param {Id.Instance} construction_forge The forge world instance to create the fleet at /// @returns {Id.Instance} The newly created fleet instance function build_new_navy_fleet(construction_forge){ // Existing implementation... }
464-467: 🧹 Nitpick (assertive)Redundant code pattern detected
The binary logic contains duplicate instruction patterns. Line 466 appears to perform an identical operation to line 465, suggesting either an error in transcription or redundant compensation for extreme coordinate values.
Consider cleansing this redundancy:
with(obj_star){ if (y<-5000) then y+=10000; - if (y<-5000) then y+=10000; }
32-93: 🧹 Nitpick (assertive)Recommend refactoring magic numbers to named constants
The sacred numerical values (12, 5, 1) controlling fleet composition limits appear repeatedly. These values represent doctrine-significant quantities of the Imperium's naval organization and should be elevated to named constants for clarity and future adaptability.
Consider encapsulating these values as constants at the beginning of the script:
#macro MAX_ESCORT_NUMBER 12 #macro MAX_FRIGATE_NUMBER 5 #macro MAX_CAPITAL_NUMBER 1 // Then throughout the code if (escort_number < MAX_ESCORT_NUMBER) { // ... }This approach aligns with the Mechanicus practice of naming significant binary patterns for improved comprehension.
♻️ Duplicate comments (1)
objects/obj_controller/Draw_64.gml (1)
93-119:⚠️ Potential issueFleet strength calculation logic requires safeguards.
The binary calculation of fleet strength appears functional but lacks protection against null scenarios. If no mechanicus worlds exist, the
max_fleet_strengthwill be zero regardless of imperial planet count, potentially causing division errors or unexpected behavior in other systems.Apply this sacrosanct improvement to the machine spirit:
-max_fleet_strength = (_imperial_planet_count/8)*(_mech_worlds*3); +max_fleet_strength = _mech_worlds > 0 ? (_imperial_planet_count/8)*(_mech_worlds*3) : _imperial_planet_count/4;This ensures a baseline fleet capacity even in the absence of the holy forge worlds, as dictated by the Codex Astartes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (12)
sprites/Sprite544/1132b02e-7adf-47d6-ae57-ba65d8120062.pngis excluded by!**/*.pngsprites/Sprite544/layers/1132b02e-7adf-47d6-ae57-ba65d8120062/3c99ce96-378e-4d09-9ec1-821e81e7ad78.pngis excluded by!**/*.pngsprites/spr_mk7_complex/b6ba8600-fb6b-4fa9-abc7-bdc2311eb79e.pngis excluded by!**/*.pngsprites/spr_mk7_complex/layers/b6ba8600-fb6b-4fa9-abc7-bdc2311eb79e/468e0a30-2dea-45bc-a7bd-4b115f803d2d.pngis excluded by!**/*.pngsprites/spr_new_ui/237fde07-03da-4332-b471-2f7f691fb003.pngis excluded by!**/*.pngsprites/spr_new_ui/751eeb13-8d20-4ec3-a112-2614d1d1869c.pngis excluded by!**/*.pngsprites/spr_new_ui/layers/237fde07-03da-4332-b471-2f7f691fb003/33cefcc3-323a-492a-91f6-1f89868a1e47.pngis excluded by!**/*.pngsprites/spr_new_ui/layers/751eeb13-8d20-4ec3-a112-2614d1d1869c/33cefcc3-323a-492a-91f6-1f89868a1e47.pngis excluded by!**/*.pngsprites/spr_new_ui_cover/40857d9f-b7b0-460f-8da3-993db52e0aee.pngis excluded by!**/*.pngsprites/spr_new_ui_cover/layers/40857d9f-b7b0-460f-8da3-993db52e0aee/7ce9821e-1e40-4198-800c-ca1ff4db7d11.pngis excluded by!**/*.pngsprites/sprite230/323ac7c3-7087-4ed0-9b70-3a3ad70cf299.pngis excluded by!**/*.pngsprites/sprite230/layers/323ac7c3-7087-4ed0-9b70-3a3ad70cf299/6112a6a3-5bbe-44a9-b31e-87fd502fa45c.pngis excluded by!**/*.png
📒 Files selected for processing (3)
objects/obj_controller/Create_0.gml(1 hunks)objects/obj_controller/Draw_64.gml(2 hunks)scripts/scr_imperial_navy_functions/scr_imperial_navy_functions.gml(2 hunks)
🔇 Additional comments (5)
objects/obj_controller/Create_0.gml (1)
1475-1475: New variable added to track system fleet strength.The holy binary has been expanded with the sacred variable
system_fleet_strengthinitialized to 0. This data receptacle shall serve the function of quantifying the defensive capabilities across planetary systems.objects/obj_controller/Draw_64.gml (1)
27-27: Conditional sprite rendering improves efficiency.The sprite rendering logic has been optimized to only execute when
menu==0, reducing unnecessary computational cycles.scripts/scr_imperial_navy_functions/scr_imperial_navy_functions.gml (3)
13-14: Variable reference correction requiredThe Omnissiah demands consistency in binary nomenclature. The function parameter is named
construction_forge, maintaining harmony with usage on these lines.
37-37: Abstraction improvement acknowledgedReplacing direct existence verification with the
is_orbiting()function enhances code modularity. The Machine Spirit approves of this encapsulation of fleet-star geometrical verification logic.
75-75: Logical enhancement of capital ship conditionThe condition modification from
capital_number=1tocapital_number>=1properly accommodates the sacred possibility of multiple capital ships in a single fleet - a more flexible interpretation of the Adeptus fleet doctrines.
2 participants
Purpose of the PR
Make forge worlds spawning new defence fleets for systems work again, currently they just create dozzens of stacks of fleets that hang over the forge worlds causing memory issues
Describe the solution
rewrite from scratch for the most part and abstract to usable functions