[script][combat-trainer] Pass summoned_weapons_adjective as settings argument to shape_summoned_weapon calls#7266
Conversation
…amily_vault_issues [scripts][inventory-manager] Fix some minor family vault issues
…hape_summoned_weapon
…r-pass-custom-weapon-summon-adj
…r-pass-custom-weapon-summon-adj
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to f7b6d32 in 2 minutes and 35 seconds. Click for details.
- Reviewed
26lines of code in1files - Skipped
0files when reviewing. - Skipped posting
5draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. combat-trainer.lic:5610
- Draft comment:
In prepare_summoned_weapon the code accesses info['turn'], info['push'], and info['pull'] without checking if info is nil. This may lead to a nil-dereference error if summoned_info returns nil. - Reason this comment was not posted:
Comment was on unchanged code.
2. combat-trainer.lic:1928
- Draft comment:
There is use of eval (e.g. eval(@training_spells[skill]['must_be_true'])). Using eval on configuration strings can be a security risk and hard to maintain. Consider refactoring to use a safer alternative. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. combat-trainer.lic:20
- Draft comment:
The file is extremely large and monolithic with many responsibilities (e.g. multiple Process classes for Setup, Spell, Pet, Ability, Attack, etc.). Consider refactoring the code into smaller, focused modules or separate files to improve maintainability and testability. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. combat-trainer.lic:1
- Draft comment:
There is heavy reliance on global variables (e.g. $debug_mode_ct, DRStats, DRSpells) which make the code state-dependent and harder to test. Consider reducing global state via dependency injection or encapsulation. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. combat-trainer.lic:100
- Draft comment:
Multiple DRC.bput calls contain inline regular expressions for matching output messages. This use of hard‐coded regex patterns can hurt maintainability. Consider defining these regex patterns as named constants, improving readability and easier future updates. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_cEvXIJauyj5kTEMU
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
|
|
||
| def prepare_summoned_weapon(weapon_already_summoned) | ||
| info = summoned_info(weapon_skill) | ||
| adj = JSON.parse("{\"summoned_weapons_adjective\": \"#{@summoned_weapons_adjective}\"}", object_class: OpenStruct) |
There was a problem hiding this comment.
The JSON.parse call to create an adj object seems unnecessarily heavy for constructing a simple data structure. Consider using a plain Ruby hash (e.g. { summoned_weapons_adjective: @summoned_weapons_adjective }) instead.
There was a problem hiding this comment.
That's how I originally had it, though, and kept getting an error about trying to call summoned_weapons_adjective as a function.
There was a problem hiding this comment.
It's wanting you to do:
adj = OpenStruct.new({ summoned_weapons_adjective: @summoned_weapons_adjective })Or if you don't need it to be an OpenStruct, change the original:
DRCS.shape_summoned_weapon(weapon_skill, @summoned_weapons_ingot, { summoned_weapons_adjective: @summoned_weapons_adjective }) if weapon_already_summoned || DRStats.moon_mage?
Top level profile yaml setting
summoned_weapons_adjectivefor shaping summoned weapons with custom adjectives needs to be passed in on the shape_summoned_weapon call, as it is expected by the DRCS function.Important
Pass
summoned_weapons_adjectiveas a settings argument toshape_summoned_weaponinGameStateto meetDRCSfunction expectations.summoned_weapons_adjectiveas a settings argument toshape_summoned_weaponinprepare_summoned_weapon()inGameState.@summoned_weapons_adjectivefrom settings and log it if$debug_mode_ctis enabled.This description was created by
for f7b6d32. You can customize this summary. It will automatically update as commits are pushed.