Remap menu #76
Remap menu #76
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes update various project configurations, scene layouts, and import settings. The repository’s ignore patterns now include additional file types. The engine’s stage management is expanded with a new "Controls" value and a corresponding scene transition to a remap interface. Several UI scenes have adjusted node positions, textures, gradients, and key mappings. New classes and methods support control scheme selection and input reconfiguration. Multiple asset import configuration files have been added or updated across remapping and note management modules. Changes
Sequence Diagram(s)sequenceDiagram
participant Game
participant StageProducer
participant RemapScene
Game->>StageProducer: Request transition to Controls stage
StageProducer->>RemapScene: Load "Remap.tscn" scene
RemapScene-->>Game: Display remapping interface
sequenceDiagram
participant User
participant TitleScreen
participant ControlSettings
participant InputHandler
User->>TitleScreen: Click "Change Controls" button
TitleScreen->>ControlSettings: Open control settings UI
ControlSettings->>User: Show control scheme options
User->>ControlSettings: Select desired control scheme
ControlSettings->>InputHandler: Update project settings & invoke LoadControlScheme()
InputHandler->>Engine: Reconfigure input mappings
Poem
🪧 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (12)
scenes/Puppets/scripts/PuppetTemplate.cs (2)
27-27: Extract magic number to a constant.Consider extracting the value
155to a named constant at the class level for better maintainability and clarity.public partial class PuppetTemplate : Node2D { + private const float HEALTH_BAR_VERTICAL_OFFSET = 155f; // ... - _healthBar.Position += Vector2.Down * 155; + _healthBar.Position += Vector2.Down * HEALTH_BAR_VERTICAL_OFFSET;
4-6: Address the TODO comment about interfaces.The TODO comment suggests exploring interfaces. Consider implementing an
IBattleEntityinterface to define the contract for entities with health and damage mechanics.Would you like me to help design and implement the interface?
Globals/StageProducer.cs (1)
163-165: Add error handling for scene transition.Consider adding error handling to gracefully handle cases where the Remap scene file is missing.
case Stages.Controls: + var scenePath = "res://scenes/Remapping/Remap.tscn"; + if (!ResourceLoader.Exists(scenePath)) + { + GD.PrintErr($"Scene file not found: {scenePath}"); + return; + } - GetTree().ChangeSceneToFile("res://scenes/Remapping/Remap.tscn"); + GetTree().ChangeSceneToFile(scenePath); break;scenes/SceneTransitions/TitleScreen.tscn (1)
108-112: Consider button placement and visibility.The new button is placed at the bottom of VBoxContainer, separate from other buttons in HBoxContainer. Consider:
- Moving it into the HBoxContainer for consistent layout
- Adding visible property like other buttons
-[node name="Button" type="Button" parent="VBoxContainer"] +[node name="MarginContainer4" type="MarginContainer" parent="VBoxContainer/HBoxContainer"] +layout_mode = 2 +size_flags_horizontal = 3 +size_flags_stretch_ratio = 0.5 +theme_override_constants/margin_left = 20 +theme_override_constants/margin_top = 20 +theme_override_constants/margin_right = 20 +theme_override_constants/margin_bottom = 20 + +[node name="Button" type="Button" parent="VBoxContainer/HBoxContainer/MarginContainer4"] layout_mode = 2 +size_flags_horizontal = 3 text = "Change Controls" script = ExtResource("2_7f3m6") ScenePath = 4scenes/Maps/scripts/Cartographer.cs (1)
66-66: Remove debug print statement.Debug print statements should be removed before production deployment.
- GD.Print(validButtons);scenes/Remapping/ControlSchemes.cs (1)
52-75: Consider using constants for asset paths.Hardcoded asset paths make maintenance more difficult and are prone to typos.
Consider creating constants or a configuration file for asset paths:
public static class AssetPaths { public const string KEY_ASSETS_BASE = "res://scenes/Remapping/assets/"; public static class Keys { public const string A_KEY = KEY_ASSETS_BASE + "A_Key_Light.png"; public const string D_KEY = KEY_ASSETS_BASE + "D_Key_Light.png"; // ... other keys } }Then use these constants in the SpriteMappings dictionary.
scenes/Remapping/ControlSettings.cs (2)
59-59: Remove debug print statement.Debug print statement should be removed before merging.
-GD.Print(scheme);
35-37: Add error handling for project settings save.The code doesn't verify if the project settings were saved successfully.
Add error handling:
ProjectSettings.SetSetting("game/input_scheme", scheme); -ProjectSettings.Save(); +Error err = ProjectSettings.Save(); +if (err != Error.Ok) +{ + GD.PrintErr($"Failed to save project settings: {err}"); +}Also applies to: 43-45, 51-53
scenes/BattleDirector/test_battle_scene.tscn (1)
11-18: Consider extracting gradient as a shared resource.The gradient resource is used for visual styling. If this gradient style is used across multiple scenes, consider moving it to a shared resource file to maintain consistency and ease updates.
Create a new shared resource file:
+ # res://resources/gradients/dark_theme.tres + [gd_resource type="GradientTexture2D" load_steps=2 format=3] + [sub_resource type="Gradient" id="Gradient_8uy3a"] + offsets = PackedFloat32Array(0, 0.766234, 1) + colors = PackedColorArray(0.0823529, 0, 0.0784314, 1, 0.305882, 0.247059, 0.321569, 1, 0.27451, 0.243137, 0.403922, 1) + [resource] + gradient = SubResource("Gradient_8uy3a") + fill_from = Vector2(1, 0) + fill_to = Vector2(0.738532, 1)scenes/NoteManager/note_manager.tscn (2)
20-22: Consider using a scene for repeated arrow outline setup.The arrow outline setup is repeated for each direction. Consider creating a reusable arrow scene to reduce duplication and ease maintenance.
Create a new scene file:
+ # res://scenes/NoteManager/arrow.tscn + [gd_scene format=3] + [node name="Arrow" type="Sprite2D"] + texture = ExtResource("new_arrow") + [node name="Outline" type="Sprite2D" parent="."] + modulate = Color(0, 0, 0, 1) + texture = ExtResource("arrow_outline")Also applies to: 30-32, 40-42, 49-51
55-56: Document the reason for hiding dividers.The dividers node is now hidden. Add a comment explaining why this change was made to help future maintenance.
[node name="dividers" type="Node2D" parent="ui"] +# Hidden as part of the remapping UI update visible = falsescenes/BattleDirector/NotePlacementBar.tscn (1)
36-37: Document the reason for swapping note paths.The current and next note paths have been swapped. Add a comment explaining this change to prevent accidental reversions.
+# Swapped for correct visual order in remapping UI _currentNote = NodePath("NoteQueueSprite/NextNote") _nextNote = NodePath("NoteQueueSprite/CurrentNote")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (18)
scenes/BattleDirector/assets/BattleFrame1.pngis excluded by!**/*.pngscenes/BattleDirector/assets/CoolBG.jpgis excluded by!**/*.jpgscenes/BattleDirector/assets/bgupdate.pngis excluded by!**/*.pngscenes/BattleDirector/assets/temp_note_queue.pngis excluded by!**/*.pngscenes/NoteManager/assets/arrow_outline.pngis excluded by!**/*.pngscenes/NoteManager/assets/new_arrow.pngis excluded by!**/*.pngscenes/Remapping/assets/A_Key_Light.pngis excluded by!**/*.pngscenes/Remapping/assets/Arrow_Down_Key_Light.pngis excluded by!**/*.pngscenes/Remapping/assets/Arrow_Left_Key_Light.pngis excluded by!**/*.pngscenes/Remapping/assets/Arrow_Right_Key_Light.pngis excluded by!**/*.pngscenes/Remapping/assets/Arrow_Up_Key_Light.pngis excluded by!**/*.pngscenes/Remapping/assets/D_Key_Light.pngis excluded by!**/*.pngscenes/Remapping/assets/E_Key_Light.pngis excluded by!**/*.pngscenes/Remapping/assets/Q_Key_Light.pngis excluded by!**/*.pngscenes/Remapping/assets/R_Key_Light.pngis excluded by!**/*.pngscenes/Remapping/assets/S_Key_Light.pngis excluded by!**/*.pngscenes/Remapping/assets/T_Key_Light.pngis excluded by!**/*.pngscenes/Remapping/assets/W_Key_Light.pngis excluded by!**/*.png
📒 Files selected for processing (38)
.gitignore(1 hunks)Globals/FunkEngineNameSpace.cs(1 hunks)Globals/StageProducer.cs(1 hunks)project.godot(2 hunks)scenes/BattleDirector/NotePlacementBar.tscn(3 hunks)scenes/BattleDirector/assets/BattleFrame1.png.import(1 hunks)scenes/BattleDirector/assets/bgupdate.png.import(1 hunks)scenes/BattleDirector/scripts/NotePlacementBar.cs(1 hunks)scenes/BattleDirector/scripts/TextParticle.cs(1 hunks)scenes/BattleDirector/test_battle_scene.tscn(2 hunks)scenes/ChartViewport/ChartManager.cs(2 hunks)scenes/ChartViewport/ChartViewport.tscn(1 hunks)scenes/Maps/cartographer.tscn(1 hunks)scenes/Maps/scripts/Cartographer.cs(2 hunks)scenes/NoteManager/assets/arrow_outline.png.import(1 hunks)scenes/NoteManager/assets/new_arrow.png.import(1 hunks)scenes/NoteManager/note.tscn(1 hunks)scenes/NoteManager/note_manager.tscn(1 hunks)scenes/NoteManager/scripts/InputHandler.cs(1 hunks)scenes/NoteManager/scripts/NoteArrow.cs(1 hunks)scenes/Puppets/scripts/PuppetTemplate.cs(1 hunks)scenes/Remapping/ControlSchemes.cs(1 hunks)scenes/Remapping/ControlSettings.cs(1 hunks)scenes/Remapping/Remap.tscn(1 hunks)scenes/Remapping/assets/A_Key_Light.png.import(1 hunks)scenes/Remapping/assets/Arrow_Down_Key_Light.png.import(1 hunks)scenes/Remapping/assets/Arrow_Left_Key_Light.png.import(1 hunks)scenes/Remapping/assets/Arrow_Right_Key_Light.png.import(1 hunks)scenes/Remapping/assets/Arrow_Up_Key_Light.png.import(1 hunks)scenes/Remapping/assets/D_Key_Light.png.import(1 hunks)scenes/Remapping/assets/E_Key_Light.png.import(1 hunks)scenes/Remapping/assets/Q_Key_Light.png.import(1 hunks)scenes/Remapping/assets/R_Key_Light.png.import(1 hunks)scenes/Remapping/assets/S_Key_Light.png.import(1 hunks)scenes/Remapping/assets/T_Key_Light.png.import(1 hunks)scenes/Remapping/assets/W_Key_Light.png.import(1 hunks)scenes/SceneTransitions/TitleScreen.tscn(1 hunks)scenes/SceneTransitions/testTransition.tscn(0 hunks)
💤 Files with no reviewable changes (1)
- scenes/SceneTransitions/testTransition.tscn
✅ Files skipped from review due to trivial changes (17)
- scenes/BattleDirector/scripts/NotePlacementBar.cs
- .gitignore
- scenes/BattleDirector/assets/BattleFrame1.png.import
- scenes/Remapping/assets/D_Key_Light.png.import
- scenes/Remapping/assets/Arrow_Right_Key_Light.png.import
- scenes/Remapping/assets/Arrow_Up_Key_Light.png.import
- scenes/NoteManager/assets/new_arrow.png.import
- scenes/Remapping/assets/W_Key_Light.png.import
- scenes/Remapping/assets/Arrow_Down_Key_Light.png.import
- scenes/Remapping/assets/E_Key_Light.png.import
- scenes/Remapping/assets/Arrow_Left_Key_Light.png.import
- scenes/Remapping/assets/T_Key_Light.png.import
- scenes/Remapping/assets/A_Key_Light.png.import
- scenes/Remapping/assets/R_Key_Light.png.import
- scenes/Remapping/assets/Q_Key_Light.png.import
- scenes/NoteManager/assets/arrow_outline.png.import
- scenes/BattleDirector/assets/bgupdate.png.import
🔇 Additional comments (24)
scenes/ChartViewport/ChartViewport.tscn (5)
10-10: LGTM! Viewport height reduction is properly synchronized.The viewport height reduction is consistently applied to both the container and sub-viewport, maintaining proper coordination between parent and child nodes.
Also applies to: 17-17
22-25: LGTM! Visual adjustments improve coverage and contrast.The expanded horizontal bounds and darker background color enhance the visual presentation while maintaining the original color scheme.
28-28: LGTM! Camera position aligns with viewport changes.The adjusted camera position coordinates well with the expanded viewport bounds.
1-49: LGTM! Scene structure maintains integrity.The scene hierarchy, node references, and resource connections remain well-structured after the visual adjustments.
36-46:Details
❓ Verification inconclusive
Verify the visual impact of removing modulate property.
The removal of the modulate property (previously Color(2,2,2,1)) aligns with the changes in ChartManager.cs to use SelfModulate. Please ensure this change maintains the desired visual appearance of the chart backgrounds.
🏁 Script executed:
#!/bin/bash # Search for related modulate/self_modulate property usage rg -l "(?i)modulate|self_modulate" --type csLength of output: 212
Verify the visual consistency of the chart backgrounds
The modulate property (previously set to Color(2,2,2,1)) has been removed from the ChartBG nodes in
scenes/ChartViewport/ChartViewport.tscn, and the corresponding brightness adjustments now rely onself_modulateinscenes/ChartViewport/ChartManager.cs. Please ensure that these changes render the chart backgrounds with the intended brightness, and verify that no unintended visual regressions occur.
- Confirm that the new
self_modulatelogic in ChartManager.cs correctly replicates the previous brightness effect.- Manually test the chart backgrounds to ensure visual consistency.
scenes/BattleDirector/scripts/TextParticle.cs (1)
10-10: LGTM! Setting ZIndex ensures proper rendering order.The change ensures text particles are rendered above other elements like note arrows (which use
ZIndex = 1).Globals/FunkEngineNameSpace.cs (1)
36-36: LGTM! Stage added for control remapping.The addition of
Controlsto theStagesenum aligns with the PR's objective of implementing a remap menu.scenes/NoteManager/scripts/NoteArrow.cs (1)
16-18: LGTM! Property added for outline customization.The
OutlineSpriteproperty is well-designed:
- Marked with
[Export]for Godot editor integration- Clear naming that indicates its purpose
- Appropriate type for visual customization
scenes/SceneTransitions/TitleScreen.tscn (1)
112-112:Details
✅ Verification successful
Verify ScenePath enumeration value.
Ensure that ScenePath = 4 correctly maps to Stages.Controls in the SceneChange script.
🏁 Script executed:
#!/bin/bash # Search for ScenePath enum definition rg -A 5 "enum.*ScenePath|Stages" --type csLength of output: 4507
ScenePath Enumeration Mapping Verified
The mapping is correct. The value 4 assigned to ScenePath in
scenes/SceneTransitions/TitleScreen.tscnaligns with theStages.Controlscase in the scene change logic (as seen inGlobals/StageProducer.cs). The enumeration order—assuming it is defined as Title (0), Battle (1), Quit (2), Map (3), and Controls (4)—ensures that setting ScenePath to 4 correctly triggers the "Controls" transition.scenes/NoteManager/scripts/InputHandler.cs (1)
64-66: LGTM! Good error handling for missing settings.The code properly handles missing project settings by defaulting to "ARROWS".
scenes/Remapping/assets/S_Key_Light.png.import (1)
1-34: LGTM! Standard texture import configuration.The import settings are appropriate for a UI texture asset.
scenes/ChartViewport/ChartManager.cs (2)
112-113: Good use of SelfModulate for individual node color modulation!Using
SelfModulateinstead ofModulateis more appropriate here as it only affects the individual node without propagating to its children.
124-124: LGTM: Consistent outline coloring with arrow type.Setting the outline color to match the arrow type's color maintains visual consistency.
scenes/Maps/cartographer.tscn (1)
9-13: LGTM: Well-structured scene configuration!The background sprite is properly configured with appropriate modulation, position, and scaling.
scenes/Remapping/ControlSchemes.cs (1)
35-38: Consider revising QWERT key mapping for intuitive controls.The current mapping (E=up, W=down) might be counterintuitive for users as W is typically associated with upward movement in games.
Consider swapping the keys to match common gaming conventions:
- { "arrowUp", "E" }, - { "arrowDown", "W" }, + { "arrowUp", "W" }, + { "arrowDown", "S" },project.godot (2)
15-15: LGTM!The icon path has been updated to use a character asset.
34-36: LGTM!Added new game configuration section with input scheme setting.
scenes/NoteManager/note.tscn (1)
3-14: LGTM!Good improvements to the visual feedback:
- Updated arrow textures
- Added outline sprite for better visibility
scenes/BattleDirector/test_battle_scene.tscn (3)
55-60: LGTM! Background texture setup looks good.The background texture is properly configured with appropriate z-index and dimensions.
6-7:Details
✅ Verification successful
Verify texture resource paths.
The scene references new texture resources
bgupdate.pngandBattleFrame1.png. Ensure these assets are properly committed and their import configurations are correct.
🏁 Script executed:
#!/bin/bash # Check if texture files exist and have import configurations for texture in "bgupdate.png" "BattleFrame1.png"; do echo "Checking $texture..." fd "$texture" "scenes/BattleDirector/assets" fd "$texture.import" "scenes/BattleDirector/assets" doneLength of output: 729
Texture resource paths verified.
After running the checks, bothbgupdate.pngandBattleFrame1.pngalong with their.importconfigurations were confirmed to exist in thescenes/BattleDirector/assetsdirectory. No further action on these resources is required.
38-42:Details
❓ Verification inconclusive
Verify SubViewport positioning impact.
The
SubViewportnode's position has been adjusted. Ensure this change doesn't affect the visibility or interaction with child elements.
🏁 Script executed:
#!/bin/bash # Search for references to SubViewport positioning rg -A 3 "SubViewport.*offset" "scenes/BattleDirector"Length of output: 53
Action Required: Verify the Impact of Updated SubViewport Offsets
The
SubViewportnode now uses the following offsets inscenes/BattleDirector/test_battle_scene.tscn(lines 38–42):[node name="SubViewport" parent="." instance=ExtResource("2_cupb3")] offset_left = 80.0 offset_top = 180.0 offset_right = 560.0 offset_bottom = 360.0No additional references to offset changes were detected in our searches. Please manually verify that these adjustments do not adversely affect the visibility or interaction of child elements. In particular, ensure that the node’s content remains properly aligned and that no functional regressions occur.
- Confirm that the updated offsets do not clip or misalign any child nodes.
- Perform a quick playtest or visual review to ensure the layout behaves as expected.
scenes/NoteManager/note_manager.tscn (1)
4-6:Details
❓ Verification inconclusive
Verify texture resource compatibility.
The arrow textures have been updated. Ensure the new textures maintain visual consistency with the game's art style.
🏁 Script executed:
#!/bin/bash # Check texture dimensions and import settings fd "new_arrow.png|arrow_outline.png" "scenes/NoteManager/assets" -e png -x identify {}Length of output: 168
Manual Verification Needed: Texture Resource Compatibility
The texture resources declared in
scenes/NoteManager/note_manager.tscnappear correct. However, the automated check using theidentifycommand couldn’t run (command not found), which prevents us from programmatically verifying texture dimensions and import settings. Please manually confirm that bothnew_arrow.pngandarrow_outline.png:
- Exist in the
scenes/NoteManager/assetsdirectory.- Conform to the intended dimensions and overall visual style of the game.
scenes/BattleDirector/NotePlacementBar.tscn (2)
16-18: Verify gradient color scheme accessibility.The gradient colors have been modified. Ensure the new color scheme maintains sufficient contrast for accessibility.
#!/usr/bin/env python3 import colorsys def check_contrast(color1, color2): # Convert RGB to relative luminance def get_luminance(r, g, b): return 0.2126 * r + 0.7152 * g + 0.0722 * b # Extract RGB values r1, g1, b1 = color1[:3] r2, g2, b2 = color2[:3] # Calculate luminance l1 = get_luminance(r1, g1, b1) l2 = get_luminance(r2, g2, b2) # Calculate contrast ratio ratio = (max(l1, l2) + 0.05) / (min(l1, l2) + 0.05) return ratio >= 4.5 # WCAG AA standard for normal text # Check adjacent colors in the gradient colors = [ (0, 1, 0, 1), # Green (1, 0, 0.4, 1), # Red (0, 1, 0.95, 1), # Cyan (1, 0, 0, 1) # Red ] for i in range(len(colors)-1): print(f"Checking contrast between colors {i} and {i+1}") print(f"Contrast ratio acceptable: {check_contrast(colors[i], colors[i+1])}")
65-78: LGTM! Note queue sprite setup looks good.The note queue sprite and its children are properly configured with correct positions and rotations.
| var selectedScheme = ControlSchemes.Schemes[scheme]; | ||
| foreach (var arrow in Arrows) | ||
| { | ||
| if (selectedScheme.ContainsKey(arrow.Key)) | ||
| { | ||
| InputEventKey eventKey = new InputEventKey(); | ||
| eventKey.Keycode = (Key)Enum.Parse(typeof(Key), selectedScheme[arrow.Key]); | ||
| InputMap.ActionAddEvent(arrow.Key, eventKey); | ||
| } | ||
| } |
There was a problem hiding this comment.
Add null safety and error handling.
The code should handle cases where:
- The scheme doesn't exist in ControlSchemes.Schemes
- The enum parsing fails for invalid key values
- var selectedScheme = ControlSchemes.Schemes[scheme];
+ if (!ControlSchemes.Schemes.TryGetValue(scheme, out var selectedScheme))
+ {
+ GD.PrintErr($"Control scheme '{scheme}' not found, defaulting to ARROWS");
+ selectedScheme = ControlSchemes.Schemes["ARROWS"];
+ }
foreach (var arrow in Arrows)
{
if (selectedScheme.ContainsKey(arrow.Key))
{
- InputEventKey eventKey = new InputEventKey();
- eventKey.Keycode = (Key)Enum.Parse(typeof(Key), selectedScheme[arrow.Key]);
- InputMap.ActionAddEvent(arrow.Key, eventKey);
+ try
+ {
+ InputEventKey eventKey = new InputEventKey();
+ eventKey.Keycode = (Key)Enum.Parse(typeof(Key), selectedScheme[arrow.Key]);
+ InputMap.ActionAddEvent(arrow.Key, eventKey);
+ }
+ catch (ArgumentException ex)
+ {
+ GD.PrintErr($"Invalid key value '{selectedScheme[arrow.Key]}' for {arrow.Key}: {ex.Message}");
+ }
}📝 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.
| var selectedScheme = ControlSchemes.Schemes[scheme]; | |
| foreach (var arrow in Arrows) | |
| { | |
| if (selectedScheme.ContainsKey(arrow.Key)) | |
| { | |
| InputEventKey eventKey = new InputEventKey(); | |
| eventKey.Keycode = (Key)Enum.Parse(typeof(Key), selectedScheme[arrow.Key]); | |
| InputMap.ActionAddEvent(arrow.Key, eventKey); | |
| } | |
| } | |
| if (!ControlSchemes.Schemes.TryGetValue(scheme, out var selectedScheme)) | |
| { | |
| GD.PrintErr($"Control scheme '{scheme}' not found, defaulting to ARROWS"); | |
| selectedScheme = ControlSchemes.Schemes["ARROWS"]; | |
| } | |
| foreach (var arrow in Arrows) | |
| { | |
| if (selectedScheme.ContainsKey(arrow.Key)) | |
| { | |
| try | |
| { | |
| InputEventKey eventKey = new InputEventKey(); | |
| eventKey.Keycode = (Key)Enum.Parse(typeof(Key), selectedScheme[arrow.Key]); | |
| InputMap.ActionAddEvent(arrow.Key, eventKey); | |
| } | |
| catch (ArgumentException ex) | |
| { | |
| GD.PrintErr($"Invalid key value '{selectedScheme[arrow.Key]}' for {arrow.Key}: {ex.Message}"); | |
| } | |
| } | |
| } |
scenes/Maps/scripts/Cartographer.cs
Outdated
| validButtons[i].FocusNeighborRight = validButtons[(i + 1) % (validButtons.Length)] | ||
| .GetPath(); | ||
| validButtons[(i + 1) % (validButtons.Length)].FocusNeighborLeft = validButtons[i] | ||
| .GetPath(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add null check and empty array validation.
The modulo operation could cause issues if validButtons is empty. Consider adding validation.
- validButtons[i].FocusNeighborRight = validButtons[(i + 1) % (validButtons.Length)]
- .GetPath();
- validButtons[(i + 1) % (validButtons.Length)].FocusNeighborLeft = validButtons[i]
- .GetPath();
+ if (validButtons.Length > 1)
+ {
+ validButtons[i].FocusNeighborRight = validButtons[(i + 1) % (validButtons.Length)]
+ .GetPath();
+ validButtons[(i + 1) % (validButtons.Length)].FocusNeighborLeft = validButtons[i]
+ .GetPath();
+ }Committable suggestion skipped: line range outside the PR's diff.
| "deadzone": 0.5, | ||
| "events": [Object(InputEventKey,"resource_local_to_scene":false,"resource_name":"","device":-1,"window_id":0,"alt_pressed":false,"shift_pressed":false,"ctrl_pressed":false,"meta_pressed":false,"pressed":false,"keycode":0,"physical_keycode":87,"key_label":0,"unicode":119,"location":0,"echo":false,"script":null) | ||
| , Object(InputEventKey,"resource_local_to_scene":false,"resource_name":"","device":-1,"window_id":0,"alt_pressed":false,"shift_pressed":false,"ctrl_pressed":false,"meta_pressed":false,"pressed":false,"keycode":0,"physical_keycode":4194320,"key_label":0,"unicode":0,"location":0,"echo":false,"script":null) | ||
| , Object(InputEventKey,"resource_local_to_scene":false,"resource_name":"","device":-1,"window_id":0,"alt_pressed":false,"shift_pressed":false,"ctrl_pressed":false,"meta_pressed":false,"pressed":false,"keycode":0,"physical_keycode":69,"key_label":0,"unicode":101,"location":0,"echo":false,"script":null) |
There was a problem hiding this comment.
Fix conflicting and unintuitive key mappings.
There are several issues with the key mappings:
arrowDownmaps to 'W' which conflicts witharrowUp- The mapping of 'E' for up, 'Q' for left, and 'R' for right seems unintuitive
Consider this alternative mapping:
-physical_keycode":69,"key_label":0,"unicode":101 # 'E' for up
+physical_keycode":84,"key_label":0,"unicode":116 # 'T' for up
-physical_keycode":87,"key_label":0,"unicode":119 # 'W' for down (conflicts)
+physical_keycode":71,"key_label":0,"unicode":103 # 'G' for down
-physical_keycode":81,"key_label":0,"unicode":113 # 'Q' for left
+physical_keycode":70,"key_label":0,"unicode":102 # 'F' for left
-physical_keycode":82,"key_label":0,"unicode":114 # 'R' for right
+physical_keycode":72,"key_label":0,"unicode":104 # 'H' for rightAlso applies to: 51-51, 58-58, 65-65
| [node name="QWERTButton" type="Button" parent="Panel"] | ||
| layout_mode = 0 | ||
| offset_left = 459.0 | ||
| offset_top = 152.0 | ||
| offset_right = 555.0 | ||
| offset_bottom = 183.0 | ||
| text = "QWER" | ||
|
|
There was a problem hiding this comment.
Fix inconsistent button label.
The button is labeled as "QWER" but the code in ControlSettings.cs uses "QWERT" as the scheme identifier.
Either update the button text to match the scheme or update the scheme identifier in the code:
-text = "QWER"
+text = "QWERT"📝 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.
| [node name="QWERTButton" type="Button" parent="Panel"] | |
| layout_mode = 0 | |
| offset_left = 459.0 | |
| offset_top = 152.0 | |
| offset_right = 555.0 | |
| offset_bottom = 183.0 | |
| text = "QWER" | |
| [node name="QWERTButton" type="Button" parent="Panel"] | |
| layout_mode = 0 | |
| offset_left = 459.0 | |
| offset_top = 152.0 | |
| offset_right = 555.0 | |
| offset_bottom = 183.0 | |
| text = "QWERT" |
| var selectedScheme = ControlSchemes.SpriteMappings[scheme]; | ||
| GD.Print(scheme); | ||
| leftKey.Texture = GD.Load<Texture2D>(selectedScheme["left"]); | ||
| rightKey.Texture = GD.Load<Texture2D>(selectedScheme["right"]); | ||
| upKey.Texture = GD.Load<Texture2D>(selectedScheme["up"]); | ||
| downKey.Texture = GD.Load<Texture2D>(selectedScheme["down"]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for sprite mappings.
The code assumes the scheme exists in SpriteMappings and all required textures can be loaded, which could lead to runtime errors.
Add error handling:
private void ChangeKeySprites(string scheme)
{
+ if (!ControlSchemes.SpriteMappings.ContainsKey(scheme))
+ {
+ GD.PrintErr($"Unknown control scheme: {scheme}");
+ return;
+ }
var selectedScheme = ControlSchemes.SpriteMappings[scheme];
- GD.Print(scheme);
+ try
+ {
leftKey.Texture = GD.Load<Texture2D>(selectedScheme["left"]);
rightKey.Texture = GD.Load<Texture2D>(selectedScheme["right"]);
upKey.Texture = GD.Load<Texture2D>(selectedScheme["up"]);
downKey.Texture = GD.Load<Texture2D>(selectedScheme["down"]);
+ }
+ catch (Exception e)
+ {
+ GD.PrintErr($"Failed to load textures for scheme {scheme}: {e.Message}");
+ }
}📝 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.
| var selectedScheme = ControlSchemes.SpriteMappings[scheme]; | |
| GD.Print(scheme); | |
| leftKey.Texture = GD.Load<Texture2D>(selectedScheme["left"]); | |
| rightKey.Texture = GD.Load<Texture2D>(selectedScheme["right"]); | |
| upKey.Texture = GD.Load<Texture2D>(selectedScheme["up"]); | |
| downKey.Texture = GD.Load<Texture2D>(selectedScheme["down"]); | |
| private void ChangeKeySprites(string scheme) | |
| { | |
| if (!ControlSchemes.SpriteMappings.ContainsKey(scheme)) | |
| { | |
| GD.PrintErr($"Unknown control scheme: {scheme}"); | |
| return; | |
| } | |
| var selectedScheme = ControlSchemes.SpriteMappings[scheme]; | |
| try | |
| { | |
| leftKey.Texture = GD.Load<Texture2D>(selectedScheme["left"]); | |
| rightKey.Texture = GD.Load<Texture2D>(selectedScheme["right"]); | |
| upKey.Texture = GD.Load<Texture2D>(selectedScheme["up"]); | |
| downKey.Texture = GD.Load<Texture2D>(selectedScheme["down"]); | |
| } | |
| catch (Exception e) | |
| { | |
| GD.PrintErr($"Failed to load textures for scheme {scheme}: {e.Message}"); | |
| } | |
| } |
added a seperate scene were the player can choose from the three schemes, defaults to arrows. slight tweak to inputhandler to refresh the controls. button to control scene was addded. will try to use sprites/icons to show which keys will correspend to each arrow in next push
Added icons to represent which key correspond to each direction, changes as the user changes the scheme. Fixed a typo that caused it to always default to WASD. Sprite paths are added to the ControlSchemes as a dictionary set. Icons are from this free pack: https://thoseawesomeguys.com/prompts/
removed dupe code from rebase
Summary by CodeRabbit
New Features
Style