Sprint 4 #105
Sprint 4 #105
Conversation
still need to update graphics to support chinese. the only graphic I think we need to update would be the "RELICS" tab during battle
Options menu can be overlayed on other screens Remap can be access from options Input should still be consistently navigable with arrow keys
Added WASD for menu navigation Added 1234 for rhythm game
Reworked SaveSystem to save config file
Added default settings for error prevention
Added ids to notes and relics for persistence Added Stages.Load enum Changed CurRoom to int
After looking through the code, it seemed to all be fine. Instead I decided to cut off the "trailing audio". I also shortened the midi file to cut off at the point when the audio began to trail.
Add contrast filter
Used audacity to add fade-in and fade-out to all songs, and trimmed the dead audio from all songs. Also updated README to credit every song.
No more desync (hopefully)
Added new button for controller in settings, will also swap to controller controls if gamepad buttons are pressed in battle if they were not selected. currently only for d-pad directions, added functionality for menu navigation, pausing game(start button), opening inventory(select button), confirming (A button). Testing with 8BitDo Ultimate Cotroller
Left stick is now usable during battle, can be used interchangably with the d-pad. helper function added in input handler to translate joystick input
Controller input is dependent on a controller actually being connected Controls can update mid-battle Simplified control changing for better maintainability
The more a song's length is over where a natural beat should end, the more looped notes would jump into place at playback = 0. Instead looped notes are a set offset instead of beats+bpl
Controller support
Warnings are pushed when save can't be deserialized or found. Losing battle deletes save.
Note queue sprites are no longer the wrong way around.
The note being placed uses it's effect at the "okay" timing, since this is the middle option for note damage this should be good enough for the player to at least feel the effect instantly.
Currently multiplies the base level into the effect, but can always change this to addition later. This is to help later introduce relics that effect specific / overall note damage.
- enemies have been given individual HP stats - player notes no longer do their effects when missed
Requires midi file contains correct time signature and bpm
Simple animations on damage and heal Small fixes to ordering of control flow for damage and heal Standardized tweens to be on node not tree
Small particle effects spawn on note queue and placed note Fixed some spaghetti with initial noteArrow position
Cleanup code and last features
Clean up comments and TODO's Fix ChartLooped getting called multiple times due to modulo of int instead of float
WalkthroughThis update introduces numerous modifications and additions across the project. Key changes include updates to ignore rules in the version control configuration, several UID updates for audio import files, and significant updates to core game classes by altering constructors, adding new properties, and refining methods. The project’s configuration files have been updated for audio, input, and localization support. Additionally, the save system and scene files were restructured. New UI scenes and scripts for options and control remapping were introduced, and various shader, particle, and battle logic adjustments refine gameplay and visual feedback. Changes
Sequence Diagram(s)sequenceDiagram
participant Game as Game System
participant SP as StageProducer
participant Save as SaveSystem
Game->>SP: TransitionStage(Load)
SP->>Save: LoadGame()
Note right of Save: Read and deserialize save file data
Save-->>SP: Return SaveFile object
SP->>SP: Update RNG state and set CurRoom (using room index)
SP->>Game: Transition to the correct scene
sequenceDiagram
participant User as Player
participant TS as TitleScreen
participant OM as OptionsMenu
participant Save as SaveSystem
User->>TS: Press Options Button
TS->>OM: Call OpenOptions()
OM->>OM: Initialize UI components and focus controls
User->>OM: Interact with sliders/buttons (volume, contrast, etc.)
OM->>Save: Update configuration (UpdateConfig)
OM->>TS: Close Options and return focus to TitleScreen
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 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: 6
🔭 Outside diff range comments (1)
scenes/UI/assets/combobar.png.import (1)
35-35:⚠️ Potential issue[ACTION] Remove Extraneous Content
Line 35 appears to contain a stray value (
35) that does not adhere to the expected key–value format of this configuration file. This extraneous line could lead to parsing issues; please remove it if it is not intentional.
🧹 Nitpick comments (46)
Globals/ContrastFilter/ContrastFilter.gdshader (2)
8-8: Fix typo in shader codeThere's an unnecessary space before
vec3.-COLOR.rgb =vec3(0.6 * screen.r + 0.6 * screen.g + 0.2 * screen.b); +COLOR.rgb = vec3(0.6 * screen.r + 0.6 * screen.g + 0.2 * screen.b);
3-9: Consider adding configurability to shaderThe current implementation uses hard-coded values (0.6, 0.6, 0.2) for the color weights. For better flexibility, consider adding uniform variables to adjust these weights at runtime.
uniform sampler2D SCREEN_TEXTURE : hint_screen_texture, filter_linear_mipmap; +uniform float red_weight : hint_range(0.0, 1.0) = 0.6; +uniform float green_weight : hint_range(0.0, 1.0) = 0.6; +uniform float blue_weight : hint_range(0.0, 1.0) = 0.2; void fragment() { vec4 screen = texture(SCREEN_TEXTURE, SCREEN_UV); - COLOR.rgb =vec3(0.6 * screen.r + 0.6 * screen.g + 0.2 * screen.b); + COLOR.rgb = vec3(red_weight * screen.r + green_weight * screen.g + blue_weight * screen.b); }scenes/Puppets/Enemies/BossBlood/P_BossBlood.cs (1)
10-11: Boss health initialization looks good, consider future scalability.Setting initial health values for the boss is appropriate, but hardcoding health values might make balancing more difficult as the game evolves.
Consider implementing a more scalable approach by:
- Defining health values in a configuration file
- Creating a formula based on game progression/difficulty
- Adding a difficulty multiplier for health values
- _currentHealth = 100; - _maxHealth = 100; + // Load from configuration or calculate based on game state + _maxHealth = EnemyConfiguration.GetBossHealth("BossBlood", GameState.CurrentDifficulty); + _currentHealth = _maxHealth;scenes/Puppets/Enemies/Parasifly/P_Parasifly.cs (1)
8-9: Consider alternative ways to set health values.While setting the health values works as implemented, consider:
- Defining these as class constants for better maintainability
- Using property initialization at the class level
- Implementing these values in a data-driven way (e.g., from a configuration file)
This would make balancing easier and separate data from behavior.
public partial class P_Parasifly : EnemyPuppet { + private const int DEFAULT_HEALTH = 50; + public override void _Ready() { - _currentHealth = 50; - _maxHealth = 50; + _currentHealth = DEFAULT_HEALTH; + _maxHealth = DEFAULT_HEALTH; base._Ready(); var enemTween = CreateTween();scenes/UI/scripts/DisplayButton.cs (1)
22-24: Consider the implications of triggering on focusConnecting
FocusEnteredto theSelectedmethod will cause buttons to activate as soon as they receive focus (during keyboard/controller navigation), which might lead to unintended actions being triggered.Consider implementing this with proper button group management to prevent multiple buttons from activating simultaneously during navigation. This would align with your TODO comment on line 26.
scenes/SceneTransitions/scripts/TitleScreen.cs (2)
28-31: Consider extracting the hardcoded scene pathThe hardcoded path to the options menu scene could be moved to a constant or exported variable to improve maintainability.
+ [Export] + private string OptionsMenuPath = "res://scenes/Options/OptionsMenu.tscn"; + private void OpenOptions() { - OptionsMenu optionsMenu = GD.Load<PackedScene>("res://scenes/Options/OptionsMenu.tscn") + OptionsMenu optionsMenu = GD.Load<PackedScene>(OptionsMenuPath) .Instantiate<OptionsMenu>();
26-32: Prevent multiple instances of the options menuThere's no check to prevent opening multiple options menus if the button is pressed repeatedly. Consider adding a guard clause.
private void OpenOptions() { + // Prevent opening multiple options menus + if (HasNode("OptionsMenu")) + return; + OptionsMenu optionsMenu = GD.Load<PackedScene>("res://scenes/Options/OptionsMenu.tscn") .Instantiate<OptionsMenu>(); + optionsMenu.Name = "OptionsMenu"; AddChild(optionsMenu); optionsMenu.OpenMenu(this); }scenes/Options/scripts/LanguageSelection.cs (3)
13-24: Consider using constants for language codesThe hardcoded language codes ("en", "cn") appear in multiple places in the class. Consider defining them as constants to avoid duplication and make future language additions easier.
public partial class LanguageSelection : OptionButton { + private const string ENGLISH = "en"; + private const string CHINESE = "cn"; // Rest of the code... public void OnLanguageSelected(int index) { switch (index) { case 0: - ChangeLanguage("en"); + ChangeLanguage(ENGLISH); break; case 1: - ChangeLanguage("cn"); + ChangeLanguage(CHINESE); break; } }
26-30: Add error handling for configuration updatesThe
SaveSystem.UpdateConfigcall doesn't have any error handling. Consider adding try-catch to handle potential failures when saving configuration.private void ChangeLanguage(string language) { TranslationServer.SetLocale(language); - SaveSystem.UpdateConfig(SaveSystem.ConfigSettings.LanguageKey, language); + try + { + SaveSystem.UpdateConfig(SaveSystem.ConfigSettings.LanguageKey, language); + } + catch (Exception e) + { + GD.PrintErr($"Failed to save language setting: {e.Message}"); + } }
32-43: Consider a more extensible approach for language selectionThe current implementation uses a switch statement that would need modification for each new language. Consider a more extensible approach for future language additions.
private void PresetDropdown(string language) { - switch (language) - { - case "cn": - Selected = 1; - break; - default: - Selected = 0; - break; - } + // This assumes the order of items in the dropdown matches the order they were added + int index = language == CHINESE ? 1 : 0; + Selected = index; }scenes/UI/scripts/Inventory.cs (2)
70-75: Internationalization implementation looks good, but could be more robustThe implementation correctly uses the
Trfunction with dynamically constructed keys, but the string manipulations might be error-prone.Consider creating a helper method to generate consistent translation keys:
private void DoDescription(DisplayButton dispButton) { - string itemName = dispButton.DisplayName.ToUpper(); - itemName = itemName.Replace(" ", ""); - Description.Text = - Tr(TabNames[Tabs.CurrentTab] + "_" + itemName + "_NAME") - + ": " - + Tr(TabNames[Tabs.CurrentTab] + "_" + itemName + "_TOOLTIP"); + string itemName = NormalizeNameForTranslation(dispButton.DisplayName); + string prefix = TabNames[Tabs.CurrentTab]; + Description.Text = + Tr($"{prefix}_{itemName}_NAME") + ": " + Tr($"{prefix}_{itemName}_TOOLTIP"); } +private string NormalizeNameForTranslation(string name) +{ + return name.ToUpper().Replace(" ", ""); +}
68-76: Consider validating tab indexThe code directly accesses
TabNames[Tabs.CurrentTab]without validating that the index is within bounds. While it's likely that the tab container and array will match, adding a safeguard would improve robustness.private void DoDescription(DisplayButton dispButton) { + if (Tabs.CurrentTab < 0 || Tabs.CurrentTab >= TabNames.Length) + { + GD.PrintErr($"Invalid tab index: {Tabs.CurrentTab}"); + return; + } string itemName = dispButton.DisplayName.ToUpper(); itemName = itemName.Replace(" ", ""); Description.Text = Tr(TabNames[Tabs.CurrentTab] + "_" + itemName + "_NAME") + ": " + Tr(TabNames[Tabs.CurrentTab] + "_" + itemName + "_TOOLTIP"); }scenes/Maps/scripts/Cartographer.cs (1)
22-22: Consider adding error handling for save operation.Adding a save operation during the map's
_Ready()method is a good way to ensure game progress is preserved, but this call lacks error handling. If the save operation fails, it could silently fail without the player knowing.- SaveSystem.SaveGame(); + try + { + SaveSystem.SaveGame(); + } + catch (Exception e) + { + GD.PrintErr($"Failed to save game: {e.Message}"); + // Consider showing a non-intrusive notification to the player + }scenes/UI/scripts/RewardSelect.cs (1)
24-24: Consider implementing typed functions as suggested by TODO commentThe TODO comment suggests exploring typed functions, which could improve type safety and code clarity. Consider implementing this in a future update to make the code more maintainable.
scenes/NoteManager/scripts/InputHandler.cs (1)
65-76: Implementation looks good, but comments were removed.While the implementation of particle amounts based on timing remains unchanged, the comments that previously described what each amount represented have been removed. This reduces code clarity for future developers.
Consider adding back descriptive comments to explain the significance of each particle amount value.
scenes/NoteManager/scripts/NoteArrow.cs (1)
46-53: Well-structured extraction of position calculation logic.Extracting this logic into a dedicated method improves maintainability and reusability. Consider adding XML documentation to explain what this method calculates and its return value.
+ /// <summary> + /// Calculates the horizontal position of the note based on the current time and chart parameters. + /// </summary> + /// <returns>The calculated X position for the note.</returns> private float GetNewPos() { return (float)( (-TimeKeeper.CurrentTime / TimeKeeper.LoopLength * TimeKeeper.ChartLength) % TimeKeeper.ChartLength / 2 ) + Bounds; }scenes/Options/OptionsMenu.tscn (1)
96-101: Consider adjusting volume slider range for better user control.The volume slider's minimum value is set to 50.0, which might not provide enough range for users who prefer lower volumes. Consider adjusting the range to start from 0 or a lower value to provide more flexibility.
-min_value = 50.0 +min_value = 0.0Classes/Notes/Note.cs (1)
71-74: Consider using a property instead of a getter method.While adding a getter method for
_baseValis a good practice for encapsulation, in C# it's more idiomatic to use a property with a private setter.-private int _baseVal; +public int BaseVal { get; private set; } public Note( int id, string name, string tooltip, Texture2D texture = null, PuppetTemplate owner = null, int baseVal = 1, Action<BattleDirector, Note, Timing> noteEffect = null, float costModifier = 1.0f ) { Id = id; Name = name; Owner = owner; NoteEffect = noteEffect ?? ( (BD, source, Timing) => { - BD.GetTarget(this).TakeDamage((int)Timing * source._baseVal); + BD.GetTarget(this).TakeDamage((int)Timing * source.BaseVal); } ); - _baseVal = baseVal; + BaseVal = baseVal; Texture = texture; Tooltip = tooltip; CostModifier = costModifier; } -public int GetBaseVal() -{ - return _baseVal; -}scenes/UI/scripts/HowToPlay.cs (1)
18-18: Unnecessary empty _Process method.The
_Processmethod is defined but left empty. In Godot, you don't need to implement this method unless you need to perform actions each frame.- public override void _Process(double delta) { }Classes/MidiMaestro/MidiMaestro.cs (1)
13-14: Consider making the static properties readonly.Static mutable state can lead to unexpected behavior in complex applications. Making these properties
readonlywould ensure they can't be modified after initialization.- public static TempoMap TempoMap; - public static TimeSignature TimeSignature; + public static readonly TempoMap TempoMap; + public static readonly TimeSignature TimeSignature;scenes/BattleDirector/scripts/NotePlacementBar.cs (2)
4-4: Check usage of new namespace import.
It appearsusing FunkEngine;was introduced. Confirm that classes or methods fromFunkEngineare indeed used in this file to avoid redundant references.
136-154: Refactor to reduce note retrieval complexity.
The newly introducedSprite2D selectedNotevariable and subsequent branching for_currentNotevs._nextNoteis clearer. However, consider extracting note retrieval and particle spawning logic into a small helper method to avoid duplication and reduce complexity in this block.scenes/Puppets/scripts/PuppetTemplate.cs (2)
46-49: Frame-based shake invocation.
InvokingProcessShakefrom_Process(double delta)is a good approach as it ties the animation directly to the frame loop. Ensure performance remains acceptable when more puppets or additional processing is involved.
64-80: Evaluate performance inProcessShake.
The limiting approach (_limiter = (_limiter + 1) % 3) accomplishes a partial decoupling from high frame rates. Keep an eye on jitter or abrupt changes at lower frame rates.Globals/Scribe.cs (6)
27-39: PlayerBase note logic.
Introducing theTiming.Missearly return is sensible. Confirm that the code accounts for partial hits (e.g., if there's a “Bad” or “OK” timing with smaller damage) which is currently implied by(int)timing.
55-67: PlayerHeal note logic.
Returning early onTiming.Missis correct, and healing is computed from(int)timing * note.GetBaseVal(). Ensure the healing limit cannot exceed player max HP if that’s a gameplay requirement.
69-82: PlayerVampire synergy.
Simultaneously healing the player and damaging the enemy is conceptually strong. If vampiric synergy with other relics or states arises, consider hooking into an event for synergy-based modifications (e.g., “+20% vamp healing”).
103-119: “Breakfast” relic for max HP.
Adding a relic that increases max HP is straightforward. Confirm potential integer overflow if the player repeatedly picks up max HP relics.
121-136: “Good Vibes” relic synergy.
Healing the player whenever they place a note can stack quickly with other notes (e.g.,PlayerHeal). Monitor for potential design oversights if players chain these to trivialize battles.
138-154: “Auroboros” relic incremental combo.
Incrementing combo bonus each loop is fun but can lead to large damage multipliers. Consider capping or scaling for longer loops.Globals/StageProducer.cs (5)
54-54: Using first room’s Idx asCurRoom.
This logic is fine for a default start. If future expansions allow multiple start rooms, ensure this is updated.
86-90: Concise map generation with consistent RNG.
Using the seed plus shift forGlobalRng.Stateis interesting. Confirm the operator precedence (<< 5 / 2) matches your intended logic. Parentheses may clarify.- GlobalRng.State = GlobalRng.Seed << 5 / 2; + GlobalRng.State = (GlobalRng.Seed << 5) / 2;
97-100: Changing cur room by int.
Replacing the old approach that took aMapGrid.Roomwith a direct integer is simpler but less type-safe. If referencing rooms with IDs is common, keep an eye out for invalid IDs.
135-139: AddingStages.Loadlogic.
Falling back toStartGame()ifLoadGame()fails is user-friendly. Confirm a user-facing error message in case of load failures.
144-144: Error handling improvement.
Pushing an error to Godot’s console is beneficial to track unexpected stages. If environment allows, consider logging beyond console for better debugging.scenes/Options/OptionsMenu.cs (3)
27-27: Minimum volume constant.
MinVolumeVal = 50fis a reasonable baseline if you're offsetting volumes by80dB. Ensure clarity about decibel ranges in the code docs or comments.
96-103:ChangeVolumeusesAudioServerwithvalue - 80.
The offset of 80 dB is consistent with your config approach. The additional check forMinVolumeValto toggle mute is a nice detail, but watch out for edge floating comparisons (Math.Abs(value - MinVolumeVal) < .1).
105-109: High-contrast mode toggle.
TogglingStageProducer.ContrastFilter.Visibleis straightforward. If usage extends beyond visuals (e.g., color-coded text or shapes), consider a more robust approach for accessible design.SaveData/SaveSystem.cs (4)
38-43:SaveConfig: Immediate writes toUserConfigPath.
Ensuring the config is saved upon updates is convenient but consider if frequent writes are necessary or if batching might improve performance.
45-68:UpdateConfigmethod.
Switch-based approach for each setting is explicit, although it means repeated code. If many config settings grow, consider a more generic approach or a dictionary-based mapping for improved scalability.
79-100:VerifyConfiguses a "sus" array check.
This approach tries to catch suspicious lines in the config. It's somewhat hacky; a more robust solution might parse the config or store a known header.
215-218:ClearSavemethod.
Deleting the save file withDirAccess.RemoveAbsolute(UserSavePath)is direct. Confirm the user is informed or asked for confirmation in the UI before clearing.scenes/BattleDirector/scripts/BattleDirector.cs (2)
46-53: Consider decouplingNotePlacementBarfromBattleDirector.
Passingthis(the entireBattleDirectorinstance) toNotePlacementBar.PlaceNote(...)can introduce tight coupling if only a subset of data is needed. Whenever possible, pass only the required parameters or use dependency injection to reduce direct references.
306-306: Remove or guard debug code for production.
CallingEnemy.TakeDamage(1000);is a debug kill. Consider wrapping this in a debug flag or removing for release builds to prevent accidental usage in production.scenes/Remapping/ControlSettings.cs (1)
109-114: Consider localizing controller label.
Currently, you set the label text to “Controller Selected” without usingTr(). If you wish to fully localize, you can wrap that inTr("CONTROLS_TITLE_TYPE_CONTROLLER").project.godot (1)
18-20: Set or remove empty default bus layout.
buses/default_bus_layout=""is currently unused. Either specify a bus layout resource or remove the empty key to avoid confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (10)
Audio/Song1.oggis excluded by!**/*.oggAudio/Song2.oggis excluded by!**/*.oggAudio/Song3.oggis excluded by!**/*.oggGlobals/Translations/translations.csvis excluded by!**/*.csvscenes/BattleDirector/assets/bgupdate.pngis excluded by!**/*.pngscenes/Remapping/assets/Positional_Prompts_Down.pngis excluded by!**/*.pngscenes/Remapping/assets/Positional_Prompts_Left.pngis excluded by!**/*.pngscenes/Remapping/assets/Positional_Prompts_Right.pngis excluded by!**/*.pngscenes/Remapping/assets/Positional_Prompts_Up.pngis excluded by!**/*.pngscenes/UI/assets/combobar.pngis excluded by!**/*.png
📒 Files selected for processing (67)
.gitignore(1 hunks)Audio/Song1.ogg.import(1 hunks)Audio/Song2.ogg.import(1 hunks)Audio/Song3.ogg.import(1 hunks)Classes/MidiMaestro/MidiMaestro.cs(3 hunks)Classes/Notes/Note.cs(3 hunks)Classes/Relics/RelicTemplate.cs(1 hunks)Funk Engine.csproj(1 hunks)Globals/BgAudioPlayer/BGAudioPlayer.tscn(1 hunks)Globals/BgAudioPlayer/BgAudioPlayer.cs(1 hunks)Globals/ContrastFilter/ContrastFilter.gdshader(1 hunks)Globals/ContrastFilter/ContrastFilter.tscn(1 hunks)Globals/FunkEngineNameSpace.cs(2 hunks)Globals/Scribe.cs(5 hunks)Globals/StageProducer.cs(3 hunks)Globals/Translations/translations.csv.import(1 hunks)README.md(1 hunks)SaveData/SaveData.json(0 hunks)SaveData/SaveSystem.cs(1 hunks)project.godot(1 hunks)scenes/BattleDirector/NotePlacementBar.tscn(1 hunks)scenes/BattleDirector/notePoofParticles.tscn(1 hunks)scenes/BattleDirector/scripts/BattleDirector.cs(8 hunks)scenes/BattleDirector/scripts/Conductor.cs(3 hunks)scenes/BattleDirector/scripts/NotePlacementBar.cs(5 hunks)scenes/BattleDirector/scripts/NoteQueueParticlesFactory.cs(1 hunks)scenes/BattleDirector/scripts/TextParticle.cs(1 hunks)scenes/BattleDirector/test_battle_scene.tscn(1 hunks)scenes/ChartViewport/scripts/ChartManager.cs(5 hunks)scenes/ChartViewport/scripts/HitParticles.cs(1 hunks)scenes/ChartViewport/scripts/Loopable.cs(0 hunks)scenes/ChestScene/ChestScene.cs(1 hunks)scenes/ChestScene/ChestScene.tscn(2 hunks)scenes/Maps/scripts/Cartographer.cs(4 hunks)scenes/NoteManager/scripts/InputHandler.cs(2 hunks)scenes/NoteManager/scripts/NoteArrow.cs(2 hunks)scenes/Options/OptionsMenu.cs(1 hunks)scenes/Options/OptionsMenu.tscn(1 hunks)scenes/Options/scripts/LanguageSelection.cs(1 hunks)scenes/Puppets/Enemies/BossBlood/P_BossBlood.cs(1 hunks)scenes/Puppets/Enemies/EnemyPuppet.cs(0 hunks)scenes/Puppets/Enemies/Parasifly/P_Parasifly.cs(1 hunks)scenes/Puppets/Enemies/TheGWS/P_TheGWS.cs(1 hunks)scenes/Puppets/scripts/PuppetTemplate.cs(3 hunks)scenes/Remapping/ControlSchemes.cs(1 hunks)scenes/Remapping/ControlSettings.cs(2 hunks)scenes/Remapping/Remap.tscn(3 hunks)scenes/Remapping/assets/Positional_Prompts_Down.png.import(1 hunks)scenes/Remapping/assets/Positional_Prompts_Left.png.import(1 hunks)scenes/Remapping/assets/Positional_Prompts_Right.png.import(1 hunks)scenes/Remapping/assets/Positional_Prompts_Up.png.import(1 hunks)scenes/SceneTransitions/TitleScreen.tscn(7 hunks)scenes/SceneTransitions/assets/TitleFont.gdshader(1 hunks)scenes/SceneTransitions/assets/font.TTF.import(1 hunks)scenes/SceneTransitions/assets/transparentStars.gdshader(1 hunks)scenes/SceneTransitions/scripts/TitleScreen.cs(1 hunks)scenes/UI/EndScreen.tscn(4 hunks)scenes/UI/HowToPlay.tscn(1 hunks)scenes/UI/Pause.tscn(4 hunks)scenes/UI/RewardSelectionUI.tscn(2 hunks)scenes/UI/assets/combobar.png.import(1 hunks)scenes/UI/inventory.tscn(3 hunks)scenes/UI/scripts/DisplayButton.cs(1 hunks)scenes/UI/scripts/HowToPlay.cs(1 hunks)scenes/UI/scripts/Inventory.cs(3 hunks)scenes/UI/scripts/PauseMenu.cs(2 hunks)scenes/UI/scripts/RewardSelect.cs(2 hunks)
💤 Files with no reviewable changes (3)
- scenes/Puppets/Enemies/EnemyPuppet.cs
- scenes/ChartViewport/scripts/Loopable.cs
- SaveData/SaveData.json
🔇 Additional comments (173)
scenes/SceneTransitions/assets/transparentStars.gdshader (2)
16-16: Minor Formatting Fix in Variable AssignmentThe change at line 16 removes an unnecessary space in the assignment
alpha = star_intensity;. This stylistic update improves consistency without affecting functionality.
18-18: Cleanup of Blank LineThe removal of an extraneous blank line after the
ifblock (as indicated on line 18) cleans up the code formatting without any impact on behavior.scenes/UI/assets/combobar.png.import (3)
1-10: [INFO] Remap Block ReviewThe
[remap]section correctly specifies the texture importer settings with clear fields such asimporter,type,uid,path, and a well-definedmetadatablock. Ensure that theuidvalue is unique and conforms to your asset management guidelines.
11-15: [INFO] Dependencies Block ReviewThe
[deps]section appropriately sets thesource_fileand maps the destination file in thedest_filesarray. Verify that these paths match the actual asset locations in your project.
16-34: [INFO] Parameters Block ReviewThe
[params]section is comprehensively populated with the configuration parameters for compression, mipmap generation, roughness settings, and processing flags. The values (e.g.,compress/lossy_quality=0.7,mipmaps/limit=-1) look standard, but please double-check these against your project's quality and performance requirements.Globals/ContrastFilter/ContrastFilter.tscn (1)
1-20: Scene setup looks good for a fullscreen filter overlayThe CanvasLayer with a high layer value (128) and ColorRect with high z-index (4096) ensure this filter renders above other UI elements. The anchors and grow properties correctly make it cover the entire screen, and setting mouse_filter to 2 properly ensures that mouse events pass through to elements beneath.
Globals/BgAudioPlayer/BgAudioPlayer.cs (1)
10-10: Good improvement for continuous music playback!Setting
ProcessModetoProcessModeEnum.Alwaysensures that the background music continues playing even when the game is paused or during scene transitions. This is particularly important for maintaining a seamless audio experience.scenes/Puppets/Enemies/TheGWS/P_TheGWS.cs (1)
8-9:Details
✅ Verification successful
Health initialization looks good, but consider sequence.
The health values are appropriately set for this enemy type. However, note that these values are set before calling
base._Ready(). Ensure this is intentional, as the base class implementation might override these values if it also sets health properties.
🏁 Script executed:
#!/bin/bash # Check the base EnemyPuppet class to see if it also sets health values rg "_currentHealth|_maxHealth" --type cs -A 3 -B 3Length of output: 6453
Health initialization order verified
In
P_TheGWS.cs, setting_currentHealthand_maxHealthbefore callingbase._Ready()is consistent with the implementation in other enemy classes (e.g.,P_Parasifly.csandP_BossBlood.cs). The base_Ready()(inPuppetTemplate.cs) uses these values to initialize the health bar without overriding them, so this ordering appears to be intentional.
- Health values are established upfront to ensure the base class uses the enemy-specific values.
- The pattern across enemy implementations confirms this sequence is intentional and correct.
.gitignore (1)
14-15: Appropriate version control exclusions for project.Good additions to the
.gitignorefile:
export_presets.cfg- Contains environment-specific export settings*.translation- Generated translation files that shouldn't be trackedThis follows best practices for Godot projects, especially ones with localization support.
Globals/Translations/translations.csv.import (1)
1-18: Well-configured translation import settings.The import configuration properly sets up:
- Translation files for English and Chinese
- Appropriate source and destination paths
- Compression enabled for optimization
- Standard delimiter setting
This is a solid foundation for the game's localization system.
Globals/BgAudioPlayer/BGAudioPlayer.tscn (1)
3-3: Script path updated to match directory structureThe script path has been updated to reflect the new directory structure, which keeps the script file in the same directory as its scene file. This improves organization and follows better project structure practices.
scenes/ChartViewport/scripts/ChartManager.cs (6)
31-31: New TrueBeatsPerLoop property for improved timing precisionAdding this double property will provide more precise timing calculations compared to the integer-based BeatsPerLoop.
48-49: Improved beat calculation with preserved precisionThe calculation now preserves floating-point precision with TrueBeatsPerLoop until it's cast to an integer for BeatsPerLoop. This will result in more accurate rhythm gameplay and visual representation.
63-63: Simplified tween creationUsing
CreateTween()directly instead ofGetTree().CreateTween()is more concise and leverages the method available in the Node class.
98-100: Enhanced loop visualization with dummy arrowCreating a dummy arrow with an offset provides visual continuity when the chart loops, improving the player experience.
109-109: Added loopOffset parameter to support loop visualizationThe new parameter allows for proper positioning of notes in looped sections of the chart.
113-115: Improved note positioning calculationUsing TrueBeatsPerLoop for the bounds calculation provides more precise note positioning, while the loopOffset parameter ensures proper visual alignment in looped sections.
Audio/Song2.ogg.import (1)
5-5: Updated resource UIDThe UID for this audio resource has been updated, which is likely a result of resource reimporting or project regeneration. This change is normally automatically handled by the Godot engine.
Audio/Song1.ogg.import (1)
5-5: Updated resource UIDThe UID for this audio resource has been updated, consistent with similar changes in other audio files. This is typically an automatic change handled by the Godot engine when resources are reimported.
Audio/Song3.ogg.import (1)
5-5:Details
✅ Verification successful
UID update looks good, ensure all references are updated.
The UID change for this audio file is likely due to reimporting or project migration. This change itself is fine.
🏁 Script executed:
#!/bin/bash # Check for any hardcoded references to the old UID that might need updating rg "uid://ceyw5mjkem2pi" --type-not=importLength of output: 73
UID Update Verification – Please Verify Manually
The UID update in
Audio/Song3.ogg.importappears correct. However, the initial search produced an error due to an unrecognized file type flag. The revised command above excludes.importfiles properly. Please run it manually and confirm that no other references touid://ceyw5mjkem2piexist in the codebase.
🏁 Script executed:
#!/bin/bash # Re-run the search to check for any hardcoded references to the old UID, # while excluding files with the ".import" extension. rg "uid://ceyw5mjkem2pi" --glob '!*.import'Length of output: 165
UID update verified – All references checked
The updated UID in
Audio/Song3.ogg.import(now set touid://d4nmixdl8xoic) appears correct, and a search (excluding.importfiles) for any lingering references to the old UID (uid://ceyw5mjkem2pi) returned no results. Please ensure this search covers all relevant parts of the codebase.scenes/ChestScene/ChestScene.cs (1)
37-37: Updated room reference for consistency.The change from passing the room object to passing the room index (
BattleRoom.Idx) aligns with the broader architectural changes in the StageProducer system.scenes/BattleDirector/test_battle_scene.tscn (1)
75-75: Good localization implementation!The text has been changed from a hardcoded English string to a localization key "BATTLE_ROOM_BEGIN_BUTTON", which is consistent with the localization approach being implemented across the project. This will make it easier to support multiple languages.
scenes/UI/RewardSelectionUI.tscn (3)
41-41: Good localization implementation!Changed from hardcoded "Rewards!" text to the localization key "CHEST_ROOM_REWARDS", which follows a consistent naming pattern and supports multi-language functionality.
97-97: Good localization implementation!Changed from hardcoded "Skip" text to the localization key "CHEST_ROOM_SKIP", maintaining consistency with the project's localization approach.
102-102: Good localization implementation!Changed from hardcoded "Accept" text to the localization key "CHEST_ROOM_ACCEPT", completing the localization of this UI scene.
scenes/BattleDirector/scripts/TextParticle.cs (1)
9-9: Code simplification improvement.Good refactoring from
GetTree().CreateTween()to simplyCreateTween(). This uses the method inherited from the Node class, resulting in cleaner and more concise code while maintaining the same functionality.scenes/UI/scripts/DisplayButton.cs (1)
26-30: Ensure this behavior is intentionalThis implementation makes buttons activate when they receive focus, not just when clicked. This is a significant change in UI behavior that could cause confusion if users aren't expecting it.
If this is intentional, consider adding a parent class comment explaining this non-standard button behavior, as it deviates from typical button interactions where users expect to click/press to activate.
scenes/UI/EndScreen.tscn (2)
38-38: LGTM! Proper move toward localizationReplacing hardcoded text with localization keys is a good practice for supporting multiple languages.
Ensure the localization system (likely using Godot's Translation resource) is properly configured to include these new keys.
56-56: Consistent localization key naming conventionUsing consistent UPPERCASE_WITH_UNDERSCORES naming for localization keys follows good internationalization practices.
Also applies to: 70-70, 84-84
scenes/SceneTransitions/scripts/TitleScreen.cs (1)
18-24: LGTM! Good focus managementSetting focus to the Options button when no other GUI element has focus is a good practice for keyboard/controller navigation.
scenes/ChartViewport/scripts/HitParticles.cs (2)
14-14: LGTM! Removed potentially misleading commentThe removed comment might have been misleading as it mentioned 0.5 seconds while the actual code uses 0.25f.
The code is now cleaner without redundant comments that could become outdated.
19-19: LGTM! Removed redundant commentRemoving the comment about timer cleanup makes the code cleaner, as the action is self-explanatory.
scenes/UI/HowToPlay.tscn (1)
1-162: Well-structured UI scene for player instructions.This new "How To Play" scene is well-organized with a clear layout structure. The scene uses a combination of controls, labels, and sprites to present gameplay information in a visually structured way. The layout is divided into three main sections (margin containers) that appear to explain different gameplay mechanics with appropriate visual aids.
I particularly like how the directional inputs are represented by rotated sprites, making it intuitive for players to understand the controls. The use of translation keys (e.g., "HOW_TO_PLAY_BLOCK1") also shows good internationalization practices.
scenes/Remapping/assets/Positional_Prompts_Up.png.import (1)
1-35: Standard Godot texture import configuration.This import file for the "Positional_Prompts_Up.png" texture uses appropriate default settings for a UI element. The compression settings (mode 0, no mipmaps) are suitable for UI graphics that need to remain crisp.
scenes/SceneTransitions/assets/font.TTF.import (2)
19-19: Enhanced font rendering with MSDF enabled.Enabling the multichannel signed distance field (MSDF) is a good choice for improving font rendering quality, especially when text might be scaled or transformed. This will make the font appear smoother at various sizes.
24-24: Font hinting disabled for more accurate rendering.Changing the hinting value from 1 to 0 disables font hinting. This typically results in a more accurate representation of the original font design at the expense of potentially less crisp rendering at very small sizes. This change works well in combination with the MSDF approach and is appropriate for a game interface.
README.md (1)
21-23: Music attribution updates look goodThe updates to music attribution correctly reflect the new audio files being used in the game. This aligns with the overall changes in the pull request where audio files and configurations have been modified.
scenes/Options/scripts/LanguageSelection.cs (1)
4-12: Language selection implementation looks goodThe
LanguageSelectionclass properly extendsOptionButtonand correctly connects the signal in the_Readymethod. Good practice to use the current locale fromTranslationServerto preset the dropdown.scenes/UI/scripts/Inventory.cs (3)
18-19: Good use of static array for internationalizationUsing a static array for tab names is a good approach for consistent key generation. This will help maintain consistency across the internationalization system.
55-55: Input handling improvementAdding support for the "ui_cancel" action is a good improvement for user experience, allowing more intuitive ways to exit the inventory.
65-65: Simplification looks goodRemoving the comment and keeping just the
QueueFree()call makes the code cleaner.scenes/SceneTransitions/assets/TitleFont.gdshader (1)
7-9: Shader simplification looks goodThe shader has been simplified to only handle the vertex manipulation for the wave effect. This is likely more performant and easier to maintain than the previous version with additional color effects.
The removal of the fragment shader and various uniforms indicates a design decision to simplify the visual effects, which aligns with the changes in the TitleScreen.tscn file mentioned in the summaries.
scenes/Remapping/assets/Positional_Prompts_Down.png.import (1)
1-34: Import configuration for UI texture looks good.The configuration settings for this texture are appropriate for a UI element:
- No compression with
compress/mode=0preserves visual quality for UI elements- Alpha border fixing enabled with
process/fix_alpha_border=truefor clean transparency- Mipmaps disabled with
mipmaps/generate=false, which is suitable for UI elements that maintain their size- Properly configured for 2D with
detect_3d/compress_to=1These settings are well-suited for a directional prompt texture in the remapping UI.
scenes/Remapping/assets/Positional_Prompts_Right.png.import (1)
1-34: Import configuration maintains consistency with other directional prompts.This texture import configuration matches the settings used for the Down prompt, maintaining consistency across the directional prompt assets. The unique identifier is properly set with a distinct UID as required by the Godot engine.
The settings (no compression, alpha border fixing, no mipmaps) are appropriate for UI elements that require crisp visuals at their intended display size.
scenes/BattleDirector/notePoofParticles.tscn (1)
1-33: Well-configured particle effect for note feedback.This particle effect is properly set up to create a satisfying visual feedback when notes are processed:
- One-shot emission with high explosiveness (1.0) creates an immediate burst effect
- Short lifetime (0.75 seconds) keeps the effect brief and non-distracting
- The scale curve ensures particles fade out smoothly
- Random velocity parameters (30.77-61.55) and spread (180°) create natural-looking dispersion
The effect reuses the heal note texture, which creates visual consistency with the note system.
scenes/Maps/scripts/Cartographer.cs (4)
23-26: Good refactoring to use getter method instead of direct property access.Changing from direct property access (
StageProducer.CurRoom) to using the getter method (StageProducer.GetCurRoom()) improves encapsulation and maintains consistency with proper object-oriented practices.
81-81: Consistent use of getter method for room access.This change maintains the consistency of using the getter method instead of direct property access, which is good for encapsulation and code maintenance.
111-111: Consistent use of getter method throughout the file.This change completes the refactoring pattern of using the
GetCurRoom()method instead of direct property access, maintaining consistency throughout the file.
151-151: Good localization implementation using translation keys.Replacing the hardcoded string "You Win!" with a translatable string
Tr("BATTLE_ROOM_WIN")improves the game's internationalization support. This allows for easy translation to different languages without code changes.scenes/UI/scripts/RewardSelect.cs (2)
101-103: Good implementation of localization for note descriptionsThe change to use the
Tr()function with standardized translation keys is a good practice for localization. Converting the note name to uppercase helps maintain consistent translation key formatting.
109-112: Good implementation of localization for relic descriptionsSimilar to the notes implementation, using the
Tr()function with standardized translation keys improves localization support. The additional step to remove spaces from relic names ensures consistent key formatting.scenes/Remapping/assets/Positional_Prompts_Left.png.import (1)
1-34: Standard Godot texture import file looks goodThis is an auto-generated Godot import file with standard texture settings. The file correctly specifies the import parameters for the controller prompt image, which will be used in the new controller support implementation.
scenes/UI/scripts/PauseMenu.cs (5)
13-15: Menu button handlers reorganized correctlyThe pause menu button handlers have been reorganized to include the new Options menu functionality. Button indexing is consistent, with a logical progression from Resume → Options → Quit → Quit to Main Menu.
19-25: Good UI focus managementThe new
_Processmethod ensures that a button always has focus when the pause menu is active, which improves keyboard/controller navigation. This is a good UX practice for menu interfaces.
27-33: Well-implemented Options menu integrationThe new
OpenOptionsmethod properly loads, instantiates, and configures the options menu. Passingthisto theOpenMenumethod allows for proper parent-child relationship management between menus.
37-37: Improved input handling with standard Godot actionChanging from a custom "Pause" action to the standard Godot "ui_cancel" action improves consistency with the engine's input system and makes the code more maintainable.
47-47: Proper cleanup in Resume methodAdding
QueueFree()ensures the pause menu is properly removed from the scene tree when resuming the game, preventing potential memory leaks or visual artifacts.scenes/NoteManager/scripts/InputHandler.cs (2)
86-92: Good addition of controller input detectionThis new method properly detects when a controller button is pressed and updates the input configuration accordingly. This replaces the previous
LoadControlSchememethod with a more Godot-aligned approach using the_UnhandledInputmethod.
105-110: Good implementation of scheme-based input actionsThe input scheme is now correctly prepended to the action keys, ensuring that the right input mapping is used based on the current control scheme.
scenes/BattleDirector/scripts/NoteQueueParticlesFactory.cs (1)
4-28: Well-designed particle factory with good resource managementThis new factory class provides a clean, reusable way to create particle effects with automatic cleanup. The implementation:
- Lazily loads the particle scene resource
- Properly configures the particles with customizable parameters
- Sets up automatic cleanup when particles finish emitting
- Returns the instance for additional configuration if needed
One minor improvement could be to make the scene path configurable rather than hardcoded:
- private static PackedScene particlesScene = null; + private static PackedScene particlesScene = null; + private static readonly string DefaultParticlesPath = "res://scenes/BattleDirector/notePoofParticles.tscn"; public static GpuParticles2D NoteParticles( Node parent, Texture2D texture, float amountModifier = 1 ) { - particlesScene ??= GD.Load<PackedScene>( - "res://scenes/BattleDirector/notePoofParticles.tscn" - ); + particlesScene ??= GD.Load<PackedScene>(DefaultParticlesPath);scenes/UI/Pause.tscn (5)
14-14: LGTM! pauseButtons array properly updated with new OptionsButtonThe pauseButtons array has been correctly updated to include the new OptionsButton, which ensures it will be properly initialized by the PauseMenu script.
48-49: Button navigation and localization properly updatedThe ResumeButton's focus_neighbor_bottom property has been correctly updated to point to the new OptionsButton, and the text has been changed to use a localization key.
75-77: Button navigation and localization properly updatedThe QuitButton's focus_neighbor_top property has been correctly updated to point to the new OptionsButton instead of ResumeButton, and the text has been changed to use a localization key.
91-91: Placeholder button text updated to use localizationThe PlaceHolderButton's text has been changed to use a localization key.
51-64:Details
❓ Verification inconclusive
New options button added with proper UI integration
The new OptionsButton has been properly added with:
- Consistent margin settings matching other buttons
- Correct focus navigation setup
- Localized text using the "TITLE_OPTIONS" key
🏁 Script executed:
#!/bin/bash # Verify that the localization key "TITLE_OPTIONS" exists in the translation files fd "\.translation$" --exec cat {} | grep -i "TITLE_OPTIONS"Length of output: 61
Attention: Localization Key Verification Required
- The new OptionsButton is integrated correctly in terms of layout and focus navigation.
- However, our automated search did not find the "TITLE_OPTIONS" key in any translation files. Please manually verify that this localization key is defined (or add it if missing) to ensure the button text is properly localized.
Classes/Relics/RelicTemplate.cs (4)
8-8: Good addition of ID property for RelicTemplateAdding an ID property is a good enhancement that allows for easier reference and tracking of relics throughout the system.
15-20: Constructor parameter updates with improved naming conventionThe constructor has been updated to include the new
idparameter and renamedEffectTagstoeffectTagsfollowing proper C# naming conventions for parameters (camelCase).
22-24: Constructor implementation updated for new ID propertyThe ID property is properly initialized and the parameter naming has been updated accordingly.
31-31: Clone method updated to preserve IDThe Clone method now correctly passes the ID to the new instance, ensuring that relic identity is preserved during cloning.
scenes/NoteManager/scripts/NoteArrow.cs (2)
29-29: Good refactoring to use the extracted method!The use of the new
GetNewPos()method here makes the initialization cleaner and consistent with the position calculation in_Process.
38-38: Good refactoring to simplify position calculations.Using the extracted
GetNewPos()method here simplifies the code and follows the DRY principle by centralizing position calculation logic.scenes/Options/OptionsMenu.tscn (2)
6-14: Well-structured Options Menu with clear node references.The OptionsMenu is well-organized with properly exported node references for the script to access. The use of
process_mode = 3ensures the menu can operate independently of the game's pause state.
103-110: Language selection is well implemented.The language selection dropdown with English and Chinese options is well implemented, with a dedicated script to handle language switching functionality.
scenes/BattleDirector/scripts/Conductor.cs (3)
82-83: Method renaming improves code clarity.Renaming from
AddExampleNotestoAddInitialNotesbetter reflects the method's purpose and improves code readability.
85-94: Good simplification of note initialization logic.The refactored
AddInitialNotesmethod now directly uses beat information from MIDI notes, which simplifies the code and makes it more maintainable.
52-60:Details
✅ Verification successful
Good enhancement with particle effects for visual feedback.
The addition of particle effects for non-active notes enhances visual feedback. The comment clarification that
isActivemeans an enemy note is helpful for code readability.
🏁 Script executed:
#!/bin/bash # Check if NoteQueueParticlesFactory is properly imported in the file rg "using .+NoteQueueParticlesFactory" scenes/BattleDirector/scripts/ # Find the NoteQueueParticlesFactory class definition to verify it exists fd -e cs | xargs grep -l "NoteQueueParticlesFactory" | xargs catLength of output: 11485
Verified: Particle Effects and "Enemy Note" Clarification
The enhancements for visual feedback using particle effects are correctly implemented. The
NoteQueueParticlesFactoryis properly referenced and defined, and the distinction between enemy (non-active) notes and regular notes via theisActiveflag is clear and adds to the code’s readability.
- The else branch correctly instantiates and attaches the particle effects.
- The inline comment clarifying that
isActiverepresents an enemy note is beneficial.Classes/Notes/Note.cs (5)
12-12: Good addition of ID property for note identification.Adding an ID property to the Note class allows for unique identification of notes, which is valuable for tracking and reference purposes.
16-17: Parameter change in NoteEffect signature.The Action delegate now includes a Timing parameter which will be used in damage calculations. This allows for more dynamic gameplay where timing affects damage output.
22-33: Constructor updated to include ID parameter.The constructor now accepts and initializes the ID property, ensuring each note has a unique identifier assigned at creation time.
40-41: Damage calculation now incorporates timing.Modifying the damage calculation to include timing information makes gameplay more dynamic, rewarding players for better timing with increased damage.
58-67: Clone method updated to preserve note identity.The Clone method now passes the ID when creating a new note, ensuring the clone maintains the same identity as the original note.
scenes/UI/scripts/HowToPlay.cs (3)
20-25: Approve menu handling implementation.The
OpenMenumethod correctly stores the previous scene and its process mode before disabling it, allowing for proper menu state management.
27-31: Approve cleanup implementation.The
CloseMenumethod properly restores the previous scene's process mode and removes the menu from the scene tree.
33-40: Good implementation of UI cancel handling.The input handler correctly implements the escape key functionality by calling
CloseMenu()and marking the input as handled to prevent it from propagating further.Classes/MidiMaestro/MidiMaestro.cs (5)
34-34: Improved error handling.The change from
GD.PrintErrtoGD.PushErroris a good improvement as it properly integrates with Godot's error reporting system, making errors more visible and traceable.
38-39: Static field initialization looks good.The initialization of the static fields correctly retrieves the TempoMap and TimeSignature from the MIDI file.
47-47: Simplified constructor by removing parameter.The removal of the
tempoMapparameter from the constructor is a good simplification since the class now uses the static fields fromMidiMaestro.
91-95: Implemented new method to calculate beats.The new
GetStartTimeBeatmethod correctly calculates the start time in beats using the static TempoMap and TimeSignature.
99-101: Updated method to use static TempoMap.The
GetStartTimeSecondsmethod has been updated to use the static TempoMap instead of a local variable, which is consistent with the new design.scenes/ChestScene/ChestScene.tscn (3)
11-15: Shader material parameters updated.The ShaderMaterial has been updated with a new ID and the gradient_ratio parameter has been changed from 1.22 to 1.0, which may affect the visual appearance of the shader.
50-68: Improved render architecture with SubViewport.The StarShader has been moved from the root level to inside a SubViewportContainer with a SubViewport. This is a better architectural approach as it:
- Isolates the shader rendering to its own viewport
- Provides more control over how the shader is displayed
- Potentially improves performance by separating the rendering process
4-4:Details
✅ Verification successful
Audio resource reference updated.
The AudioStream resource reference has been updated to a new UID. Ensure that the new audio file is correctly imported and accessible.
🏁 Script executed:
#!/bin/bash # Check if the audio file exists and is accessible fd "Song1.ogg" --type fLength of output: 62
Resolved: Audio Resource Reference Verified
The updated AudioStream reference in
scenes/ChestScene/ChestScene.tscn(line 4) now correctly points tores://Audio/Song1.ogg. The verification output confirms that the audio file (and its corresponding import file) exists, so no further action is needed.Globals/FunkEngineNameSpace.cs (2)
81-85: Good property implementation in the Room class.The Room class now properly exposes its properties with appropriate access modifiers. This is a good improvement for encapsulation:
- Properties have public getters but private setters
- Children is initialized with an empty array to avoid null references
- All properties are now explicitly defined rather than implicitly
65-65:Details
❓ Verification inconclusive
Replaced 'Controls' enum value with 'Load'.
The enum value
Controlshas been replaced withLoadin theStagesenum. Make sure that all references toStages.Controlsthroughout the codebase have been updated toStages.Load.
🏁 Script executed:
#!/bin/bash # Check for any remaining references to Stages.Controls rg "Stages\.Controls" --type csLength of output: 33
Action Required: Please Manually Verify That All References to
Stages.ControlsHave Been UpdatedThe automated search for
Stages.Controls(usingrg "Stages\.Controls" --type cs) produced no matches, which suggests that the enum value might have been fully replaced withLoad. However, due to the ambiguous output, it's not fully conclusive.
- Confirm that the change in
Globals/FunkEngineNameSpace.cs(Line 65) correctly reflects the intended replacement.- Please re-run a full-codebase search (or manually verify) to ensure there are no residual references to
Stages.Controls.scenes/BattleDirector/scripts/NotePlacementBar.cs (2)
41-41: Rename rationale and consistent shaking logic.
Renaming_randomStrengthto_baseShakeand using_shakeStrength = _baseShake;clarifies intent. Ensure that_baseShakeis properly calibrated so that the screen shake isn’t too subtle or extreme for gameplay.Also applies to: 53-53
190-199: AddedBattleDirectorparameter for placed note logic.
The revised signaturepublic Note PlacedNote(BattleDirector BD)integrates theBattleDirectorcontext well. The call toOnHit(BD, Timing.Okay)is a workable placeholder. Ensure that eventual expansions to timing logic are handled here if more precise note timing is implemented in the future.scenes/Puppets/scripts/PuppetTemplate.cs (5)
5-5: Docstring improvement acknowledged.
Maintaining comprehensive documentation fosters better collaboration and readability.
57-63: Initialization of shake variables.
Using constants likeBaseShake = 100fis fine. Verify that the final shake amplitude aligns well with the game’s visual style. If needed, consider making_shakeFadeand_baseAnimDurationexported for real-time tweaking.
82-110: Damage/healing animation adds valuable feedback.
The newly introducedDamageAnimate(int amount)method cleanly handles color flash and positional tweening. For more advanced feedback, consider interpolating color or layering particles.
114-116: Early return prevents unnecessary updates.
Skipping further logic when_currentHealth <= 0oramount == 0is efficient. Good short-circuit approach.
132-132: Unified damage and heal animation approach.
ReusingDamageAnimatewith a negative amount for healing is a clever unification. Ensure that future expansions of healing effects also remain consistent with this approach.Also applies to: 135-135
Globals/Scribe.cs (4)
15-25: EnemyBase note calculation.
The expression(3 - (int)timing)for damage suggests a decreasing damage with better timing. Double-check that negative or zero values don’t occur when timing is higher than 3.
41-53: Double damage note.
The note is labeled “double,” but the actual formula isnote.GetBaseVal() * (int)timing. Ensure the final multiplier is genuinely double relative toPlayerBasefor correct balancing.
84-97: PlayerQuarter cost and damage.
This note has a cheaper cost (0.25f) but deals(int)timing + note.GetBaseVal(). Verify that integer rounding always yields the intended partial advantage.
156-172: “Colorboros” relic and FreeStyle bar.
Increasing the bar by 20 each loop is a significant buff. Confirm that the bar’s max value is respected to avoid overfilling.Globals/StageProducer.cs (6)
27-30: Initialization in_EnterTree.
DeferringInitFromCfgto_EnterTreeis valid for Godot’s lifecycle. Ensure no race conditions happen if you rely on these configs in other node_Ready()calls.
51-51: Call toGenerateMapConsistentat game start.
Randomizing and then generating the map ensures a fresh layout per run. This is good for roguelike variety.
58-84:LoadGamemethod.
Overwriting RNG seed and state from the save file is correct for reproducing the exact level. Ensure that any discrepancies (like changed code or data) don’t break existing saves.
92-96: Getter forGetCurRoom().
ExposingMap.GetRooms()[CurRoom]is straightforward. Confirm boundary checks ifCurRoomgoes out of range.
109-109: Temporarily removingContrastFilter.
Pausing the filter’s parent-child relationship can avoid visual flickers, but re-adding it can reset any runtime changes to the filter. Keep that in mind.
148-150: Re-adding theContrastFilter.
This step ensures the filter remains consistent across scene transitions. Just confirm no race conditions or flicker upon re-add.scenes/Remapping/Remap.tscn (9)
1-1: Confirm the reducedload_steps.
By loweringload_stepsfrom 9 to 8, ensure that all deferred scene loading steps still complete properly without causing unexpected initialization issues.Would you like a script to check whether all required nodes are being initialized correctly at runtime?
20-25: NewColorRectoverlay appears correct.
This overlay can be used for dimming or background shading. Ensure the chosen color and offsets align with your intended design.
40-45: Localized text identifier for WASD scheme.
Switching to"CONTROLS_WASD_BUTTON"is beneficial for future i18n expansions or string table lookups. This looks good.
53-53: Maintain naming consistency for Arrow scheme.
Using"CONTROLS_ARROW_BUTTON"fosters consistent naming with the other control scheme identifiers.
61-61: Refined text for QWER scheme.
Renaming to"CONTROLS_QWER_BUTTON"ensures uniform naming and potential for i18n.
63-69: NewControllerButtonnode.
Adding this button extends the control scheme to support controllers. Ensure the underlying logic is implemented inControlSettings.csor a relevant script.
77-77: Updated return button text.
Replacing existing text with"CONTROLS_RETURN_BUTTON"aligns with the localized or standardized approach.
142-142: Reduced scales for direction key sprites.
Scaling each key sprite toVector2(0.4, 0.4)preserves consistent sizing and visual symmetry.Also applies to: 147-147, 152-152, 157-157
11-18:Details
✅ Verification successful
Add usage checks for newly introduced
_closeButton.
Thenode_pathsnow include_closeButton, which is mapped to "Panel/TitleButton". Confirm that external scripts or signals referencing this button handle the new NodePath correctly.
🏁 Script executed:
#!/bin/bash # Searches for all references to `_closeButton` usage to verify everything is consistent. rg "_closeButton"Length of output: 951
Usage of
_closeButtonverified across the codebaseThe search results confirm that
_closeButtonis not only defined inscenes/Remapping/Remap.tscnbut is also consistently referenced in external scripts. In particular, the following observations were made:
ControlSettings.cs (scenes/Remapping/ControlSettings.cs)
_closeButtonis declared and itsPressedevent is correctly hooked up toCloseMenu. Additionally, visibility checks involving_closeButtonindicate that its state is actively monitored.OptionsMenu (scenes/Options/OptionsMenu.tscn and OptionsMenu.cs)
The_closeButtonNodePath in the Options scene correctly maps to its corresponding UI element, and the button’s event handling (i.e.,Pressed += CloseMenu) is in place.These findings suggest that the external references to
_closeButtonare handling the new NodePath as expected. No further modifications are required regarding its usage.scenes/SceneTransitions/TitleScreen.tscn (5)
41-41: Extended node paths to includeOptions.
Ensure the_Optionsfield in the linked script references this new path. The new path is "VBoxContainer/Options", which looks consistent with the newly introduced button.Also applies to: 53-53
109-111: RepositionedStarShadernode.
Decreasedoffset_rightto475andoffset_bottomto177, then scaled to(2, 2). Verify that the starfield or background effect still displays as intended on different screen resolutions.
177-177: Updated button texts and scene paths.
Migrating from hard-coded strings such as"Start Game"to localized placeholders like"TITLE_START","TITLE_CONTINUE", and"TITLE_QUIT"helps with i18n. TheScenePathchanges appear correct, but confirm that path6is the intended "continue" or "quit" route.Also applies to: 194-194, 196-196, 210-210
215-217: Visibility changed and text updated for "Controls" button.
visible = falsewithtext = "TITLE_CONTROLS". Confirm the controlling logic clarifies why we hide this button initially.
221-224: NewOptionsbutton.
Labeled"TITLE_OPTIONS", this button likely opens the newOptionsMenu. Double-check the script handling for consistency with previous patterns (e.g.,SceneChangeusage or direct scene instantiation).scenes/Options/OptionsMenu.cs (11)
1-3: Namespace imports look correct.
The usage ofSystemandGodotis standard for a Godot C# script.
4-7: Partial class introduction forOptionsMenu.
Inheritance fromCanvasLayerallows the menu to overlay existing scenes. The_previousScenereference is a clean approach to track the underlying scene.
8-9: Exported UI control fields.
Declaring fields like_focused,_volumeSlider,_closeButton, etc., via[Export]is consistent with exposing them in the Godot editor.Also applies to: 12-13, 15-16, 18-19, 21-22, 24-25
29-47: _Ready lifecycle logic.
Focus grabbing, slider initialization, and toggling high-contrast are well-structured. Make sure the references toSaveSystemalign with your final config keys (Volume, HighContrast).
49-56: Child focus fallback in_Process.
The logic re-captures focus if none is set, preventing unexpected focus loss. This is good for consistent menu navigation.
58-65:_Inputhandling for "ui_cancel".
Gracefully closing the menu on "ui_cancel" is intuitive. Confirm thatGetViewport().SetInputAsHandled()does not conflict with other UI elements.
67-72:OpenMenudisables the previous scene.
Preventing updates in the underlying scene is a common approach for a modal menu. This is well-reasoned.
74-78:CloseMenure-enables the previous processing mode.
Restoring_previousProcessModeis essential. Good job ensuring the prior state is remembered.
80-86:OpenControlsloadsRemap.tscn.
This method retrieves the scene as aControlSettingsobject and opens it. Verify thatControlSettingsindeed has anOpenMenu(this)method and a matching signature.
88-94:VolumeChangeddefers toChangeVolume.
Conditionally callingChangeVolumebased onvalueChangedhelps avoid repeated saves during slider drag. ConfirmSaveSystem.UpdateConfigusage for volume is seamlessly integrated.
111-117:OpenHowToPlay.
InstantiatingHowToPlayscene and callingOpenMenu(this)ensures consistent layering. Keep a watchful eye on potential overlaps with other UI layers if multiple submenus open at once.SaveData/SaveSystem.cs (11)
1-1: File-scoped imports and aliases.
Declaringusing System.Linq;and aliasingFileAccessis typical in Godot C#. The alias helps avoid collisions withSystem.IO.Also applies to: 4-4, 7-7
11-13: NewConfigFileapproach in#region Config.
Using aConfigFilefor user settings can be more robust than unstructured storage. Just ensure consistent usage across the codebase.
15-19: Default constants for volume, input, language, and high contrast.
These defaults are well-chosen. A note that 80f asDefaultVolumemust align with your decibel offset usage inOptionsMenu.
20-26: EnumConfigSettings.
Providing a strongly typed approach for config keys. This is good for code clarity.
28-36:InitConfigseeds defaults.
The function forcibly updates config settings, ensuring a baseline. This is a straightforward approach.
70-77:AssertConfigFile.
If the config file isnull, you load it. This ensures usage consistency.
103-112:LoadConfigData.
You fallback to creating a new config if none is found. Good for first-time user support.
114-134:GetConfigValuesafely returns defaults if keys are missing.
This is a user-friendly approach preventing crashes on unknown keys.
141-172:SaveFileclass.
Bundling RNG state, last room, note IDs, relic IDs, and health in one object is neat. Make sure future expansions (e.g., settings or progress) also remain consistent.
174-192:SaveGamemethod.
Serializing to JSON is straightforward; just watch out for versioning or expansions in theSaveFileclass over time.Would you like a script to check for references to
SaveGameto confirm that all relevant data gets included?
194-213:LoadGamemethod.
Null is returned if no file or a deserialization error occurs. This is a safe fallback but ensure you handlenulloutcomes gracefully in the game’s logic.scenes/UI/inventory.tscn (3)
14-15: Ensure references in scripts are updated.
Renaming these node paths is straightforward and helps clarify their roles. However, verify that all scripts referencing the oldRelicsandNotespaths are updated to avoid missing node errors.
43-55: Naming convention looks consistent.
Using theINVENTORY_TAB_prefix is a good way to improve clarity about each tab’s purpose. Everything looks cohesive here.
60-73: Confirm relic referencing.
As with the notes section, ensure outside references toRelicsare replaced withINVENTORY_TAB_RELICS.scenes/BattleDirector/scripts/BattleDirector.cs (6)
120-122: Check the new parameter usage for room transitions.
Switching fromBattleRoomtoBattleRoom.Idxlikely aligns with newly introduced changes, but ensure no downstream references still expect the old room object.
128-136: ValidateCM.TrueBeatsPerLoop.
UsingTrueBeatsPerLoopfor timing is presumably more accurate. Confirm thatTrueBeatsPerLoopis initialized and consistent with chart data to avoid undefined or incorrect timing loops.
162-173: Localized combo messages for misses.
DisplayingTr("BATTLE_ROOM_" + Timing.Miss.ToString().ToUpper())ensures misses are fully localized, which is great. Confirm the translation keys exist to prevent fallback to the raw string.
187-192: Localized combo messages for hits.
Similarly, this snippet localizes hit feedback. Good consistency with the miss logic.
235-235: Assess the impact of clearing the entire save.
SaveSystem.ClearSave();is a critical change. Some users might expect partial or no wipe on battle loss. Confirm this is truly intended for your gameplay flow.
240-248: Verify newly localized text for battle victory.
UsingTr("BATTLE_ROOM_WIN")is consistent with other localized strings, but verify the translation entry is present.scenes/Remapping/ControlSettings.cs (9)
18-24: Maintain references carefully.
Storing the previous scene and process mode is a sound approach for menu toggling. Just ensure_previousSceneis not null and is still valid when you re-enable it.
33-40: Check initial controller setup.
You’re connecting_controllerButtonand callingControllerConnectionChanged(-1, false)to initialize the state. Verify that using device ID-1is intentional (it may act as a placeholder for “no device”).
54-58: CONTROLLER scheme integration.
Good job introducing a “CONTROLLER” case to handle gamepads. This complements the existing schemes.
63-74: Menu toggling approach is logical.
Disabling the previous scene’s process mode is a clean way to pause background behavior. Carefully handle cases where the previous scene might already be paused.
76-81: Menu closure via “ui_cancel”.
This makes sense for quickly backing out from the menu. Confirm it doesn’t conflict with other “ui_cancel” usage or expected behavior in nested menus.
87-89: Label text uses localized string references.
Appending localized strings is consistent. Double-check that the “WASD” label chunk is indeed in your translation files if you plan to localize it verbatim.
95-97: Arrow button labeling.
Same approach as WASD. This is consistent.
103-105: QWERT input scheme labeling.
Continuing the same pattern for QWERT is good for consistency.
116-126: Validate fallback logic when no controller is connected.
The code switches back to arrow keys if the user is set to “CONTROLLER” but_closeButton.Visibleis false. Confirm that lines up with user expectations—some might prefer to remain in a “controller only” state.project.godot (8)
24-27: Updated autoload references.
Ensure all scripts referencingStageProducerandBgAudioPlayerare aligned with these new resource paths.
33-33: Canvas items stretch mode.
Switching to"canvas_items"can improve pixel-perfect scaling. Verify that this matches your desired scaling behavior across UI scenes.
42-49: New “ui_accept” input events.
Assigning multiple keys and a joypad button is helpful for accessibility. Looks solid.
50-56: New “ui_cancel” input events.
Same approach asui_accept. Including multiple triggers is good for universal input.
57-112: Directional inputs unify keyboard & joypad.
Definingui_left,ui_right,ui_up, andui_downensures consistent logic. Verify no older references to direct arrow key events remain in your code.
131-150: CONTROLLER arrow inputs
Great to see explicit definitions for controllers. Confirm your game logic uses these newly defined actions rather than hardcoded joypad inputs.
151-190: WASD & QWERT sets
Mapping these ensures all alternative schemes are recognized. Just confirm they match user expectations (e.g., QWERT using “Q” & “E” for vertical movement).
192-195: Translations integrated
Adding translation files is a positive step for localization. Confirm the relevant strings referenced in code match keys in these.translationfiles.
collectioncard
left a comment
collectioncard
left a comment
There was a problem hiding this comment.
Need to remove debug enemy kill.
Omfg vertical slice is complete.
Summary by CodeRabbit
New Features
Enhancements