-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dialog box choice stuff (mega-PR lol) #49
Conversation
WalkthroughThe project has undergone a UI enhancement and codebase cleanup, focusing on streamlining user interactions and improving the visual elements. Key updates include repositioning UI components, enriching text content, adopting shorthand coding conventions, and refactoring for better modularity. New functionalities for list management and animation have been introduced, alongside semantic changes in variable naming for clarity. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (12)
- pets-gd/scenes/dialogchoice.tscn (1 hunks)
- pets-lib/src/battle/mod.rs (2 hunks)
- pets-lib/src/battle/player.rs (2 hunks)
- pets-lib/src/consts.rs (3 hunks)
- pets-lib/src/dialogue/dbox/dchoice.rs (2 hunks)
- pets-lib/src/dialogue/dbox/mod.rs (10 hunks)
- pets-lib/src/lib.rs (5 hunks)
- pets-lib/src/main_menu.rs (3 hunks)
- pets-lib/src/stats/stats_interface.rs (1 hunks)
- pets-lib/src/util.rs (2 hunks)
- pets-lib/src/world/playercb.rs (1 hunks)
- pets-lib/src/wrapped.rs (1 hunks)
Files skipped from review due to trivial changes (1)
- pets-lib/src/battle/player.rs
Additional comments: 28
pets-gd/scenes/dialogchoice.tscn (3)
- 4-5: The
offset_right
andoffset_bottom
properties of theDChoice
node have been increased significantly from 40.0 to 200.0. This is a large change and could affect the layout significantly. Ensure that this change is consistent with the intended design and does not cause any visual issues in the UI.- 10-10: The
Label
node's default color has been changed. Verify that this new color aligns with the game's color scheme and UI design guidelines.- 13-13: The text for the
Label
node has been changed from "Amogus" to "Sample Text Words Words More Words". Ensure that this change is intentional and correctly reflects the desired content in the game's dialog choices.pets-lib/src/consts.rs (3)
- 5-5: The
TransitionType
is now being imported from the parent module. Confirm that this change does not affect any other modules that might be usingTransitionType
differently.- 33-34: The constants
DBOX_CHOICE_TWEEN_TIME
andDBOX_CHOICE_TWEEN_TRANS
in thedialogue
module have been modified to use values frommain_menu
. This change suggests a unification of tweening behavior across different UI elements. Confirm that this shared behavior is consistent with the design requirements.- 46-46: The
MENU_WAVE_BBCODE
inmain_menu
now referencesDBOX_SELECTION_BBCODE
fromdialogue
. This change indicates a shared visual effect between menu and dialogue choices. Verify that this is the intended effect and that it works correctly in both contexts.pets-lib/src/lib.rs (2)
- 29-29: The
choicelist
module has been removed, and thewrapped
module has been added. Ensure that all references tochoicelist
have been properly updated to usewrapped
and that no orphaned code remains.- 62-65: Instances of
StatsInterface
andDBoxInterface
are now created usingnew_alloc
instead ofalloc_gd
. Confirm that this change aligns with the memory management strategy and that the new method properly handles the lifecycle of these instances.pets-lib/src/util.rs (1)
- 48-60: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [36-57]
The
tween
function's parameters have been renamed fromfrom
tostart_value
andtarget
toend_value
. This is a good change for clarity. Ensure that all calls totween
throughout the codebase have been updated to reflect these new parameter names.pets-lib/src/dialogue/dbox/dchoice.rs (3)
- 17-20: The base class of
DChoice
has been changed fromContainer
toMarginContainer
. Verify that this change does not introduce any layout issues and that theMarginContainer
properties are being utilized correctly.- 30-32: A private method
txt_label
has been added to encapsulate the retrieval of the text label. This is a good encapsulation practice. Ensure that it is being used consistently wherever the text label is needed.- 35-47: New methods
tween_label
andnew_container
have been added for label animation and container creation. Verify that these methods are correctly implemented and that the animations perform as expected.pets-lib/src/stats/stats_interface.rs (1)
- 36-42: The variable
lv
has been renamed tolvl
within thefn_name
function. This is a minor change but improves readability. Confirm that this change has been propagated to all usages of the variable to maintain consistency.pets-lib/src/world/playercb.rs (1)
- 101-101: The logic for updating past rotations in the
PlayerCB
class has been simplified. Confirm that this change does not alter the intended behavior and that the simplification maintains the same functionality.pets-lib/src/battle/mod.rs (3)
- 56-56: The
choices
field in theBattleEngine
struct has been changed to use theWrapped
struct. Ensure that the new structure is properly integrated and that all related methods have been updated accordingly.- 102-103: The
ready
method now initializeschoices
usingfrom_children_of
from thewrapped
module. Verify that this method correctly initializes the choices and that the order of elements is maintained.- 107-135: The
process
method has been refactored to use functions from thewrapped
module for input processing and action handling. Ensure that the refactored method correctly processes user input and that the battle choices are handled as intended.pets-lib/src/main_menu.rs (3)
- 26-26: The
tween_choice_to
function has been modified to take a non-mutable reference toRichTextLabel
. Confirm that this change does not affect the functionality of the tweening behavior and that it is consistent with the intended usage.- 70-70: The
choices
field has been updated to use theWrapped
struct. Verify that the new structure is properly integrated and that the logic for menu choice processing has been correctly updated.- 76-118: The logic for processing menu choices has been restructured using pattern matching. Ensure that the new structure correctly handles user input and that the menu choices are processed as intended.
pets-lib/src/wrapped.rs (2)
- 73-76: The
Wrapped
struct has been introduced to manage lists of elements. Verify that this new structure is correctly implemented and that it integrates well with the rest of the system, particularly in managing user input and list creation.- 113-126: The
from_children_of
function creates aWrapped
list from the child nodes of a parent node. Confirm that this function correctly maps the children to the corresponding enum variants and that the order is preserved.pets-lib/src/dialogue/dbox/mod.rs (6)
- 64-65: The
awaiting_choice
field has been added to theDialogBox
struct. Ensure that this new field is used correctly to manage the state of the dialog box when awaiting user input.- 66-67: The
choices
field has been updated to use theWrapped
struct. Verify that this new structure is properly integrated and that the dialog box choices are managed correctly.- 155-169: The
run_ix_ending
method has been refactored. Confirm that the refactoring maintains the original functionality and that the method correctly handles the different types of dialogue endings.- 171-184: The
on_accept
method has been added to handle user input. Verify that this method correctly processes the acceptance action and that it properly advances the dialog or ends the interaction as intended.- 186-227: The
process_choice_input
method has been refactored. Ensure that the refactored method correctly processes user input for choices and that the dialog box responds appropriately.- 396-428: The
MetaPair
struct has been added. Verify that this new struct is used correctly to manage temporary and permanent metadata for dialogue and that it integrates well with the existing system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- pets-lib/src/util.rs (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- pets-lib/src/util.rs
873751e
to
075d0c4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- pets-lib/src/util.rs (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- pets-lib/src/util.rs
This one's been going on for way too long. There's one last bug where the second page of the Rodrick Sign interaction won't let you move onto the third, but besides that it seems to work up until then. Gonna merge what I have into master so far, so the battle engine branch can stay somewhat up-to-date, since those 2 features are somewhat related.
Summary by CodeRabbit
New Features
Wrapped
struct to manage menu choices.Bug Fixes
Refactor
wrapped
module functions.Style
Documentation