refactor: efficiency of manage constants - #1436
Conversation
There was a problem hiding this comment.
2 issues found across 3 files
Confidence score: 5/5
- In
scripts/scr_ui_manage/scr_ui_manage.gml, replacing the fixed loop changes manage-screen tooltip ordering from armour-first to weapon-1/weapon-2/armour/gear/mobi, which may make the UI less consistent; preserve the intended ordering when assembling the tooltip. - In
scripts/scr_unit_equip_functions/scr_unit_equip_functions.gml,set_attribute_stringcan leave a trailing newline in thehptooltip when the last item is present; avoid appending the separator after the final item or trim the result.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/scr_unit_equip_functions/scr_unit_equip_functions.gml">
<violation number="1" location="scripts/scr_unit_equip_functions/scr_unit_equip_functions.gml:702">
P3: `set_attribute_string` appends `\n` after every non-empty item string, so when the last present item contributes, the returned string ends with a trailing newline. In the `hp` tooltip (`_hp_tool += _equip_data.set_attribute_string("hp_mod");` in scr_ui_manage) nothing is appended afterward, so the tooltip renders with a trailing blank line. Join with the separator between entries (and skip the round-trip through `_m_string`) instead of appending it after each one.</violation>
</file>
<file name="scripts/scr_ui_manage/scr_ui_manage.gml">
<violation number="1" location="scripts/scr_ui_manage/scr_ui_manage.gml:176">
P3: Replacing the fixed-loop with `set_attribute_string` silently changes the manage-screen tooltips: contributing items are now listed in weapon-1/weapon-2/armour/gear/mobi order instead of armour-first, and positive DR/armour values now render with a '+' prefix (e.g. "5%" becomes "+5%"). If the reorder or the added '+' is not intended, preserve the old slot order and plain formatting; otherwise call out the change explicitly.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| var _str = ""; | ||
| for (var i = 0; i < array_length(present_items); i++){ | ||
| var _item = equipment[$ present_items[i]]; | ||
| var _m_string = _item.item_attribute_string(attribute); |
There was a problem hiding this comment.
P3: set_attribute_string appends \n after every non-empty item string, so when the last present item contributes, the returned string ends with a trailing newline. In the hp tooltip (_hp_tool += _equip_data.set_attribute_string("hp_mod"); in scr_ui_manage) nothing is appended afterward, so the tooltip renders with a trailing blank line. Join with the separator between entries (and skip the round-trip through _m_string) instead of appending it after each one.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_unit_equip_functions/scr_unit_equip_functions.gml, line 702:
<comment>`set_attribute_string` appends `\n` after every non-empty item string, so when the last present item contributes, the returned string ends with a trailing newline. In the `hp` tooltip (`_hp_tool += _equip_data.set_attribute_string("hp_mod");` in scr_ui_manage) nothing is appended afterward, so the tooltip renders with a trailing blank line. Join with the separator between entries (and skip the round-trip through `_m_string`) instead of appending it after each one.</comment>
<file context>
@@ -695,6 +695,16 @@ function UnitEquipment(equipment_set, _unit = noone) constructor {
+ var _str = "";
+ for (var i = 0; i < array_length(present_items); i++){
+ var _item = equipment[$ present_items[i]];
+ var _m_string = _item.item_attribute_string(attribute);
+ _str += _m_string != "" ? _m_string + "\n" : "";
+ }
</file context>
| _res_tool += $"{name}: {dr}%\n"; | ||
| } | ||
| } | ||
| _res_tool += _equip_data.set_attribute_string("damage_resistance_mod"); |
There was a problem hiding this comment.
P3: Replacing the fixed-loop with set_attribute_string silently changes the manage-screen tooltips: contributing items are now listed in weapon-1/weapon-2/armour/gear/mobi order instead of armour-first, and positive DR/armour values now render with a '+' prefix (e.g. "5%" becomes "+5%"). If the reorder or the added '+' is not intended, preserve the old slot order and plain formatting; otherwise call out the change explicitly.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_ui_manage/scr_ui_manage.gml, line 176:
<comment>Replacing the fixed-loop with `set_attribute_string` silently changes the manage-screen tooltips: contributing items are now listed in weapon-1/weapon-2/armour/gear/mobi order instead of armour-first, and positive DR/armour values now render with a '+' prefix (e.g. "5%" becomes "+5%"). If the reorder or the added '+' is not intended, preserve the old slot order and plain formatting; otherwise call out the change explicitly.</comment>
<file context>
@@ -173,44 +173,7 @@ function reset_manage_unit_constants(unit) {
- _res_tool += $"{name}: {dr}%\n";
- }
- }
+ _res_tool += _equip_data.set_attribute_string("damage_resistance_mod");
_res_tool += $"CON: {round(unit.constitution / 2)}%";
</file context>
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Confidence score: 2/5
- In
scripts/scr_equipment_struct/scr_equipment_struct.gml,stat_display_label_conversioncalls undefinedlocalisewhen a tooltip contains any simple stat, causing tooltip generation to fail; change it to the existinglocalizefunction.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/scr_equipment_struct/scr_equipment_struct.gml">
<violation number="1" location="scripts/scr_equipment_struct/scr_equipment_struct.gml:222">
P1: When a tooltip contains any simple stat, `stat_display_label_conversion` calls the undefined `localise` function and tooltip generation fails. Call the existing `localize` function.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| _label = "Maintenance"; | ||
| break; | ||
| } | ||
| return localise(_label); |
There was a problem hiding this comment.
P1: When a tooltip contains any simple stat, stat_display_label_conversion calls the undefined localise function and tooltip generation fails. Call the existing localize function.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_equipment_struct/scr_equipment_struct.gml, line 222:
<comment>When a tooltip contains any simple stat, `stat_display_label_conversion` calls the undefined `localise` function and tooltip generation fails. Call the existing `localize` function.</comment>
<file context>
@@ -129,28 +165,86 @@ function EquipmentStruct(item_data = undefined, core_type = "", quality_request
+ _label = "Maintenance";
+ break;
+ }
+ return localise(_label);
+ };
+
</file context>
| return localise(_label); | |
| return localize(_label); |
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
…optomise_manage_constants
There was a problem hiding this comment.
2 issues found across 1 file (changes from recent commits).
Confidence score: 3/5
datafiles/lang/zh.jsonremoves the#Requires {0} EXP#translation whilescr_equipment_struct.gml:381still requests it, so Chinese users may see an untranslated requirements line—restore the key or update the caller and verify localization output.datafiles/lang/zh.jsonchanges “Quality” from “品质” to “品”, which loses the intended meaning unless a UI-width constraint requires it—restore “品质” or document and validate the space-saving requirement.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="datafiles/lang/zh.json">
<violation number="1" location="datafiles/lang/zh.json:1667">
P3: The "Quality" value regressed from "品质" to "品", which alone reads as "item/goods" and loses the intended meaning "quality". Unless there is an explicit UI-width constraint for this label, revert to "品质".</violation>
<violation number="2" location="datafiles/lang/zh.json:1682">
P2: Removing the `"#Requires {0} EXP#"` translation breaks Chinese for the requirements line: `scripts/scr_equipment_struct/scr_equipment_struct.gml:381` still calls `localize("#Requires {0} EXP#", [req_exp])` with the old composite key, which is no longer present in zh.json, so zh users now see the English fallback "#Requires {0} EXP#" instead of "#需要 {0} 经验#". The new "Requires"/"EXP" split keys don't help that caller. Keep the composite key (it remains the English source key in en.json too) until that call site is migrated, or the requirements text for Chinese players will show English.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| "Ranged Burden: {0}#": "远程负担:{0}#", | ||
| "Ranged Burden Cap: {0}#": "远程负担上限:{0}#", | ||
| "#Properties:#{0}#": "#特性:#{0}#", | ||
| "#Requires {0} EXP#": "#需要 {0} 经验#", |
There was a problem hiding this comment.
P2: Removing the "#Requires {0} EXP#" translation breaks Chinese for the requirements line: scripts/scr_equipment_struct/scr_equipment_struct.gml:381 still calls localize("#Requires {0} EXP#", [req_exp]) with the old composite key, which is no longer present in zh.json, so zh users now see the English fallback "#Requires {0} EXP#" instead of "#需要 {0} 经验#". The new "Requires"/"EXP" split keys don't help that caller. Keep the composite key (it remains the English source key in en.json too) until that call site is migrated, or the requirements text for Chinese players will show English.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At datafiles/lang/zh.json, line 1682:
<comment>Removing the `"#Requires {0} EXP#"` translation breaks Chinese for the requirements line: `scripts/scr_equipment_struct/scr_equipment_struct.gml:381` still calls `localize("#Requires {0} EXP#", [req_exp])` with the old composite key, which is no longer present in zh.json, so zh users now see the English fallback "#Requires {0} EXP#" instead of "#需要 {0} 经验#". The new "Requires"/"EXP" split keys don't help that caller. Keep the composite key (it remains the English source key in en.json too) until that call site is migrated, or the requirements text for Chinese players will show English.</comment>
<file context>
@@ -1664,24 +1664,25 @@
+ "Ranged Burden": "远程负担",
+ "Ranged Burden Cap": "远程负担上限",
+ "Properties": "特性",
+ "Keywords": "关键词",
+ "Maintenance": "维护",
+ "Requires": "#需要",
</file context>
| "#Requires {0} EXP#": "#需要 {0} 经验#", | ||
| "#Keywords:#{0}#": "#关键词:#{0}#", | ||
| "Maintenance: {0}#": "维护:{0}#", | ||
| "Quality": "品", |
There was a problem hiding this comment.
P3: The "Quality" value regressed from "品质" to "品", which alone reads as "item/goods" and loses the intended meaning "quality". Unless there is an explicit UI-width constraint for this label, revert to "品质".
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At datafiles/lang/zh.json, line 1667:
<comment>The "Quality" value regressed from "品质" to "品", which alone reads as "item/goods" and loses the intended meaning "quality". Unless there is an explicit UI-width constraint for this label, revert to "品质".</comment>
<file context>
@@ -1664,24 +1664,25 @@
- "#Requires {0} EXP#": "#需要 {0} 经验#",
- "#Keywords:#{0}#": "#关键词:#{0}#",
- "Maintenance: {0}#": "维护:{0}#",
+ "Quality": "品",
+ "Armour": "护甲",
+ "Health Mod": "生命修正",
</file context>
| "Quality": "品", | |
| "Quality": "品质", |
There was a problem hiding this comment.
1 existing issue remains and no new issues found across 2 files (changes from recent commits).
Confidence score: 3/5
datafiles/lang/zh.jsonremoves the#Requires {0} EXP#key whilescr_equipment_struct.gml:381still requests it, causing the Chinese required-experience tooltip to fall back to untranslated English; restore the key or update the localization call.
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Requires human review: Auto-approval blocked by 5 unresolved issues from previous reviews.
Re-trigger cubic
Summary by cubic
Unifies equipment stat display and Manage constants to speed up UI updates and keep tooltips consistent. Old behavior built DR/HP/Armor contributor lines with per-slot loops and localized templates; new behavior aggregates them via
_equip_data.set_attribute_string(attr)backed byEquipmentStruct.item_attribute_string(attr)and unified label/value conversion. No stat calculations changed.stat_display_has_value,stat_display_label_conversion, andstat_display_value_conversion; zeros are hidden; percent/sign formatting is consistent; quality uses the “##” terminator; “Maintenance” now renders as a simple stat. “Properties” and “Keywords” headings use the same label conversion; “Requires EXP” is assembled from separate localized tokens inzh.json(label-only keys).set_attribute_string("damage_resistance_mod" | "hp_mod" | "armour_value"). DR intro and its CON line are literal English; HP/Armor intros remain localized. Contributor lines use raw item names.present_itemscovers and orders contributors fordamage_resistance_mod,hp_mod, andarmour_value; verify no blank/duplicate lines. Confirm tooltip type inference now uses the item’stypefield directly without regressions.Written for commit 89766ff. Summary will update on new commits.