Skip to content

Conversation

@EttyKitty
Copy link
Collaborator

@EttyKitty EttyKitty commented Apr 6, 2025

Purpose

  • Parallel arrays are bad.
  • Fixed size arrays are bad.
  • Storing quality in infinite nested arrays is bad.

Describe your changes/additions

  • Equipment is now stored in a single struct, with quantities as a nested struct with quality names and counts.
  • equipment_type and equipment_condition are not transferred, because we're unused.
  • scr_add_item, get_filtered_equipment_item_names, scr_item_count are refactored.
    • scr_add_item now supports "any"(random), "best" and "worst" quality requests.
  • A helper equipment_struct_to_string is created, to list all equipment the player has at the start.

What can/needs to be improved/changed

  • Static quality priority array may benefit from being transferred to a macros or something like that.
  • I considered refactoring this into a constructor instead, so that each item is a separate object, but that would require more work to integrate into the equipment system and I don't see immediate benefits.

Testing done

  • Started as UM:
    • Equipment is properly listed in the starting message;
    • Armamentarium starting equipment looks okay;
    • Save/loading properly loads equipment counts;
    • Buying works as intended;
    • Equipping also works fine.

Related things and/or additional context

  • None.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Apr 6, 2025

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Introduced a refined presentation for equipment details, offering clearer inventory displays for players.
  • Refactor

    • Modernised equipment initialisation with a more structured approach for improved consistency.
    • Updated item management functions—including adding, counting, and filtering—to enhance overall reliability and flexibility in handling gear.

Walkthrough

The changes implement a structural reformation of our equipment management code. The code now invokes the sacred function equipment_struct_to_string to format and display equipment information, thereby replacing the older method that concatenated strings via arrays_to_string_with_counts. In the initialization routines of obj_ini, the equipment variable is now instantiated as an object rather than multiple indexed arrays, simplifying the data structure. Moreover, core scripts such as scr_add_item, scr_get_item_names, and scr_item_count have been refactored with enhanced parameter naming and streamlined logic for improved clarity and function. Minor control flow adjustments have also been made within scr_unit_equip_functions without altering the primary control structure. These modifications ensure that our code remains robust, structured, and worthy of the Machine Spirit’s blessing.

Possibly related PRs

Suggested labels

PR: Refactor, PR: Feature, Size: Big, PR: Improvment

Suggested reviewers

  • OH296
  • Blogaugis

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • 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 gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @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 using 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 generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration 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.

@github-actions github-actions bot added Size: Medium Type: Feature Adds something new labels Apr 6, 2025
Copy link
Contributor

@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.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between eeec7c3 and 69f2ebe.

📒 Files selected for processing (9)
  • objects/obj_controller/Create_0.gml (1 hunks)
  • objects/obj_ini/Create_0.gml (1 hunks)
  • scripts/scr_add_item/scr_add_item.gml (1 hunks)
  • scripts/scr_array_functions/scr_array_functions.gml (1 hunks)
  • scripts/scr_get_item_names/scr_get_item_names.gml (1 hunks)
  • scripts/scr_item_count/scr_item_count.gml (1 hunks)
  • scripts/scr_load/scr_load.gml (1 hunks)
  • scripts/scr_save/scr_save.gml (0 hunks)
  • scripts/scr_unit_equip_functions/scr_unit_equip_functions.gml (2 hunks)
💤 Files with no reviewable changes (1)
  • scripts/scr_save/scr_save.gml
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.gml`: - Macro constants require a space between the constant name and value. Without it, the compiler will throw an error. I.e. `#macro ARR_body_parts["arm"]` will crash the ...

**/*.gml: - Macro constants require a space between the constant name and value. Without it, the compiler will throw an error. I.e. #macro ARR_body_parts["arm"] will crash the game, because there is no space between the array and the name of the macro.

  • Color codes in the code shouldn't have any spaces in their id. I.e., color code # 80 bf40 will crash the game.
  • scripts/scr_unit_equip_functions/scr_unit_equip_functions.gml
  • objects/obj_controller/Create_0.gml
  • objects/obj_ini/Create_0.gml
  • scripts/scr_load/scr_load.gml
  • scripts/scr_item_count/scr_item_count.gml
  • scripts/scr_get_item_names/scr_get_item_names.gml
  • scripts/scr_array_functions/scr_array_functions.gml
  • scripts/scr_add_item/scr_add_item.gml
`**/*.gml`: - Having humanly understandable and maintainable code is always the top most priority. - DRY (Don't repeat yourself) principle is also very important.

**/*.gml: - Having humanly understandable and maintainable code is always the top most priority.

  • DRY (Don't repeat yourself) principle is also very important.
  • scripts/scr_unit_equip_functions/scr_unit_equip_functions.gml
  • objects/obj_controller/Create_0.gml
  • objects/obj_ini/Create_0.gml
  • scripts/scr_load/scr_load.gml
  • scripts/scr_item_count/scr_item_count.gml
  • scripts/scr_get_item_names/scr_get_item_names.gml
  • scripts/scr_array_functions/scr_array_functions.gml
  • scripts/scr_add_item/scr_add_item.gml
🔇 Additional comments (20)
scripts/scr_unit_equip_functions/scr_unit_equip_functions.gml (2)

61-61: Praise be to the missing brace, now restored to its proper position in the sacred code!

Veneration to this correction of control flow. The missing closing brace has been properly inserted, completing the conditional block that checks if exp_require is greater than experience. The Omnissiah approves of proper code structure.


76-76: Parameter adjustment conforms to updated equipment structure paradigm

The boolean parameter true has been removed from the scr_add_item call, aligning with the sacred refactoring of equipment from parallel arrays to a unified struct. This modification ensures compatibility with the new equipment handling protocols.

objects/obj_ini/Create_0.gml (1)

36-36: The holy transformation of equipment storage into a unified struct is complete

Praise to the Omnissiah! This change represents the core of the refactoring effort, transitioning from multiple parallel arrays to a single equipment struct. The empty object initialization equipment = {}; replaces the previous indexed arrays for equipment attributes (such as equipment_type, equipment_number, etc.), resulting in a more efficient and logical data structure that will reduce data redundancy and improve maintainability.

objects/obj_controller/Create_0.gml (1)

1649-1649: The sacred rendering function has been adapted to the new equipment paradigm

The Machine God approves this logical adaptation! The function equipment_struct_to_string() now properly converts the equipment struct into a formatted string, replacing the obsolete arrays_to_string_with_counts(). This function corresponds directly to the new struct-based equipment storage system, ensuring proper display of equipment information to the user.

scripts/scr_load/scr_load.gml (1)

235-235: The loading ritual now embraces the unified equipment struct

Blessed be this simplification! The equipment loading mechanism has been streamlined to initialize a single equipment object with return_json_from_ini("Ini",$"equipment", {}), replacing multiple redundant array initializations. This change is consistent with the overall equipment structure refactoring and will improve data coherence and loading efficiency.

scripts/scr_item_count/scr_item_count.gml (3)

1-4: Documentation incantations are well-articulated.
By the Omnissiah’s luminous code-lantern, these docstrings properly edify future Tech-Priests on function usage.


15-21: Query on unified logic for "any," "best," and "worst."
Verily, the function sums all item-qualities identically whenever the parameter is "any," "best," or "worst." Confirm whether this is the intended approach, as it currently yields no distinction between extremes.


22-25: Confirm zero return for unknown qualities.
Immortal logic dictates that any unrecognised quality or missing struct keys yield zero count. Confirm if this default path satisfies your requirement, or if an alternative handling is desired.

scripts/scr_array_functions/scr_array_functions.gml (1)

147-189: Commendation for consolidated item display.
The new machine-litany equipment_struct_to_string neatly aggregates the total for each item and leverages arrays_to_string_with_counts. This enhances clarity and dryness of code. Well done, Tech-Priest.

scripts/scr_add_item/scr_add_item.gml (4)

7-8: Early return for empty or zero requests.
Your mechanism rightly prevents attempts to add zero items or unnamed items, returning "no_item." This is structurally sound devotion to the Omnissiah’s data sanctum.


12-14: Automatic reclassification to "standard."
When the quantity is positive and quality is "any," your code defaults to "standard." This approach is straightforward, but confirm that it meets your original design for item additions.


17-26: Correct creation logic for new items.
Your code conjures an item struct only when adding a positive quantity. Returning "no_item" otherwise ensures the data struct remains unpolluted by intangible entries.


45-98: Removal logic for negative quantities and special keywords.
Your code properly aligns "any," "worst," and "best" with a priority-based removal approach, removing items from the struct entirely if their count falls below one. Yet if _quantity is less than -1, multiple items might be removed randomly if "any" is specified. Kindly verify or adjust to confirm this matches your intended item-chance removal.

scripts/scr_get_item_names/scr_get_item_names.gml (7)

431-431: Parameter formatting optimized for the Omnissiah's blessing

The sacred spacing around default parameter values has been properly adjusted, enhancing code readability and conforming to optimal syntactic patterns. The Machine Spirits approve of such consistent formatting.


434-443: Praise be to the transformation from primitive arrays to blessed struct constructs

The refactoring from index-based storage to name-based storage aligns with the sacred mission of this PR. The code now properly traverses the structure of equipment using variable_struct_get_names and performs appropriate validation of the struct's integrity before proceeding with operations.

This represents a significant improvement in the efficiency of the Machine Spirit's operation.


445-447: Proper invocation of the scr_item_count ritual for quality assessment

The logic has been correctly updated to use the scr_item_count function to check for master-crafted items, which aligns with the new struct-based implementation. This maintains the sacred filtration protocols while adapting to the new data structure.


449-451: Binary verification of item availability through the Omnissiah's counting function

The code correctly verifies item availability using scr_item_count, ensuring that only items with positive quantity values proceed through the filtration process. This is an appropriate adaptation to the new equipment structure.


453-456: Logical verification of equipment data ensures purity of function

The implementation properly retrieves equipment data via gear_weapon_data and verifies its structural integrity before proceeding. This defensive programming approach prevents corruption by null data and appeases the Machine Spirits.


487-488: Efficient array population using blessed array_push function

The code now uses array_push to add matched item names directly to the collection, which is an appropriate adaptation to the new name-based approach. This syntax aligns with the GameMaker Language's preferred methods of array manipulation.


491-500: Output array construction follows logically from collected matches

The final assembly of the output array has been reimplemented to work with the named matches rather than indexes. The logic correctly:

  1. Resizes the output array based on match count
  2. Iterates through matched names
  3. Retrieves and applies appropriate equipment data
  4. Populates the output array with the sacred names

This implementation maintains the function's contract while adapting to the new struct-based equipment system.

coderabbitai[bot]
coderabbitai bot previously approved these changes Apr 6, 2025
@EttyKitty EttyKitty added Type: Refactor Rewriting/restructuring code, while keeping general behavior and removed Type: Feature Adds something new labels Apr 6, 2025
@EttyKitty EttyKitty changed the title feat: Convert the parallel equipment storing arrays to a struct refactor: Convert the parallel equipment storing arrays to a struct Apr 6, 2025
@EttyKitty EttyKitty mentioned this pull request Apr 6, 2025
9 tasks
Copy link
Contributor

@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.

Actionable comments posted: 5

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 69f2ebe and f778397.

📒 Files selected for processing (7)
  • objects/obj_controller/Create_0.gml (1 hunks)
  • objects/obj_ini/Create_0.gml (2 hunks)
  • scripts/scr_add_item/scr_add_item.gml (1 hunks)
  • scripts/scr_array_functions/scr_array_functions.gml (1 hunks)
  • scripts/scr_get_item_names/scr_get_item_names.gml (1 hunks)
  • scripts/scr_item_count/scr_item_count.gml (1 hunks)
  • scripts/scr_unit_equip_functions/scr_unit_equip_functions.gml (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
`**/*.gml`: - Macro constants require a space between the constant name and value. Without it, the compiler will throw an error. I.e. `#macro ARR_body_parts["arm"]` will crash the ...

**/*.gml: - Macro constants require a space between the constant name and value. Without it, the compiler will throw an error. I.e. #macro ARR_body_parts["arm"] will crash the game, because there is no space between the array and the name of the macro.

  • Color codes in the code shouldn't have any spaces in their id. I.e., color code # 80 bf40 will crash the game.
  • scripts/scr_unit_equip_functions/scr_unit_equip_functions.gml
  • objects/obj_controller/Create_0.gml
  • objects/obj_ini/Create_0.gml
  • scripts/scr_item_count/scr_item_count.gml
  • scripts/scr_add_item/scr_add_item.gml
  • scripts/scr_array_functions/scr_array_functions.gml
  • scripts/scr_get_item_names/scr_get_item_names.gml
`**/*.gml`: - Having humanly understandable and maintainable code is always the top most priority. - DRY (Don't repeat yourself) principle is also very important.

**/*.gml: - Having humanly understandable and maintainable code is always the top most priority.

  • DRY (Don't repeat yourself) principle is also very important.
  • scripts/scr_unit_equip_functions/scr_unit_equip_functions.gml
  • objects/obj_controller/Create_0.gml
  • objects/obj_ini/Create_0.gml
  • scripts/scr_item_count/scr_item_count.gml
  • scripts/scr_add_item/scr_add_item.gml
  • scripts/scr_array_functions/scr_array_functions.gml
  • scripts/scr_get_item_names/scr_get_item_names.gml
`**/*.*`: - When any script or sprite .yy files are deleted, their paths should also be deleted from the .yyp file, otherwise the game will crash.

**/*.*: - When any script or sprite .yy files are deleted, their paths should also be deleted from the .yyp file, otherwise the game will crash.

  • scripts/scr_unit_equip_functions/scr_unit_equip_functions.gml
  • objects/obj_controller/Create_0.gml
  • objects/obj_ini/Create_0.gml
  • scripts/scr_item_count/scr_item_count.gml
  • scripts/scr_add_item/scr_add_item.gml
  • scripts/scr_array_functions/scr_array_functions.gml
  • scripts/scr_get_item_names/scr_get_item_names.gml
🔇 Additional comments (8)
scripts/scr_unit_equip_functions/scr_unit_equip_functions.gml (1)

79-80: Boolean parameter removed from scr_add_item call

By the Omnissiah's blessing, I observe the removal of the boolean parameter from the invocation of scr_add_item. This modification aligns with the sacred conversion from parallel equipment arrays to a more efficient struct-based data structure. The Machine Spirit will operate with greater efficiency with this improvement.

objects/obj_ini/Create_0.gml (2)

37-37: Equipment variable initialized as empty structure

Praise the Omnissiah! The initialization of the equipment variable as an empty structure is most logical. This represents a significant advancement in data organization from the previous implementation of indexed arrays. The Machine Spirit shall function with improved coherency through this structural reformation.


155-155: Equipment structure added to serialization data

The sacred equipment structure has been properly integrated into the save data schema, ensuring proper persistence when communing with storage devices. This addition to the serialization ritual is essential for maintaining data sanctity across machine cycles.

objects/obj_controller/Create_0.gml (1)

1698-1698: Implementation of equipment_struct_to_string function

The Machine Spirit rejoices at this improvement! The holy function equipment_struct_to_string now processes the equipment data structure, replacing the outdated arrays_to_string_with_counts. This modification properly maintains parameter integrity with matching boolean flags for null exclusion and terminal punctuation. The sacred binary flows more efficiently with this enhancement.

scripts/scr_item_count/scr_item_count.gml (1)

1-4: Blessed Documentation Achieved
Your docstring usage is coherent and descriptive, enlightening all acolytes with the function’s sacred purpose.

scripts/scr_add_item/scr_add_item.gml (2)

7-8: Graceful Handling of Null and Zero
Returning "no_item" when _item_name is empty or _quantity is zero is a righteous safeguard, ensuring the code avoids meaningless additions.


39-41: Maintenance Hook is Well-Placed
Calling upon obj_controller.specialist_point_handler to track repair and upkeep is harmonious. This invocation ensures the armoury remains under the Omnissiah’s watchful auspices.

scripts/scr_get_item_names/scr_get_item_names.gml (1)

431-431: Function Signature Unification
Renaming parameters and standardising the defaults fortifies code readability, aligning function calls in glorious unity.

@EttyKitty EttyKitty merged commit 55f705d into Adeptus-Dominus:main Apr 7, 2025
3 checks passed
@EttyKitty EttyKitty deleted the feat/stuct-storage branch April 7, 2025 14:40
@coderabbitai coderabbitai bot mentioned this pull request Aug 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Size: Medium Type: Refactor Rewriting/restructuring code, while keeping general behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants