Skip to content

feat: zh-cn-localization - #1431

Open
CaesinEachonson wants to merge 21 commits into
Adeptus-Dominus:mainfrom
CaesinEachonson:feat/zh-cn-localization
Open

feat: zh-cn-localization#1431
CaesinEachonson wants to merge 21 commits into
Adeptus-Dominus:mainfrom
CaesinEachonson:feat/zh-cn-localization

Conversation

@CaesinEachonson

@CaesinEachonson CaesinEachonson commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Summary by cubic

Adds the first batch of Chinese localization for chapter creation and related UI, plus a language toggle in Settings. Extracts strings into en.json/zh.json, standardizes placeholders, applies CJK-safe fonts across UI (draw and measurement), and refreshes localized globals on language switch.

  • New Features

    • Localized chapter selection, livery/role setup, role distribution toggles, homeworld/flagship flows, event texts, tooltips, and error/popups via localize().
    • Centralized button labels with localize_button_text() and structured placeholders (LANG_ENTRY_TEXT/LANG_ENTRY_VARIABLES); UI text renderers, tooltips, and shutter buttons use cjk_font(); faction_names and stat rating labels rebuild from English sources via refresh_locale_globals().
  • Bug Fixes

    • Suppressed repeat missing-key warnings; safer placeholder replacement; normalized newline escapes across tooltips/inputs; ensured tooltip measurement and UI text rendering use CJK fonts.
    • Fixed crashes on language switch and when opening squad view (correct default unit focus); localized chapter load errors/popups.

Written for commit 7afe752. Summary will update on new commits.

Review in cubic

@github-actions github-actions Bot added Area: JSON Changes to external JSON files or their under-the-hood functionality Size: Epic labels Aug 11, 2026
@CaesinEachonson CaesinEachonson changed the title First batch of Chinese localization for testing feat: zh-cn-localization Aug 11, 2026
@github-actions github-actions Bot added the Type: Feature Adds something new label Aug 11, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 issues found across 14 files

Confidence score: 3/5

  • In scripts/scr_buttons/scr_buttons.gml, the new CJK font wrapper is only applied at some draw sites, and tooltip_draw in scr_tooltip_draw.gml still uses direct font draws; localized strings can render with missing glyphs or fallback artifacts in affected UI paths — wrap all localized draw entry points (especially tooltip rendering) with the same CJK-safe font path.
  • In scripts/scr_chapter_managent_events/scr_chapter_managent_events.gml, already-localized values are being passed into another localize() format call, which can produce awkward or incorrect output across languages and make translation keys brittle — pass raw keys/data into the formatter and localize once at the final render step.
  • In scripts/scr_role_setup/scr_role_setup.gml, the On Planet tooltip currently shows the newline token literally, so players may see escaped markers instead of line breaks — replace the marker with a real newline and update the corresponding translation key/value to match.
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_role_setup/scr_role_setup.gml">

<violation number="1" location="scripts/scr_role_setup/scr_role_setup.gml:106">
P3: The On Planet tooltip renders its newline marker as text; use a real newline in the source and normalize the matching translation key/value.</violation>
</file>

<file name="scripts/scr_buttons/scr_buttons.gml">

<violation number="1" location="scripts/scr_buttons/scr_buttons.gml:69">
P2: The CJK font wrapper introduced in this batch is applied only to a subset of draw sites. Places that draw localized text but are not wrapped—notably `tooltip_draw` (scr_tooltip_draw.gml uses fnt_40k_14/fnt_40k_14b directly) and any other unconverted draw_set_font call—will keep the original 40k/cul bitmap fonts, which have no CJK glyphs. For a Chinese-language user this produces inconsistent UI: buttons/labels render in the simhei fallback while tooltips and other unwrapped text show blank boxes or fall back to English. Before merging this first localization batch, it's worth auditing every draw path that can display localized strings and routing them through `cjk_font(...)`, so switching languages doesn't yield a half-translated, partially unreadable interface.</violation>
</file>

<file name="scripts/scr_chapter_managent_events/scr_chapter_managent_events.gml">

<violation number="1" location="scripts/scr_chapter_managent_events/scr_chapter_managent_events.gml:247">
P2: This passes already-localized values (`localize(pick.name())`, `localize(pick.role())`, and `likability` which was set from `localize(...)`) as positional placeholders into another `localize()` format key. Same latent footgun as elsewhere: `translate()` does unescaped, sequential `string_replace_all` over placeholders, so if any of these runtime values (names/roles from data) happens to contain a `{0}`/`{1}`/`{2}` token, it would be re-interpreted against another placeholder and the message would be corrupted. It also means the format key and each embedded fragment are independently translated, making coherent sentence construction harder for translators. Consider localizing a single whole-sentence key (with data placeholders only) instead of nesting pre-localized fragments inside another format string.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread scripts/scr_role_setup/scr_role_setup.gml Outdated
add_draw_return_values();

draw_set_font(font);
draw_set_font(cjk_font(font));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The CJK font wrapper introduced in this batch is applied only to a subset of draw sites. Places that draw localized text but are not wrapped—notably tooltip_draw (scr_tooltip_draw.gml uses fnt_40k_14/fnt_40k_14b directly) and any other unconverted draw_set_font call—will keep the original 40k/cul bitmap fonts, which have no CJK glyphs. For a Chinese-language user this produces inconsistent UI: buttons/labels render in the simhei fallback while tooltips and other unwrapped text show blank boxes or fall back to English. Before merging this first localization batch, it's worth auditing every draw path that can display localized strings and routing them through cjk_font(...), so switching languages doesn't yield a half-translated, partially unreadable interface.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_buttons/scr_buttons.gml, line 69:

<comment>The CJK font wrapper introduced in this batch is applied only to a subset of draw sites. Places that draw localized text but are not wrapped—notably `tooltip_draw` (scr_tooltip_draw.gml uses fnt_40k_14/fnt_40k_14b directly) and any other unconverted draw_set_font call—will keep the original 40k/cul bitmap fonts, which have no CJK glyphs. For a Chinese-language user this produces inconsistent UI: buttons/labels render in the simhei fallback while tooltips and other unwrapped text show blank boxes or fall back to English. Before merging this first localization batch, it's worth auditing every draw path that can display localized strings and routing them through `cjk_font(...)`, so switching languages doesn't yield a half-translated, partially unreadable interface.</comment>

<file context>
@@ -66,7 +66,7 @@ function draw_unit_buttons(position, text, size_mod = [1.5, 1.5], colour = c_gra
     add_draw_return_values();
 
-    draw_set_font(font);
+    draw_set_font(cjk_font(font));
     draw_set_halign(_halign);
     draw_set_valign(fa_middle);
</file context>

likability = localize("He is like by all of his tech brothers");
}
text = $"{pick.name()} is selected as the new {pick.role()} {likability}.";
text = localize("{0} is selected as the new {1} {2}.", [localize(pick.name()), localize(pick.role()), likability]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This passes already-localized values (localize(pick.name()), localize(pick.role()), and likability which was set from localize(...)) as positional placeholders into another localize() format key. Same latent footgun as elsewhere: translate() does unescaped, sequential string_replace_all over placeholders, so if any of these runtime values (names/roles from data) happens to contain a {0}/{1}/{2} token, it would be re-interpreted against another placeholder and the message would be corrupted. It also means the format key and each embedded fragment are independently translated, making coherent sentence construction harder for translators. Consider localizing a single whole-sentence key (with data placeholders only) instead of nesting pre-localized fragments inside another format string.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_chapter_managent_events/scr_chapter_managent_events.gml, line 247:

<comment>This passes already-localized values (`localize(pick.name())`, `localize(pick.role())`, and `likability` which was set from `localize(...)`) as positional placeholders into another `localize()` format key. Same latent footgun as elsewhere: `translate()` does unescaped, sequential `string_replace_all` over placeholders, so if any of these runtime values (names/roles from data) happens to contain a `{0}`/`{1}`/`{2}` token, it would be re-interpreted against another placeholder and the message would be corrupted. It also means the format key and each embedded fragment are independently translated, making coherent sentence construction harder for translators. Consider localizing a single whole-sentence key (with data placeholders only) instead of nesting pre-localized fragments inside another format string.</comment>

<file context>
@@ -233,42 +233,42 @@ function new_forge_master_chosen(pick) {
+            likability = localize("He is like by all of his tech brothers");
         }
-        text = $"{pick.name()} is selected as the new {pick.role()} {likability}.";
+        text = localize("{0} is selected as the new {1} {2}.", [localize(pick.name()), localize(pick.role()), likability]);
         if (skill_lack > 0 && skill_lack < 6) {
-            text += "There are some questions about his ability.";
</file context>

Comment thread scripts/scr_creation_draw_slides/scr_creation_draw_slides.gml Outdated
Comment thread scripts/scr_creation/scr_creation.gml
font: fnt_40k_12,
style: "box",
tooltip: $"On Planet/nCheck to have your Astartes Start on your home planet.",
tooltip: localize("On Planet/nCheck to have your Astartes Start on your home planet."),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The On Planet tooltip renders its newline marker as text; use a real newline in the source and normalize the matching translation key/value.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_role_setup/scr_role_setup.gml, line 106:

<comment>The On Planet tooltip renders its newline marker as text; use a real newline in the source and normalize the matching translation key/value.</comment>

<file context>
@@ -78,44 +78,44 @@ function update_role_data_wth_defaults() {
             font: fnt_40k_12,
             style: "box",
-            tooltip: $"On Planet/nCheck to have your Astartes Start on your home planet.",
+            tooltip: localize("On Planet/nCheck to have your Astartes Start on your home planet."),
         },
         {
</file context>

Comment thread datafiles/lang/zh.json Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 10 files (changes from recent commits).

Confidence score: 4/5

  • In objects/obj_creation_popup/Draw_0.gml, routing both the bold title (fnt_40k_14b) and regular body (fnt_40k_14) through cjk_font() can collapse text styling in CJK mode because LocalizationManager.get_font() appears to cache fallback fonts too coarsely, which risks a visible readability/UX regression in the tooltip popup—split the cache key (or lookup path) to include style/weight so bold and regular resolve distinctly.
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="objects/obj_creation_popup/Draw_0.gml">

<violation number="1" location="objects/obj_creation_popup/Draw_0.gml:30">
P2: In Chinese (CJK) mode the tooltip/role-name popup now goes through cjk_font() for both the bold title (fnt_40k_14b) and the regular body (fnt_40k_14). LocalizationManager.get_font() caches its fallback font by point size only (`string(_size)`), and both of these fonts are size 14, so they resolve to the same cached CJK font and the bold/regular visual distinction disappears (and fnt_40k_14i would collapse into the same entry too). This makes the bold title text render with body weight in the localized UI. Consider including the style/typeface in the cache key (or explicitly documenting that CJK fallback can't preserve weight), so same-size fonts with different styles don't silently share one glyph set.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread objects/obj_tooltip/Draw_75.gml Outdated
draw_set_color(CM_GREEN_COLOR);
draw_rectangle(mouse_x + 18, mouse_y + 20, mouse_x + string_width_ext(string_hash_to_newline(tooltip2), -1, 500) + 24, mouse_y + 44 + string_height_ext(string_hash_to_newline(tooltip2), -1, 500), 1);
draw_set_font(fnt_40k_14b);
draw_set_font(cjk_font(fnt_40k_14b));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: In Chinese (CJK) mode the tooltip/role-name popup now goes through cjk_font() for both the bold title (fnt_40k_14b) and the regular body (fnt_40k_14). LocalizationManager.get_font() caches its fallback font by point size only (string(_size)), and both of these fonts are size 14, so they resolve to the same cached CJK font and the bold/regular visual distinction disappears (and fnt_40k_14i would collapse into the same entry too). This makes the bold title text render with body weight in the localized UI. Consider including the style/typeface in the cache key (or explicitly documenting that CJK fallback can't preserve weight), so same-size fonts with different styles don't silently share one glyph set.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At objects/obj_creation_popup/Draw_0.gml, line 30:

<comment>In Chinese (CJK) mode the tooltip/role-name popup now goes through cjk_font() for both the bold title (fnt_40k_14b) and the regular body (fnt_40k_14). LocalizationManager.get_font() caches its fallback font by point size only (`string(_size)`), and both of these fonts are size 14, so they resolve to the same cached CJK font and the bold/regular visual distinction disappears (and fnt_40k_14i would collapse into the same entry too). This makes the bold title text render with body weight in the localized UI. Consider including the style/typeface in the cache key (or explicitly documenting that CJK fallback can't preserve weight), so same-size fonts with different styles don't silently share one glyph set.</comment>

<file context>
@@ -21,15 +21,15 @@ try {
         draw_set_color(CM_GREEN_COLOR);
         draw_rectangle(mouse_x + 18, mouse_y + 20, mouse_x + string_width_ext(string_hash_to_newline(tooltip2), -1, 500) + 24, mouse_y + 44 + string_height_ext(string_hash_to_newline(tooltip2), -1, 500), 1);
-        draw_set_font(fnt_40k_14b);
+        draw_set_font(cjk_font(fnt_40k_14b));
         draw_text(mouse_x + 22, mouse_y + 22, string_hash_to_newline(string(tooltip)));
-        draw_set_font(fnt_40k_14);
</file context>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 2 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread scripts/LocalizationManager/LocalizationManager.gml

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread scripts/LocalizationManager/LocalizationManager.gml Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment on lines +80 to +81
specialist_distribution_box = new ToggleButton({
str1: "Equal Specialist Distribution",
str1: localize("Equal Specialist Distribution"),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as the toggle buttons are already a custom constructor we should not need to localise at call only on the update method of the ToggleButton this will save lots of extra ablative wrapping of text in localise calls

Comment thread objects/obj_creation/Draw_0.gml Outdated
Comment on lines +379 to +382
draw_text(650, 550, string_hash_to_newline(localize("Imperium ({0})", [string(disposition[2])])));
draw_text(650, 575, string_hash_to_newline(localize("Adeptus Mechanicus ({0})", [string(disposition[3])])));
draw_text(650, 600, string_hash_to_newline(localize("Ecclesiarchy ({0})", [string(disposition[5])])));
draw_text(650, 625, string_hash_to_newline(localize("Inquisition ({0})", [string(disposition[4])])));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thees values (Imperium, Adeptus Mechanicus etc) sshould always be read from the global.faction_names and accessed using the enum eFACTION so global.faction_names[eFACTION.MECHANICUS] as result the faction_names global should get reset to the correct language values on language changes thus removing the need for localise calls where the translation is purely for the sake Faction names and other important variables

x1: 710,
y1: 250,
tooltip: $"Scout Distribution\nCheck if you wish for Scouts to be distributed equally across your Battle Companies rather than concentrated in the 10th.",
tooltip: localize("Scout Distribution\nCheck if you wish for Scouts to be distributed equally across your Battle Companies rather than concentrated in the 10th."),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same for tooltips, lets assume that all tooltips get localised as they're passed into the ToggleButton struct this saves going round the code base adding in a million wrapper calls

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as a result this means that the update method i the ToggleButton constructor will need to account for the fact that localise varibles may be passed to the constructor, this could be done by allowing a seperate tooltip_variables key to be passed or by allowing tooltip to be a struct or an array that is sscanned for once it is being read in the update method of the constructor. Not sure what you think is best in this instance?

either way a method that does not enforce changes to be made to ever single constrictor call as currently happens would be more ideal imo

Comment on lines 473 to 498
var _strength_ratings = [
"",
"Decimated",
"Reduced",
"Reduced",
"Reduced",
"Average",
"Above Average",
"Above Average",
"Considerable",
"Considerable",
"Overwhelming",
localize("Decimated"),
localize("Reduced"),
localize("Reduced"),
localize("Reduced"),
localize("Average"),
localize("Above Average"),
localize("Above Average"),
localize("Considerable"),
localize("Considerable"),
localize("Overwhelming"),
];
var _cooperation_ratings = [
"",
"Antagonistic",
"Uncooperative",
"Uncooperative",
"Uncooperative",
"Neutral",
"Trusted",
"Trusted",
"Trusted",
"Trusted",
"Exemplary",
localize("Antagonistic"),
localize("Uncooperative"),
localize("Uncooperative"),
localize("Uncooperative"),
localize("Neutral"),
localize("Trusted"),
localize("Trusted"),
localize("Trusted"),
localize("Trusted"),
localize("Exemplary"),
];

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we introduce a localise array funtionality (see my other comment about global.faction_names) this way instead of trying to localise each item one could just call localise_array(cooperation_ratings) and search for the array in the language pack, no need i think in this instance to make a default fallback either just assume english is the default fallback and search that file and if the value is not there bung and error message using LOGGER.error()


draw_set_color(CM_GREEN_COLOR);
draw_text_transformed(800, 120, string_hash_to_newline("Points: " + string(points) + "/" + string(maxpoints)), 0.6, 0.6, 0);
draw_text_transformed(800, 120, string_hash_to_newline(localize("Points: {0}/{1}", [string(points), string(maxpoints)])), 0.6, 0.6, 0);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
draw_text_transformed(800, 120, string_hash_to_newline(localize("Points: {0}/{1}", [string(points), string(maxpoints)])), 0.6, 0.6, 0);
draw_text_transformed(800, 120, string_hash_to_newline(localize("Points: {0}/{1}", [
points,maxpoints])), 0.6, 0.6, 0);

where posssible with interpolation remove the legacy string() uses they are legacy and automatically handled by iterpolation

@OH296

OH296 commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Nice work extending the translation, I've made a few review points, bear in mind my word is not god if you think for some reason any of the points ii've made are advantages compared to choices you've made feel free to point that out and we can make an informed choice

static default_member = function() {
var _member = company_squads[0].fetch_member(0);
if (is_struct(_member)) {
obj_controller.unit_focus = _fetched;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you cherry pick this commit into a separate pr so it can be fast tracked

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not very safe i think. this one is to fix a crash on this PR.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't worry i have handled it

@@ -202,7 +202,7 @@ function ReactiveString(text_param, x1_param = 0, y1_param = 0, data = {}) const
text = text_param;
font = fnt_40k_14;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can the cjk_font not simply be run here ad it thee update call so if the font is updated the correct new font is assigned using the cjk_font call this would reduce 90% of the cjk_font calls

static update = function(data = {}) {
move_data_to_current_scope(data);
var temp_font = draw_get_font();
draw_set_font(font);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one way of doing the above point would be to create a sspecialised version of move_data_to_current_scope for mmoving data into constructorss that on reading the "font" key always runs self.font = cjk_font(font);

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 6 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread scripts/LocalizationManager/LocalizationManager.gml Outdated
Comment thread scripts/scr_buttons/scr_buttons.gml Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 5 files (changes from recent commits).

Confidence score: 3/5

  • In scripts/scr_buttons/scr_buttons.gml, ToggleButton can keep rendering with a stale cached font after font changes via constructor data or update(), so users may see mismatched text appearance while layout/sizing follows the new font; refresh the cached font_cjk whenever font is updated to keep rendering and sizing in sync.
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_buttons/scr_buttons.gml">

<violation number="1" location="scripts/scr_buttons/scr_buttons.gml:223">
P2: Changing a `ToggleButton`'s `font` through its constructor data or a later `update()` leaves the rendered text using the old cached font, even though sizing uses the new font. Refresh `font_cjk` in `update()` or resolve `cjk_font(font)` when drawing.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread scripts/scr_creation_draw_slides/scr_creation_draw_slides.gml Outdated
Comment thread scripts/LocalizationManager/LocalizationManager.gml Outdated
Comment thread scripts/scr_buttons/scr_buttons.gml
font_cjk = cjk_font(font);
add_draw_return_values();
draw_set_font(font);
draw_set_font(font_cjk);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Changing a ToggleButton's font through its constructor data or a later update() leaves the rendered text using the old cached font, even though sizing uses the new font. Refresh font_cjk in update() or resolve cjk_font(font) when drawing.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_buttons/scr_buttons.gml, line 223:

<comment>Changing a `ToggleButton`'s `font` through its constructor data or a later `update()` leaves the rendered text using the old cached font, even though sizing uses the new font. Refresh `font_cjk` in `update()` or resolve `cjk_font(font)` when drawing.</comment>

<file context>
@@ -201,8 +218,9 @@ function ReactiveString(text_param, x1_param = 0, y1_param = 0, data = {}) const
+    font_cjk = cjk_font(font);
     add_draw_return_values();
-    draw_set_font(cjk_font(font));
+    draw_set_font(font_cjk);
     w = string_width(text);
     h = string_height(text);
</file context>
Suggested change
draw_set_font(font_cjk);
draw_set_font(cjk_font(font));

Comment thread scripts/LocalizationManager/LocalizationManager.gml Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 6 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread scripts/scr_buttons/scr_buttons.gml Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread scripts/scr_buttons/scr_buttons.gml Outdated
Comment on lines +386 to +398
/// @desc Measures the localized text's display width (plus padding) using the active CJK
/// font and stores it in text_width. Shared by the constructor and update() so the
/// font-swap measurement boilerplate lives in one place. text_ must be localized and
/// font_cjk current before calling.
/// @returns {undefined}
static measure_text_width = function() {
var _prev_font = draw_get_font();
draw_set_font(font_cjk);
text_width = string_width(text) + 2;
draw_set_font(_prev_font);
};

measure_text_width();

@OH296 OH296 Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as update is run on the initialisation of all constructors and should be sole way data is updated within constructors so this iss not really needed just slam the logic straight into the update method it never needed to be callled separately in the first place

move_data_to_current_scope(data);
str1 = localize_button_text(str1);
tooltip = localize_button_text(tooltip);
font_cjk = cjk_font(font);

@OH296 OH296 Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if font_cjk is just meant to be the defacto font there is no need to have two separate variables just use font = cjk_font(font); as you've done with the str1 variable and be done with it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot change it to font=cjk_font(font). It'll make the game unable to switch back to English once I changed language...


if (struct_exists(_mod, "int_mod") && _mod.int_mod != 0) {
_line += $" Disposition Gains : {string_plus_minus(_mod.int_mod)}{_mod.int_mod}\n";
_line += localize(" Disposition Gains : {0}\n", [string_plus_minus(_mod.int_mod) + string(_mod.int_mod)]);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as per before unless calcs are being done always favour strong interpolation over string() methods

Suggested change
_line += localize(" Disposition Gains : {0}\n", [string_plus_minus(_mod.int_mod) + string(_mod.int_mod)]);
_line += localize(" Disposition Gains : {0}\n", [$"{string_plus_minus(_mod.int_mod)}{_mod.int_mod}]);


static update = function(data = {}) {
move_data_to_current_scope(data);
font_cjk = cjk_font(font);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agains this is overly verbose font = cjk_font(font); should be fine but if as per the first thought we kake move_data_to_current_scope_ui(data) and check for if "font" is one of the keys in the update packet then we can centralise all font = cjk_font(font); into that newly created function and apply it to all constructors same goes for sstr1 and tooltip localisation tests

@OH296

OH296 commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Okay very close now on this one just a few more tweaks and we should be good to go

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 2 files (changes from recent commits).

Confidence score: 4/5

  • In scripts/scr_buttons/scr_buttons.gml, moving font-measurement logic inline inside LabeledIcon.update() introduces duplicated save/set/measure/restore steps, which raises the chance of future inconsistencies or missed font restoration when this pattern is edited again; this could cause subtle text sizing/layout regressions in button labels—restore a shared helper (or local utility) for font measurement to keep the behavior centralized.
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_buttons/scr_buttons.gml">

<violation number="1" location="scripts/scr_buttons/scr_buttons.gml:393">
P2: Custom agent: **Code Quality Review**

This PR removes the `measure_text_width()` abstraction and replaces it with inline font-swap boilerplate inside `LabeledIcon.update()`. The same save-font / set-font / measure / restore-font pattern already appears in `ReactiveString.update()`, `UnitButtonObject.update_loc()`, and at least one other function in this file, resulting in four near-identical blocks with no shared helper. Consider keeping (or promoting) a single utility for this font-swapped measurement so the logic lives in one place and future changes don't need to be copied to every UI element.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread scripts/scr_buttons/scr_buttons.gml Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: JSON Changes to external JSON files or their under-the-hood functionality Size: Epic Type: Feature Adds something new

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants