refactor(item-menus): restore scripts from decompiled to source version#200
Open
zndxcvbn wants to merge 25 commits into
Open
refactor(item-menus): restore scripts from decompiled to source version#200zndxcvbn wants to merge 25 commits into
zndxcvbn wants to merge 25 commits into
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAcross ItemMenus, classes and helpers were converted from untyped ActionScript to explicitly typed declarations with explicit public/private visibility. Signatures, fields, and internal helpers now include type annotations; observable UI/menu behavior and data-processing rules are preserved. ChangesActionScript Type System & Visibility Modernization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Suggested Reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 2
🤖 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/ItemMenus/ItemMenu.as`:
- Line 615: The base-class method updateBottomBar(a_bSelected: Boolean) is
declared private but is invoked polymorphically (this.updateBottomBar(false))
and overridden in subclasses (MagicMenu, InventoryMenu, GiftMenu, ContainerMenu,
BarterMenu); change its visibility from private to protected so subclasses can
override it while keeping encapsulation, ensure the method signature remains the
same and preserve or implement a sensible default body in
ItemMenu.updateBottomBar to avoid runtime errors when called from the base
class.
In `@source/actionscript/ItemMenus/MagicMenu.as`:
- Around line 88-91: In DragonSoulSpent(), updateBottomBar() is called without
the required Boolean selected-state parameter; change the call to pass the
current selected state (e.g., this.updateBottomBar(true) if the item remains
selected, or better: this.updateBottomBar(this.itemCard &&
this.itemCard.isSelected)) so the UI refreshes using the selected-button set;
locate the DragonSoulSpent function and the updateBottomBar(Boolean) method and
supply the appropriate Boolean (preferably this.itemCard.isSelected) when
invoking updateBottomBar.
🪄 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: abe5553e-e171-49bc-b12a-1853fde403e1
📒 Files selected for processing (15)
source/actionscript/ItemMenus/BarterDataSetter.assource/actionscript/ItemMenus/BarterMenu.assource/actionscript/ItemMenus/BottomBar.assource/actionscript/ItemMenus/CategoryList.assource/actionscript/ItemMenus/CategoryListEntry.assource/actionscript/ItemMenus/ContainerMenu.assource/actionscript/ItemMenus/GiftMenu.assource/actionscript/ItemMenus/InventoryDataSetter.assource/actionscript/ItemMenus/InventoryIconSetter.assource/actionscript/ItemMenus/InventoryLists.assource/actionscript/ItemMenus/InventoryMenu.assource/actionscript/ItemMenus/ItemMenu.assource/actionscript/ItemMenus/ItemcardDataExtender.assource/actionscript/ItemMenus/MagicIconSetter.assource/actionscript/ItemMenus/MagicMenu.as
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/ItemMenus/ItemCard.as`:
- Line 65: Remove the no-op self-assignment in ItemCard: delete the line that
assigns this.QuantitySlider_mc = this.QuantitySlider_mc; (a decompiler artifact)
from the ItemCard class/constructor so the QuantitySlider_mc field is not
redundantly reassigned; search for and remove identical patterns where a
property is assigned to itself (e.g., other occurrences like QuantitySlider_mc)
to clean up decompiler artifacts.
- Around line 310-316: Remove the duplicated calls to gotoAndStop on the
ShoutTextInstance entries: inside the branch that checks bWordKnown and the else
branch, only call this["ShoutTextInstance" + i].gotoAndStop("Known") or
this["ShoutTextInstance" + i].gotoAndStop("Unlocked") once (respectively).
Locate the code in ItemCard.as around the bWordKnown conditional and delete the
redundant second gotoAndStop invocation so each branch makes a single call to
gotoAndStop with the appropriate frame name.
🪄 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: fd64998f-d273-447b-8872-048c50748f5a
📒 Files selected for processing (3)
source/actionscript/ItemMenus/ItemCard.assource/actionscript/ItemMenus/MagicDataSetter.assource/actionscript/ItemMenus/MagicMenu.as
🚧 Files skipped from review as they are similar to previous changes (1)
- source/actionscript/ItemMenus/MagicMenu.as
| Shared.GlobalFunc.MaintainTextFormat(); | ||
| Shared.GlobalFunc.AddReverseFunctions(); | ||
| gfx.events.EventDispatcher.initialize(this); | ||
| this.QuantitySlider_mc = this.QuantitySlider_mc; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for any other self-assignments in the restored files to spot residual JPEXS artifacts.
rg -nP '\bthis\.(\w+)\s*=\s*this\.\1\s*;' -g '*.as'Repository: doodlum/SkyUI-Community
Length of output: 2652
Remove self-assignment — confirmed as JPEXS decompiler artifact.
this.QuantitySlider_mc = this.QuantitySlider_mc; is a no-op and is part of a systematic decompiler artifact pattern. A search across the codebase found 28+ identical self-assignments in restored ActionScript files (TrainingMenu, StartMenu, SleepWaitMenu, CenteredList, LevelUpMenu, DialogueMenu, Console, TweenMenu, MessageBox, CraftingMenu, Quest_Journal, and MarkerDescription). This line should be removed.
🤖 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/ItemMenus/ItemCard.as` at line 65, Remove the no-op
self-assignment in ItemCard: delete the line that assigns this.QuantitySlider_mc
= this.QuantitySlider_mc; (a decompiler artifact) from the ItemCard
class/constructor so the QuantitySlider_mc field is not redundantly reassigned;
search for and remove identical patterns where a property is assigned to itself
(e.g., other occurrences like QuantitySlider_mc) to clean up decompiler
artifacts.
Comment on lines
+310
to
+316
| } else if (bWordKnown) { | ||
| this["ShoutTextInstance" + i].gotoAndStop("Known"); | ||
| this["ShoutTextInstance" + i].gotoAndStop("Known"); | ||
| } else { | ||
| this["ShoutTextInstance" + i].gotoAndStop("Unlocked"); | ||
| this["ShoutTextInstance" + i].gotoAndStop("Unlocked"); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and read the relevant section
head -n 320 source/actionscript/ItemMenus/ItemCard.as | tail -n 20Repository: doodlum/SkyUI-Community
Length of output: 1666
🏁 Script executed:
# Check for README and project metadata
head -50 README.md 2>/dev/null || head -50 readme.txt 2>/dev/null || ls -la | head -20Repository: doodlum/SkyUI-Community
Length of output: 2319
🏁 Script executed:
# Search for other instances of duplicate gotoAndStop calls to confirm the pattern
rg "gotoAndStop\(" source/actionscript/ -A 1 | head -100Repository: doodlum/SkyUI-Community
Length of output: 6633
🏁 Script executed:
# Search for comments or TODOs around this area mentioning decompiler or artifacts
rg "decompil|JPEXS|artifact" source/actionscript/ -iRepository: doodlum/SkyUI-Community
Length of output: 3654
🏁 Script executed:
# Check if there are any .swf files or original source references in the project
find . -name "*.swf" -o -name "*original*" -o -name "*backup*" | head -20Repository: doodlum/SkyUI-Community
Length of output: 789
🏁 Script executed:
# Search for other patterns of duplicate method calls (not just gotoAndStop) to see if this is systematic
rg "this\[.*\]\.\w+\(.*\);\s*this\[.*\]\.\1\(" source/actionscript/ --multilineRepository: doodlum/SkyUI-Community
Length of output: 313
🏁 Script executed:
# Try the pattern search with PCRE2 flag for backreferences
rg "gotoAndStop\(([^)]+)\);\s*\[ShoutText[^\]]*\]\.gotoAndStop\(\1\)" source/actionscript/ --pcre2 -A 1 -B 1Repository: doodlum/SkyUI-Community
Length of output: 49
🏁 Script executed:
# Look at broader context around the suspicious code in ItemCard.as
sed -n '300,325p' source/actionscript/ItemMenus/ItemCard.asRepository: doodlum/SkyUI-Community
Length of output: 1947
🏁 Script executed:
# Check if there are any GitHub issues or discussions about duplicate gotoAndStop calls
find . -name "*.md" -o -name "*.txt" | xargs grep -l "duplicate\|gotoAndStop" 2>/dev/nullRepository: doodlum/SkyUI-Community
Length of output: 96
Remove duplicate gotoAndStop calls — confirmed decompiler artifacts.
The duplicate calls in both the Known and Unlocked branches are confirmed. Given this is a decompilation restoration project and JPEXS decompiler issues are already documented in the codebase (see BasicListEntry.as), these redundant statements are almost certainly artifacts. Calling the same method twice with identical arguments serves no purpose and should be removed.
Cleanup to apply
} else if (bWordKnown) {
this["ShoutTextInstance" + i].gotoAndStop("Known");
- this["ShoutTextInstance" + i].gotoAndStop("Known");
} else {
this["ShoutTextInstance" + i].gotoAndStop("Unlocked");
- this["ShoutTextInstance" + i].gotoAndStop("Unlocked");
}🤖 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/ItemMenus/ItemCard.as` around lines 310 - 316, Remove the
duplicated calls to gotoAndStop on the ShoutTextInstance entries: inside the
branch that checks bWordKnown and the else branch, only call
this["ShoutTextInstance" + i].gotoAndStop("Known") or this["ShoutTextInstance" +
i].gotoAndStop("Unlocked") once (respectively). Locate the code in ItemCard.as
around the bWordKnown conditional and delete the redundant second gotoAndStop
invocation so each branch makes a single call to gotoAndStop with the
appropriate frame name.
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.
Restoring the original appearance of scripts from
ItemMenusadapted for import into JPEXS.The number of
this.scripts before and after the change should be the same, and if the scripts import without errors, then everything is fine.If the number of
this.scripts is the same or there are errors, then to check:Test_1.asandTest_2.asCtrl + Shift + P--> Compare Active File With...Test_1.as, place the selected script code from the main branchcmake --build --preset debugTest_2.as, place the decompiled script code from JPEXS after importing the script_locN_variable, then everything is fineSummary by CodeRabbit
Refactor
Bug Fixes / Polish
Stability