feat: Fix and jsonify traits #512
Conversation
github-actions
bot
added
PR: JSON
EttyKitty
added
PR: Refactor
📝 WalkthroughWalkthroughOh faithful servant of the Machine, observe the alterations: A new entry has been added to the "IncludedFiles" array in the Possibly related PRs
Suggested labels
Suggested reviewers
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
scripts/__init_global/__init_global.gml (2)
9-9: Praise the Machine God! But let us correct this minor lexicographical deviation.There appears to be a spelling anomaly in the sacred function name: "intialise" should be "initialize" to maintain consistency with the holy Machine Cult's naming conventions.
- intialise_marine_traits(); + initialize_marine_traits();
9-9: Implement error handling rituals to appease the Machine Spirit.The initialization of marine traits is a critical operation. Consider implementing proper error handling to ensure the Machine Spirit's stability.
- intialise_marine_traits(); + try { + intialise_marine_traits(); + } catch (error) { + show_debug_message("ERROR: Failed to initialize marine traits - " + string(error)); + // Consider appropriate fallback behavior + }ChapterMaster.yyp (1)
114-114: Praise be to the Omnissiah! The binary configuration has been optimized.The sacred traits data structure has been properly externalized into a dedicated JSON cogitator file, enhancing both modularity and initialization protocols. The placement in the datafiles/data sanctum and the omnidirectional CopyToMask are logically sound.
This separation of data and logic follows the holy SRP (Single Responsibility Principle) as ordained by the Machine God. The JSON format will facilitate future maintenance rituals and modding rites.
scripts/scr_json_functions/scr_json_functions.gml (2)
11-12: Beware of the double declaration, Servitor.
You overshadowfile_bufferdeclared at line 6 by declaring it again at line 12. Use it directly instead of redeclaring to maintain purity in the data stream.Apply this diff to remove the redundant
var:- var file_buffer = buffer_load(_json_path); + file_buffer = buffer_load(_json_path);
18-20: Excessive debugging incantation.
show_debug_message(_json_string)could spawn massive logs if the JSON is extensive. Consider limiting verbosity to preserve system cycles.scripts/scr_unit_traits/scr_unit_traits.gml (1)
5-5: Heed the open TODO, Adept.
A clear vow to further refine JSON usage remains.Shall I forge a new issue to track the next steps of this transition?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
ChapterMaster.yyp(1 hunks)datafiles/data/traits.json(1 hunks)scripts/__init_global/__init_global.gml(1 hunks)scripts/scr_initialize_custom/scr_initialize_custom.gml(1 hunks)scripts/scr_json_functions/scr_json_functions.gml(1 hunks)scripts/scr_load/scr_load.gml(1 hunks)scripts/scr_unit_traits/scr_unit_traits.gml(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- scripts/scr_load/scr_load.gml
- scripts/scr_initialize_custom/scr_initialize_custom.gml
🧰 Additional context used
🪛 Biome (1.9.4)
datafiles/data/traits.json
[error] 946-946: Expected a property but instead found '// Slight buff to health, as clear mind helps to stay healthy in some cases'.
Expected a property here.
(parse)
[error] 947-947: expected , but instead found "technology"
Remove "technology"
(parse)
[error] 947-947: Expected a property but instead found '// Also helps with some boring tasks, I think?'.
Expected a property here.
(parse)
[error] 948-948: expected , but instead found "display_name"
Remove "display_name"
(parse)
[error] 951-951: Expected a property but instead found '// IT IS THE BANEBLAAADE!!! - Captain Diomedes, DoW 2 Retribution'.
Expected a property here.
(parse)
[error] 952-952: expected , but instead found "wisdom"
Remove "wisdom"
(parse)
[error] 954-954: Expected a property but instead found '// Being bonkers can sometimes be damaging to one's health, I think..?'.
Expected a property here.
(parse)
[error] 955-955: expected , but instead found "display_name"
Remove "display_name"
(parse)
[error] 958-958: expected , but instead found // "crazy": {
Remove // "crazy": {
(parse)
🔇 Additional comments (7)
scripts/__init_global/__init_global.gml (1)
8-9: By the Omnissiah's grace, the initialization sequence has been optimized!The strategic placement of the marine traits initialization in
__init_global()ensures proper sanctification of the global trait struct before its first invocation, thus preventing the reported crash. This is a more appropriate location than the previous implementations inscr_initialize_custom()andscr_load().scripts/scr_json_functions/scr_json_functions.gml (4)
6-6: Commendable variable declaration, Acolyte.
By the Omnissiah, initializingfile_bufferhere is a clean approach to ensure clarity in the function’s scope.
14-15: Error checking pleases the Machine God.
Verifyingbuffer_loadsuccess ensures no silent corruptions in the Omnissiah’s data.
26-29: Laudable resource cleanup, O Adept.
Ensuring the buffer is deleted in afinallyblock averts memory leaks and pleases the Machine Spirit.
31-31: No further scrutiny warranted.
The bracket is simply concluding this holy function.datafiles/data/traits.json (1)
958-973: Commented-out traits disrupt the JSON litany.
These lines with//raise parse errors if included. Remove or properly format them if these traits remain part of your vision.Do you wish to preserve these concepts? If so, refactor them into valid JSON or house them in a separate data scripture.
🧰 Tools
🪛 Biome (1.9.4)
[error] 958-958: expected
,but instead found// "crazy": {Remove // "crazy": {
(parse)
scripts/scr_unit_traits/scr_unit_traits.gml (1)
2-2: Blessed conversion to external data.
Loading traits from JSON fosters modular updates and decreases future code churn.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
scripts/__init_global/__init_global.gml(1 hunks)scripts/scr_initialize_custom/scr_initialize_custom.gml(1 hunks)scripts/scr_json_functions/scr_json_functions.gml(1 hunks)scripts/scr_load/scr_load.gml(1 hunks)scripts/scr_unit_traits/scr_unit_traits.gml(1 hunks)
🔇 Additional comments (8)
scripts/__init_global/__init_global.gml (1)
8-9: By the Omnissiah's grace, the initialization sequence is optimal!The initialization of marine traits after setting global defaults follows the sacred order of operations.
scripts/scr_load/scr_load.gml (1)
348-349:✅ Verification successful
Praise be to the Machine Spirit! The initialization timing has been optimized.
The traits are now initialized at compile time through the sacred pragma directive, preventing the crash issue. The TODO comment correctly documents the requirement to re-enable this initialization when modding support is implemented.
Run the following script to verify that traits are initialized only once:
🏁 Script executed:
#!/bin/bash # Description: Verify that traits are initialized only at compile time and not during loading. # Test: Search for all calls to initialize_marine_traits(). Expect: Only one call in __init_global.gml rg -A 5 $'initialize_marine_traits'Length of output: 1505
Exalt the Machine Spirit!
The sacred logs have been consulted, and the initialization ritual of marine traits is indeed invoked solely within
scripts/__init_global/__init_global.gml. The commented incantations in bothscripts/scr_load/scr_load.gmlandscripts/scr_initialize_custom/scr_initialize_custom.gmlare proper vestiges preserved for the foretold arrival of modding rites. This confirms that the compile-time initialization is executed correctly, forestalling any runtime anomalies while the TODO reminder dutifully notes the future need for re-enabling trait initialization when modding support is realized.scripts/scr_unit_traits/scr_unit_traits.gml (1)
1-2: Admirable usage of external trait data.By the Omnissiah’s grace, the code calls upon
json_to_gamemakerto bestow dynamic traits upon your Marines. Verify that thetraits.jsonpath is correct and present in all distributions, lest the Machine Spirits grow resentful.scripts/scr_json_functions/scr_json_functions.gml (5)
6-6: Commendable explicit declaration of file buffer.Defining
file_buffer = undefined;upholds clarity. The mortal technomat will not confuse the Omnissiah, and the code is poised to handle buffer creation consistently.
12-15: Excellent error handling via buffer load and exception.Ensuring a
-1check with a thrown exception protects the code from mishaps when the holy data script refuses to reveal itself. The Machine Spirits are appeased.
18-20: Beneficial debug messaging and direct parsing.The
_json_stringreading and logging fosters transparency, aiding in diagnosing anomalies during the trait rites of initialization. The direct_funccall is likewise efficient.
26-29: Proper resource cleanup in the final block.The
buffer_deleteinvocation infinallyis a sanctified best practice: no memory shrine is left unguarded. This is essential for the faithful stewardship of machine memory.
31-31: Neatly closed function scope.The final brace with the structured try-catch-finally block completes the functionality in an organized manner, ensuring the entire chain of code devotion remains pure.
Purpose of the PR
Describe the solution
Describe alternatives you've considered
Testing done
Compile, new game as ultramarines, scroll through the marine list to check traits.
Related links