chore: add source skyui/util scripts for editing/import #205
Open
zndxcvbn wants to merge 3 commits into
Open
Conversation
zndxcvbn
changed the title
skyui/util scripts for editing/import
Contributor
📝 WalkthroughWalkthroughThis PR introduces a new suite of seven ActionScript utility classes that extend the SkyUI framework with reusable helpers for color conversion, configuration management, dialog handling, effect icon resolution, string/array/math operations, text translation, and animation support. All utilities are implemented as static or singleton-style APIs in separate modules. ChangesSkyUI Utility Library Expansion
Sequence Diagram(s)sequenceDiagram
participant External
participant ConfigManager
participant LoadVars
participant Parser
participant Events
External->>ConfigManager: setExternalOverrideKeys/Values (Papyrus phase)
ConfigManager->>LoadVars: Load skyui/config.txt
LoadVars->>Parser: onData callback with file contents
Parser->>ConfigManager: parseData (parse INI, resolve constants, track variable refs)
ConfigManager->>Events: Dispatch configLoad event
External->>ConfigManager: setOverride (programmatic mutation)
ConfigManager->>ConfigManager: Update _config, propagate to tracked var refs
ConfigManager->>Events: Dispatch configUpdate event
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (4)
source/actionscript/Common/skyui/util/ColorFunctions.as (1)
203-207: 💤 Low valueAdd missing parameter type annotations for consistency.
These functions lack the
: Arraytype annotation on their parameters, unlike all other Array-accepting functions in this class.Suggested fix
- public static function hsvToHex(a_HSV) { return skyui.util.ColorFunctions.rgbToHex(skyui.util.ColorFunctions.hsvToRgb(a_HSV)); } + public static function hsvToHex(a_HSV: Array) { return skyui.util.ColorFunctions.rgbToHex(skyui.util.ColorFunctions.hsvToRgb(a_HSV)); } // HSB (alias for HSV) public static function hsbToRgb(a_HSB: Array) { return skyui.util.ColorFunctions.hsvToRgb(a_HSB); } - public static function hsbToHex(a_HSB) { return skyui.util.ColorFunctions.hsvToHex(a_HSB); } + public static function hsbToHex(a_HSB: Array) { return skyui.util.ColorFunctions.hsvToHex(a_HSB); }- public static function hslToHex(a_HSL) { return skyui.util.ColorFunctions.rgbToHex(skyui.util.ColorFunctions.hslToRgb(a_HSL)); } + public static function hslToHex(a_HSL: Array) { return skyui.util.ColorFunctions.rgbToHex(skyui.util.ColorFunctions.hslToRgb(a_HSL)); }Also applies to: 282-282
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/actionscript/Common/skyui/util/ColorFunctions.as` around lines 203 - 207, The HSV/HSB wrapper functions hsvToHex, hsbToRgb, and hsbToHex are missing explicit parameter type annotations; update their signatures to mark their input as an Array (e.g., change a_HSV and a_HSB parameters to : Array) to match the rest of the class and the other Array-accepting functions (also apply the same fix to the other occurrence referenced at the end of the file). Keep the function bodies unchanged—only add the : Array type annotation to each parameter in hsvToHex, hsbToRgb, and hsbToHex (and the duplicate at the noted location).source/actionscript/Common/skyui/util/EffectIconMap.as (1)
186-186: 💤 Low valueComment inconsistency: null value marked as "OK".
This entry has a null value but is marked "OK" in the comment, which is inconsistent with the pattern used elsewhere. Other null entries are typically marked "nn". Consider updating the comment to match the intended status.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/actionscript/Common/skyui/util/EffectIconMap.as` at line 186, Update the comment for the null entry corresponding to AV_LEFTWEAPONSPEEDMULT in EffectIconMap (the line with "null, // 132 - AV_LEFTWEAPONSPEEDMULT") so it uses the same "nn" status marker as other null entries instead of "OK"; locate the array of effect-to-icon mappings in EffectIconMap.as and change the trailing comment for that specific null element to "nn" to match the established pattern.source/actionscript/Common/skyui/util/Translator.as (2)
11-11: ⚡ Quick winAdd return type annotation.
The function signature is missing the return type. For better type safety and code clarity, add
: Stringafter the parameter list.📝 Proposed fix
- public static function translate(a_str: String) + public static function translate(a_str: String): String🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/actionscript/Common/skyui/util/Translator.as` at line 11, The function signature for translate is missing a return type; update the declaration of the static method translate in class Translator (public static function translate(a_str: String)) to include its return type by adding ": String" after the parameter list (i.e., public static function translate(a_str: String): String) so the method has explicit String return typing.
30-30: ⚡ Quick winAdd return type annotation.
The function signature is missing the return type. For better type safety and code clarity, add
: Stringafter the parameter list.📝 Proposed fix
- public static function translateNested(a_str: String) + public static function translateNested(a_str: String): String🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/actionscript/Common/skyui/util/Translator.as` at line 30, The function declaration for translateNested is missing a return type; update the signature of translateNested(a_str: String) to include a return type annotation (add ": String") so it reads translateNested(a_str: String): String, ensuring the function's return type is explicitly declared for type safety and clarity.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@source/actionscript/Common/skyui/util/ColorFunctions.as`:
- Around line 296-305: The modulo in loopValue doesn't handle negative a_val
because ActionScript's % preserves sign, causing loopValue(-1,360) to return -1
and breaking hsvToRgb/hslToRgb; update loopValue (the function named loopValue)
to normalize wrapping for negatives by computing a positive remainder (e.g.,
((a_val % a_max) + a_max) % a_max or add a_max when result is negative) so
returned hue values fall into the correct sextant for hsvToRgb and hslToRgb.
In `@source/actionscript/Common/skyui/util/ConfigManager.as`:
- Line 244: The parameter type for parseData is wrong: change the signature of
parseData from "parseData(a_data: Array)" to "parseData(a_data: String)" and
update any internal handling/comments to reflect that it receives the raw
response string from LoadVars.onData; also update any callers (e.g., the
LoadVars.onData assignment) or local variable types that assume an Array so they
treat a_data as a String (parse/split it into an Array where needed) to preserve
existing behavior.
- Around line 120-123: The addConstantTable function currently declares an
unused parameter a_class: Function; remove the unused parameter from the
addConstantTable signature and all its call sites so it only accepts a_name:
String, and update any references expecting the old two-arg form; verify
behavior against parseData which resolves constant tables via
_extConstantTableNames and ensure _extConstantTableNames.push(a_name) remains
unchanged (if instead the intent was to use a_class, implement logic in
addConstantTable to register/resolve that class and update parseData
accordingly—otherwise remove the parameter).
In `@source/actionscript/Common/skyui/util/DialogManager.as`:
- Around line 29-35: The close() method should avoid null dereferences: first,
only call gfx.managers.FocusHandler.instance.setFocus(...) when
skyui.util.DialogManager._previousFocus is non-null (or otherwise handle a null
previous focus safely), and then check if skyui.util.DialogManager._activeDialog
is non-null before calling _activeDialog.closeDialog() and setting _activeDialog
= null; update DialogManager.close to perform these null checks (referencing
DialogManager.close, _previousFocus and _activeDialog) so calling close() when
no dialog is active is safe.
- Line 21: The call that assigns skyui.util.DialogManager._activeDialog from
a_target.attachMovie(a_linkageID, "dialog", a_target.getNextHighestDepth(),
a_init) must validate the attachMovie result before using it; modify the code in
the function that performs this assignment to check if the returned value is
non-null/defined, and if it is null/undefined handle the error path (log/report
the missing linkageID a_linkageID and do not call methods like openDialog() or
set focus on _activeDialog), otherwise cast to BasicDialog and proceed as before
(ensuring subsequent calls like _activeDialog.openDialog() only happen when
_activeDialog is valid).
In `@source/actionscript/Common/skyui/util/EffectIconMap.as`:
- Line 256: The local variable resistType declared as "var resistType =
a_effectData.resistType" is unused; remove that declaration to eliminate the
dead variable and rely on the existing references to a_effectData.resistType
(e.g., in the switch that follows). Locate the resistType declaration in
EffectIconMap.as (near the switch that inspects a_effectData.resistType) and
delete the var resistType assignment so only the direct a_effectData.resistType
usage remains.
In `@source/actionscript/Common/skyui/util/GlobalFunctions.as`:
- Around line 43-50: The custom Array.prototype.indexOf implementation returns
undefined when not found; change it to return -1 to match standard behavior.
Locate the Array.prototype.indexOf function in GlobalFunctions.as and replace
the final return value so the function returns -1 when the element isn't found
(keeping the existing loop and return i behavior when found); ensure the
function always returns a numeric index to maintain compatibility with callers
that compare to -1 or perform arithmetic on the result.
- Around line 11-14: The clean function (GlobalFunctions.clean) incorrectly uses
a_str.indexOf(";") > 0 which ignores strings starting with ";" so
leading-commented lines won't be stripped; change the condition to detect any
presence of ";" (use >= 0 or != -1) and then slice using a_str.indexOf(";") so
that a_str = a_str.slice(0, a_str.indexOf(";")) runs whenever ";" exists (and
keep the rest of the function behavior intact).
In `@source/actionscript/Common/skyui/util/Translator.as`:
- Around line 43-56: In Translator.as inside the do/while loop that finds
placeholders, the closing brace search uses endIndex = a_str.indexOf("}",
offset) which can locate a `}` before the found `{`; change the search to start
from just after the opening brace (use indexOf("}", startIndex + 1)) so endIndex
always matches the corresponding closing brace for the found startIndex; update
the logic around startIndex/endIndex/offset (used in translate(s),
subStrings.push, and the slice operations) accordingly to ensure placeholders
are extracted and replaced correctly.
In `@source/actionscript/Common/skyui/util/Tween.as`:
- Line 18: Guard the zero-denominator when computing the local duration: in the
Tween class/method where the line computing "var duration = Math.abs((a_end -
current)/(a_end - a_start)) * a_duration;" appears, check the denominator (a_end
- a_start) first and if it's zero (or effectively zero) set duration to 0 (or
fallback to a_duration if you prefer consistent timing) instead of performing
the division; replace the direct expression with a small branch that uses the
same parameter names (a_end, a_start, current, a_duration) and assigns a safe
numeric duration to avoid NaN/Infinity.
---
Nitpick comments:
In `@source/actionscript/Common/skyui/util/ColorFunctions.as`:
- Around line 203-207: The HSV/HSB wrapper functions hsvToHex, hsbToRgb, and
hsbToHex are missing explicit parameter type annotations; update their
signatures to mark their input as an Array (e.g., change a_HSV and a_HSB
parameters to : Array) to match the rest of the class and the other
Array-accepting functions (also apply the same fix to the other occurrence
referenced at the end of the file). Keep the function bodies unchanged—only add
the : Array type annotation to each parameter in hsvToHex, hsbToRgb, and
hsbToHex (and the duplicate at the noted location).
In `@source/actionscript/Common/skyui/util/EffectIconMap.as`:
- Line 186: Update the comment for the null entry corresponding to
AV_LEFTWEAPONSPEEDMULT in EffectIconMap (the line with "null, // 132 -
AV_LEFTWEAPONSPEEDMULT") so it uses the same "nn" status marker as other null
entries instead of "OK"; locate the array of effect-to-icon mappings in
EffectIconMap.as and change the trailing comment for that specific null element
to "nn" to match the established pattern.
In `@source/actionscript/Common/skyui/util/Translator.as`:
- Line 11: The function signature for translate is missing a return type; update
the declaration of the static method translate in class Translator (public
static function translate(a_str: String)) to include its return type by adding
": String" after the parameter list (i.e., public static function
translate(a_str: String): String) so the method has explicit String return
typing.
- Line 30: The function declaration for translateNested is missing a return
type; update the signature of translateNested(a_str: String) to include a return
type annotation (add ": String") so it reads translateNested(a_str: String):
String, ensuring the function's return type is explicitly declared for type
safety and clarity.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 32773ac1-6dc2-45c5-8c31-70a1b32cbc91
📒 Files selected for processing (7)
source/actionscript/Common/skyui/util/ColorFunctions.assource/actionscript/Common/skyui/util/ConfigManager.assource/actionscript/Common/skyui/util/DialogManager.assource/actionscript/Common/skyui/util/EffectIconMap.assource/actionscript/Common/skyui/util/GlobalFunctions.assource/actionscript/Common/skyui/util/Translator.assource/actionscript/Common/skyui/util/Tween.as
Comment on lines
+296
to
+305
| private static function loopValue(a_val: Number, a_max: Number) | ||
| { | ||
| // $ trace(loopValue(360.1012, 360)) | ||
| // > 0.10120000000001 | ||
| // $ trace(loopValue(360, 360)) | ||
| // > 0 | ||
| // $ trace(loopValue(-1, 360)) | ||
| // > 359 | ||
| return (a_val % a_max); | ||
| } |
Contributor
There was a problem hiding this comment.
Negative values not handled correctly by modulo.
The comment states loopValue(-1, 360) returns 359, but ActionScript's % operator preserves the sign of the dividend. Actual result: (-1) % 360 = -1.
This causes hsvToRgb and hslToRgb to produce black (0,0,0) for any negative hue input, as the negative sextant falls through to the default case.
Proposed fix
private static function loopValue(a_val: Number, a_max: Number)
{
- // $ trace(loopValue(360.1012, 360))
- // > 0.10120000000001
- // $ trace(loopValue(360, 360))
- // > 0
- // $ trace(loopValue(-1, 360))
- // > 359
- return (a_val % a_max);
+ // Wraps value into [0, a_max) range, handling negatives correctly
+ var result: Number = a_val % a_max;
+ if (result < 0) {
+ result += a_max;
+ }
+ return result;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private static function loopValue(a_val: Number, a_max: Number) | |
| { | |
| // $ trace(loopValue(360.1012, 360)) | |
| // > 0.10120000000001 | |
| // $ trace(loopValue(360, 360)) | |
| // > 0 | |
| // $ trace(loopValue(-1, 360)) | |
| // > 359 | |
| return (a_val % a_max); | |
| } | |
| private static function loopValue(a_val: Number, a_max: Number) | |
| { | |
| // Wraps value into [0, a_max) range, handling negatives correctly | |
| var result: Number = a_val % a_max; | |
| if (result < 0) { | |
| result += a_max; | |
| } | |
| return result; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@source/actionscript/Common/skyui/util/ColorFunctions.as` around lines 296 -
305, The modulo in loopValue doesn't handle negative a_val because
ActionScript's % preserves sign, causing loopValue(-1,360) to return -1 and
breaking hsvToRgb/hslToRgb; update loopValue (the function named loopValue) to
normalize wrapping for negatives by computing a positive remainder (e.g.,
((a_val % a_max) + a_max) % a_max or add a_max when result is negative) so
returned hue values fall into the correct sextant for hsvToRgb and hslToRgb.
Comment on lines
+120
to
+123
| public static function addConstantTable(a_name: String, a_class: Function) | ||
| { | ||
| skyui.util.ConfigManager._extConstantTableNames.push(a_name); | ||
| } |
Contributor
There was a problem hiding this comment.
The a_class parameter is never used.
The function accepts a_class: Function but only uses a_name. Looking at parseData (lines 249-256), constant tables are resolved from _global by parsing the name string, so the a_class parameter serves no purpose. This is either dead code or an incomplete implementation.
If the class reference is intentionally unused, remove the parameter to avoid misleading callers.
Proposed fix if class parameter is unnecessary
- public static function addConstantTable(a_name: String, a_class: Function)
+ public static function addConstantTable(a_name: String)
{
skyui.util.ConfigManager._extConstantTableNames.push(a_name);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public static function addConstantTable(a_name: String, a_class: Function) | |
| { | |
| skyui.util.ConfigManager._extConstantTableNames.push(a_name); | |
| } | |
| public static function addConstantTable(a_name: String) | |
| { | |
| skyui.util.ConfigManager._extConstantTableNames.push(a_name); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@source/actionscript/Common/skyui/util/ConfigManager.as` around lines 120 -
123, The addConstantTable function currently declares an unused parameter
a_class: Function; remove the unused parameter from the addConstantTable
signature and all its call sites so it only accepts a_name: String, and update
any references expecting the old two-arg form; verify behavior against parseData
which resolves constant tables via _extConstantTableNames and ensure
_extConstantTableNames.push(a_name) remains unchanged (if instead the intent was
to use a_class, implement logic in addConstantTable to register/resolve that
class and update parseData accordingly—otherwise remove the parameter).
| } | ||
| } | ||
|
|
||
| private static function parseData(a_data: Array) |
Contributor
There was a problem hiding this comment.
Incorrect parameter type annotation.
LoadVars.onData passes a String, not an Array. The code works because AS2 is weakly typed, but the annotation is misleading.
- private static function parseData(a_data: Array)
+ private static function parseData(a_data: String)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private static function parseData(a_data: Array) | |
| private static function parseData(a_data: String) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@source/actionscript/Common/skyui/util/ConfigManager.as` at line 244, The
parameter type for parseData is wrong: change the signature of parseData from
"parseData(a_data: Array)" to "parseData(a_data: String)" and update any
internal handling/comments to reflect that it receives the raw response string
from LoadVars.onData; also update any callers (e.g., the LoadVars.onData
assignment) or local variable types that assume an Array so they treat a_data as
a String (parse/split it into an Array where needed) to preserve existing
behavior.
|
|
||
| skyui.util.DialogManager._previousFocus = gfx.managers.FocusHandler.instance.getFocus(0); | ||
|
|
||
| skyui.util.DialogManager._activeDialog = skyui.components.dialog.BasicDialog(a_target.attachMovie(a_linkageID, "dialog", a_target.getNextHighestDepth(), a_init)); |
Contributor
There was a problem hiding this comment.
Validate attachMovie result before use.
attachMovie can return null or undefined if the linkage ID doesn't exist in the library. If this occurs, subsequent operations (setting focus, calling openDialog()) will fail with a runtime exception.
🛡️ Proposed fix to add validation
- skyui.util.DialogManager._activeDialog = skyui.components.dialog.BasicDialog(a_target.attachMovie(a_linkageID, "dialog", a_target.getNextHighestDepth(), a_init));
+ var newDialog: MovieClip = a_target.attachMovie(a_linkageID, "dialog", a_target.getNextHighestDepth(), a_init);
+ if (!newDialog) {
+ return null;
+ }
+ skyui.util.DialogManager._activeDialog = skyui.components.dialog.BasicDialog(newDialog);
gfx.managers.FocusHandler.instance.setFocus(skyui.util.DialogManager._activeDialog, 0);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| skyui.util.DialogManager._activeDialog = skyui.components.dialog.BasicDialog(a_target.attachMovie(a_linkageID, "dialog", a_target.getNextHighestDepth(), a_init)); | |
| var newDialog: MovieClip = a_target.attachMovie(a_linkageID, "dialog", a_target.getNextHighestDepth(), a_init); | |
| if (!newDialog) { | |
| return null; | |
| } | |
| skyui.util.DialogManager._activeDialog = skyui.components.dialog.BasicDialog(newDialog); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@source/actionscript/Common/skyui/util/DialogManager.as` at line 21, The call
that assigns skyui.util.DialogManager._activeDialog from
a_target.attachMovie(a_linkageID, "dialog", a_target.getNextHighestDepth(),
a_init) must validate the attachMovie result before using it; modify the code in
the function that performs this assignment to check if the returned value is
non-null/defined, and if it is null/undefined handle the error path (log/report
the missing linkageID a_linkageID and do not call methods like openDialog() or
set focus on _activeDialog), otherwise cast to BasicDialog and proceed as before
(ensuring subsequent calls like _activeDialog.openDialog() only happen when
_activeDialog is valid).
Comment on lines
+29
to
+35
| public static function close() | ||
| { | ||
| gfx.managers.FocusHandler.instance.setFocus(skyui.util.DialogManager._previousFocus, 0); | ||
|
|
||
| skyui.util.DialogManager._activeDialog.closeDialog(); | ||
| skyui.util.DialogManager._activeDialog = null; | ||
| } |
Contributor
There was a problem hiding this comment.
Guard against null _activeDialog in close().
The close() method is public and can be called externally at any time, including when no dialog is active. Line 33 will throw a null pointer exception if _activeDialog is null. This can occur if close() is called twice in succession or before any dialog has been opened.
🐛 Proposed fix to add null guard
public static function close()
{
+ if (!skyui.util.DialogManager._activeDialog) {
+ return;
+ }
+
gfx.managers.FocusHandler.instance.setFocus(skyui.util.DialogManager._previousFocus, 0);
skyui.util.DialogManager._activeDialog.closeDialog();
skyui.util.DialogManager._activeDialog = null;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public static function close() | |
| { | |
| gfx.managers.FocusHandler.instance.setFocus(skyui.util.DialogManager._previousFocus, 0); | |
| skyui.util.DialogManager._activeDialog.closeDialog(); | |
| skyui.util.DialogManager._activeDialog = null; | |
| } | |
| public static function close() | |
| { | |
| if (!skyui.util.DialogManager._activeDialog) { | |
| return; | |
| } | |
| gfx.managers.FocusHandler.instance.setFocus(skyui.util.DialogManager._previousFocus, 0); | |
| skyui.util.DialogManager._activeDialog.closeDialog(); | |
| skyui.util.DialogManager._activeDialog = null; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@source/actionscript/Common/skyui/util/DialogManager.as` around lines 29 - 35,
The close() method should avoid null dereferences: first, only call
gfx.managers.FocusHandler.instance.setFocus(...) when
skyui.util.DialogManager._previousFocus is non-null (or otherwise handle a null
previous focus safely), and then check if skyui.util.DialogManager._activeDialog
is non-null before calling _activeDialog.closeDialog() and setting _activeDialog
= null; update DialogManager.close to perform these null checks (referencing
DialogManager.close, _previousFocus and _activeDialog) so calling close() when
no dialog is active is safe.
|
|
||
| // Replace base icon for damage health with resistType icon | ||
| if (actorValue == skyui.defines.Actor.AV_HEALTH) { | ||
| var resistType = a_effectData.resistType; |
Contributor
There was a problem hiding this comment.
Remove unused variable.
The variable resistType is declared but never used. The switch statement on line 258 references a_effectData.resistType directly.
🧹 Proposed fix
- if (actorValue == skyui.defines.Actor.AV_HEALTH) {
- var resistType = a_effectData.resistType;
-
+ if (actorValue == skyui.defines.Actor.AV_HEALTH) {
switch (a_effectData.resistType) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| var resistType = a_effectData.resistType; | |
| if (actorValue == skyui.defines.Actor.AV_HEALTH) { | |
| switch (a_effectData.resistType) { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@source/actionscript/Common/skyui/util/EffectIconMap.as` at line 256, The
local variable resistType declared as "var resistType = a_effectData.resistType"
is unused; remove that declaration to eliminate the dead variable and rely on
the existing references to a_effectData.resistType (e.g., in the switch that
follows). Locate the resistType declaration in EffectIconMap.as (near the switch
that inspects a_effectData.resistType) and delete the var resistType assignment
so only the direct a_effectData.resistType usage remains.
Comment on lines
+11
to
+14
| public static function clean(a_str: String) | ||
| { | ||
| if (a_str.indexOf(";") > 0) | ||
| a_str = a_str.slice(0,a_str.indexOf(";")); |
Contributor
There was a problem hiding this comment.
Comment at position 0 is not removed.
indexOf(";") > 0 skips strings that start with ;. If the intent is to strip comments, use >= 0 or != -1.
Proposed fix
public static function clean(a_str: String)
{
- if (a_str.indexOf(";") > 0)
+ if (a_str.indexOf(";") != -1)
a_str = a_str.slice(0,a_str.indexOf(";"));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public static function clean(a_str: String) | |
| { | |
| if (a_str.indexOf(";") > 0) | |
| a_str = a_str.slice(0,a_str.indexOf(";")); | |
| public static function clean(a_str: String) | |
| { | |
| if (a_str.indexOf(";") != -1) | |
| a_str = a_str.slice(0,a_str.indexOf(";")); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@source/actionscript/Common/skyui/util/GlobalFunctions.as` around lines 11 -
14, The clean function (GlobalFunctions.clean) incorrectly uses
a_str.indexOf(";") > 0 which ignores strings starting with ";" so
leading-commented lines won't be stripped; change the condition to detect any
presence of ";" (use >= 0 or != -1) and then slice using a_str.indexOf(";") so
that a_str = a_str.slice(0, a_str.indexOf(";")) runs whenever ";" exists (and
keep the rest of the function behavior intact).
Comment on lines
+43
to
+50
| Array.prototype.indexOf = function (a_element) | ||
| { | ||
| for (var i=0; i<this.length; i++) | ||
| if (this[i] == a_element) | ||
| return i; | ||
|
|
||
| return undefined; | ||
| }; |
Contributor
There was a problem hiding this comment.
indexOf returns undefined instead of conventional -1.
Standard Array.prototype.indexOf returns -1 when the element is not found. Returning undefined may break callers that check if (arr.indexOf(x) == -1) or use the result in arithmetic.
Proposed fix
Array.prototype.indexOf = function (a_element)
{
for (var i=0; i<this.length; i++)
if (this[i] == a_element)
return i;
- return undefined;
+ return -1;
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Array.prototype.indexOf = function (a_element) | |
| { | |
| for (var i=0; i<this.length; i++) | |
| if (this[i] == a_element) | |
| return i; | |
| return undefined; | |
| }; | |
| Array.prototype.indexOf = function (a_element) | |
| { | |
| for (var i=0; i<this.length; i++) | |
| if (this[i] == a_element) | |
| return i; | |
| return -1; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@source/actionscript/Common/skyui/util/GlobalFunctions.as` around lines 43 -
50, The custom Array.prototype.indexOf implementation returns undefined when not
found; change it to return -1 to match standard behavior. Locate the
Array.prototype.indexOf function in GlobalFunctions.as and replace the final
return value so the function returns -1 when the element isn't found (keeping
the existing loop and return i behavior when found); ensure the function always
returns a numeric index to maintain compatibility with callers that compare to
-1 or perform arithmetic on the result.
Comment on lines
+43
to
+56
| do { | ||
| var startIndex = a_str.indexOf("{", offset); | ||
| var endIndex = a_str.indexOf("}", offset); | ||
|
|
||
| if (startIndex != -1 && endIndex != -1 && startIndex < endIndex) { | ||
| var s = a_str.slice(startIndex + 1, endIndex); | ||
| subStrings.push(skyui.util.Translator.translate(s)); | ||
|
|
||
| a_str = a_str.slice(0, startIndex + 1) + a_str.slice(endIndex); | ||
|
|
||
| offset = startIndex + 2; | ||
| } else { | ||
| break; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Search for strings that might contain stray } before { which would break translateNested
# Search for patterns with } before { in likely translation string contexts
rg -nP '\}[^{]*\{' --type=as -C2Repository: doodlum/SkyUI-Community
Length of output: 94
🏁 Script executed:
#!/bin/bash
# Find the Translator.as file
find . -name "Translator.as" -type fRepository: doodlum/SkyUI-Community
Length of output: 121
🏁 Script executed:
#!/bin/bash
# Once found, read the relevant lines to verify the code matches
head -60 ./source/actionscript/Common/skyui/util/Translator.as | tail -25Repository: doodlum/SkyUI-Community
Length of output: 701
🏁 Script executed:
#!/bin/bash
# Try searching for translation strings with } before { using a simpler pattern
# Since --type=as doesn't work, search for .as files directly
rg '\}[^{]*\{' --include="*.as" -C2Repository: doodlum/SkyUI-Community
Length of output: 509
🏁 Script executed:
#!/bin/bash
# Search for translation strings with } before { using correct rg syntax
rg '\}[^{]*\{' -g "*.as" -C2Repository: doodlum/SkyUI-Community
Length of output: 39005
🏁 Script executed:
#!/bin/bash
# Also let's look at the broader context of the function to understand its purpose
sed -n '30,70p' ./source/actionscript/Common/skyui/util/Translator.asRepository: doodlum/SkyUI-Community
Length of output: 1082
Fix closing brace search to find the matching closing brace, not just any brace after offset.
Line 45 searches for } starting from offset, same as the opening brace on line 44. This breaks when a stray } appears before a { in the input string (e.g., "Text } {$Placeholder}"), causing startIndex = 6 and endIndex = 5. The condition startIndex < endIndex then fails and the loop exits without processing the placeholder.
Search for } from startIndex + 1 instead to ensure it matches the opening brace:
do {
var startIndex = a_str.indexOf("{", offset);
- var endIndex = a_str.indexOf("}", offset);
+ var endIndex = (startIndex != -1) ? a_str.indexOf("}", startIndex + 1) : -1;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| do { | |
| var startIndex = a_str.indexOf("{", offset); | |
| var endIndex = a_str.indexOf("}", offset); | |
| if (startIndex != -1 && endIndex != -1 && startIndex < endIndex) { | |
| var s = a_str.slice(startIndex + 1, endIndex); | |
| subStrings.push(skyui.util.Translator.translate(s)); | |
| a_str = a_str.slice(0, startIndex + 1) + a_str.slice(endIndex); | |
| offset = startIndex + 2; | |
| } else { | |
| break; | |
| } | |
| do { | |
| var startIndex = a_str.indexOf("{", offset); | |
| var endIndex = (startIndex != -1) ? a_str.indexOf("}", startIndex + 1) : -1; | |
| if (startIndex != -1 && endIndex != -1 && startIndex < endIndex) { | |
| var s = a_str.slice(startIndex + 1, endIndex); | |
| subStrings.push(skyui.util.Translator.translate(s)); | |
| a_str = a_str.slice(0, startIndex + 1) + a_str.slice(endIndex); | |
| offset = startIndex + 2; | |
| } else { | |
| break; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@source/actionscript/Common/skyui/util/Translator.as` around lines 43 - 56, In
Translator.as inside the do/while loop that finds placeholders, the closing
brace search uses endIndex = a_str.indexOf("}", offset) which can locate a `}`
before the found `{`; change the search to start from just after the opening
brace (use indexOf("}", startIndex + 1)) so endIndex always matches the
corresponding closing brace for the found startIndex; update the logic around
startIndex/endIndex/offset (used in translate(s), subStrings.push, and the slice
operations) accordingly to ensure placeholders are extracted and replaced
correctly.
| } | ||
|
|
||
| var current: Number = a_obj[a_prop]; | ||
| var duration = Math.abs((a_end - current)/(a_end - a_start)) * a_duration; |
Contributor
There was a problem hiding this comment.
Guard zero-denominator duration calculation.
If a_start == a_end, the current formula divides by zero and can produce NaN/Infinity, which can break tween timing and callback flow.
Suggested patch
- var current: Number = a_obj[a_prop];
- var duration = Math.abs((a_end - current)/(a_end - a_start)) * a_duration;
+ var current: Number = a_obj[a_prop];
+ var totalDelta: Number = a_end - a_start;
+ var duration: Number = (totalDelta == 0)
+ ? 0
+ : Math.abs((a_end - current) / totalDelta) * a_duration;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| var duration = Math.abs((a_end - current)/(a_end - a_start)) * a_duration; | |
| var totalDelta: Number = a_end - a_start; | |
| var duration: Number = (totalDelta == 0) | |
| ? 0 | |
| : Math.abs((a_end - current) / totalDelta) * a_duration; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@source/actionscript/Common/skyui/util/Tween.as` at line 18, Guard the
zero-denominator when computing the local duration: in the Tween class/method
where the line computing "var duration = Math.abs((a_end - current)/(a_end -
a_start)) * a_duration;" appears, check the denominator (a_end - a_start) first
and if it's zero (or effectively zero) set duration to 0 (or fallback to
a_duration if you prefer consistent timing) instead of performing the division;
replace the direct expression with a small branch that uses the same parameter
names (a_end, a_start, current, a_duration) and assigns a safe numeric duration
to avoid NaN/Infinity.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit