Milestone 3 Finalization #239
Conversation
* Created branch, added sprite * Heal Button functionality Shops now have heal button. Clicking it will restore 1/4 max hp at the cost of 30 money. This will disable the button for the reminder of time spent in shop. If player is at max hp or does not have enough money upon entering shop, button will be disabled automatically. Added display for the player's current health out of max health, with a placeholder heart sprite. * Adjust Shop For Heal Button Remove health label to use PlayerPuppet in the scene Adjust Shop UI because the heal button was too big for the container Adjust Translation to specify cost and heal amount Fixed issue where removing note didn't update money label * Adjust tutorial translation --------- Co-authored-by: LifeHckr <jarodthereal@gmail.com>
* Initial NoteChart Refactor Allow NoteChart to carry the weight of SongData This also allows all NoteCharts to be preloaded from disk * Implement Memento Pattern for SongTemplate * Base Custome Song Loading Functionality * Custom Song Conditional Availability Custom Song can only be accessed if a run has been won * Custom Level pauses BG Music Paused on transitioning to custom stage Unpaused on returning to menu * Adjust ChartLength based on Desired Speed NoteCharts can keep a desired speed Adjust ChartLength based to approximate desired speed Updated notecharts * Updated NoteQueue Sprite * Add Splash Screen * Add styling to hslider and dropdownmenu * Inventory Buttons are less clickable Attempt to avoid confusion thinking inventory items are useable * Improve Selection Screen Add translations Added labels Added a simply calculated difficulty * Added Custom Song Results Screen * Added Translations * Adjust and Fix Scoring Screens
Loops effects no longer process after battle end Holograeme is immune to relic damage
* Enemy money changes standardized the money from fights based on enemy tier * nerfed blood money, other relics and notes look fine * Adjusted enemy HP Only strange value to me is Parasifly, since we use one template and the fight can either be a 1v1 or 1v2, but this shouldn't be too big of an issue. * song speed changes value of 200 for all songs, notes now take about 2 seconds to go from being visible to being hit * standardized battle countdown * Undo countdown standardizing This was intentionally mis-timed to signify the BPM of a song. * Price adjustments * Adjust heal cost translation --------- Co-authored-by: LifeHckr <jarodthereal@gmail.com>
* Added relic/note/money to rob text The text should display the name of the relic/note/money taken from the player in paraenthesis. EventTemplate was slightly tweaked to include extra func OutcomeResolver. Adjusted EventDatabase and Event Scene accordingly. * Drastic Simplification One shortcoming with the and the previous version is if locale is changed while these messages are displayed they don't auto change. Doesn't seem to be a good way to accomplish this regardless. * Added support for the wheel results Should work correctly * Finalize Event 2 Text --------- Co-authored-by: LifeHckr <jarodthereal@gmail.com>
* Update: Initial changes to Coffeerat Changelog -> Added music, chart, and started on the script for Coffeerat. It's functional but needs some work soon. * Update: Fixed chart to 4 loops * Update Astrorat --------- Co-authored-by: Quinn-Hub <56490493+Quinn-Hub@users.noreply.github.com>
* Added war horn All normal battles are now elite battles * Added looter's lens thanks for making this so easy Jarod gives reward screens one more option to choose from * Added Quito first legendary relic 🙌 * Added soloist's triangle name pending, might have to update later once we get new note sprites * Added Tinsel also updated chart speed to be a float and multiplicative, in case we combine multiple chart speed values. * finished 3 relics (art and translations) * Finished Quito * Update new relic sprites * Finished Tinsel * Update Tinsel Sprite * Fix Duplicated Placed Note References Placed notes should be individual instances, and not share base val. * Implement Brass Cannon Note --------- Co-authored-by: LifeHckr <jarodthereal@gmail.com>
* Very hardcoded, but mostly works Doesn't "live" update. all the changes are on load, so changing the icons in the middle of a battle won't apply any changes. changing outside of battle works perfectly though. * Simplify Swap Arrow/Note Sprite Logic * Note symbol selection is now checkbox * Vertical Scroll Putting this here for safe keeping * Integrate Vertical Scroll Toggle Add new config setting Attempt to swap between normal CM and DW.CM based on setting Split Lane Lines into separate sprite Attempt to adjust sprites and combo text based on setting --------- Co-authored-by: LifeHckr <jarodthereal@gmail.com>
Make them rotate the correct direction This also means placed notes are rotated wrong, but thats ok
Replacing Song1 for now
Also tweak Midriff chart
* Initial README changes * Finished README music, added music to credits Later we need to move this giant credits string over to the credits cs file and out of the credit's scene. * forgot to include this song in credits scene * Ok, now my crashout is complete * Finalize Credits Scene Not actually gonna do the art crawl rn * Add Credit to End Screen Also add credits music --------- Resolve Issue where ChartViewport keeps changing Co-Authored-By: Connor Lowe <connorlowe21@gmail.com>
Remove Midriff from first level Correctly disable shop button after healing
Could change this to be Min(HP, 2) if we wanted, this is just a bandaid fix.
WalkthroughThis update introduces new and refactored systems for custom song support, vertical scroll mode, and 3D scene integration. It adds new enemies, notes, relics, and song maps, updates asset imports, and revises UI themes and layouts. Core gameplay logic is adjusted for chart speed, song metadata, and battle director flexibility. Changes
Sequence Diagram(s)Custom Song FlowsequenceDiagram
participant Player
participant CustomSelection
participant StageProducer
participant BattleDirector
participant CustomScore
Player->>CustomSelection: Open custom song menu
CustomSelection->>CustomSelection: Load user song templates
Player->>CustomSelection: Select a song
CustomSelection->>StageProducer: TransitionToCustom(selectedSong)
StageProducer->>BattleDirector: Start battle with custom song
BattleDirector->>BattleDirector: Play song, manage battle
BattleDirector->>CustomScore: Show custom score screen on battle end
CustomScore->>Player: Display results, await accept
Player->>CustomScore: Accept results
CustomScore->>StageProducer: Return to title
Vertical Scroll/Arrow Mode InitializationsequenceDiagram
participant SaveSystem
participant StageProducer
participant BattleDirector
participant InputHandler
SaveSystem-->>StageProducer: Load config (VerticalScroll, TypeIsArrow)
StageProducer->>BattleDirector: Set VerticalScroll flag
StageProducer->>InputHandler: Set UseArrows flag
BattleDirector->>InputHandler: Update arrow/note sprites as needed
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Gitleaks (8.26.0)Audio/Jammin' Forest (Charting Edition).ogg.import/bin/bash: -c: line 1: syntax error near unexpected token Audio/songMaps/Jammin' Forest.tres/bin/bash: -c: line 1: unexpected EOF while looking for matching `'' Classes/Relics/Assets/Relic_Soloist'sTriangle.png.import/bin/bash: -c: line 1: unexpected EOF while looking for matching `'' ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 11
🔭 Outside diff range comments (1)
Classes/MidiMaestro/MidiMaestro.cs (1)
25-35: Remove commented-out code for better maintainability.The commented-out file loading logic should be removed rather than kept as comments. Dead code makes the codebase harder to maintain and can cause confusion.
- /*if (!OS.HasFeature("editor")) - { - filePath = OS.GetExecutablePath().GetBaseDir() + "/" + filePath; - } - - if (!FileAccess.FileExists(filePath)) - { - GD.PushError("ERROR: Unable to load level songMap resource file: " + filePath); - } - - NoteChart savedChart = ResourceLoader.Load<NoteChart>(filePath);*/
♻️ Duplicate comments (1)
Audio/songMaps/ParasiflySingle.tres (1)
123-130: Same duplicate beat timing issues as ParasiflyDouble.tres.This chart has identical note layouts to ParasiflyDouble.tres but with different timing (BPM=60 vs 120). The same duplicate beat timings exist (beats 8.0, 11.0, 20.0, 22.0 appearing in multiple lanes).
Additionally, using the same audio file ("Song2.ogg") with different BPMs may cause synchronization issues unless the audio playback speed is adjusted accordingly.
🧹 Nitpick comments (16)
README.md (1)
30-58: Excellent music attribution improvements with formatting fix needed.The expanded and reorganized music attributions ensure proper licensing compliance and credit attribution. However, there are hard tab formatting issues that should be addressed.
Replace hard tabs with spaces for consistency:
-  Licensed under Creative Commons: By Attribution 4.0 License -  [http://creativecommons.org/licenses/by/4.0/](http://creativecommons.org/licenses/by/4.0/) +  Licensed under Creative Commons: By Attribution 4.0 License +  [http://creativecommons.org/licenses/by/4.0/](http://creativecommons.org/licenses/by/4.0/)Apply similar fixes to lines 44-45, 52-53, and 55-56.
Scenes/3DWizard/DimensionalWizard.cs (1)
1-8: LGTM! Simple and appropriate wrapper class for 3D chart integration.The
DimensionalWizardclass provides a clean interface for connecting 3D scenes with chart management functionality. The exported field allows proper editor configuration.Consider a more descriptive name for the
CMfield, such asChartManagerfor better code readability:[Export] -public ChartManager CM; +public ChartManager ChartManager;Scenes/NoteManager/Assets/NoteWIthOutline.png.import (1)
3-9: Filename capitalization is inconsistent.
The asset is namedNoteWIthOutline.png(capital “I” in “WIth”), which differs from the expectedNoteWithOutline.png. Consider renaming to maintain consistent casing across assets.Classes/MidiMaestro/MidiMaestro.cs (1)
46-46: Consider more specific error messaging.The error message could be more descriptive about what specifically went wrong with the songMap resource loading.
- GD.PushError("ERROR: Unable to load songMap resource file!"); + GD.PushError("ERROR: NoteChart parameter is null - unable to initialize lane data!");Scenes/Puppets/Enemies/Astrorat/P_Astrorat.cs (1)
15-22: Consider tween cleanup to prevent potential memory leaks.The infinite looping tween created in
_Ready()has no cleanup mechanism. While Godot typically handles tween cleanup when the node is freed, explicitly managing the tween lifecycle would be safer.Consider storing the tween reference and implementing cleanup:
+private Tween enemyTween; + public override void _Ready() { MaxHealth = 150; CurrentHealth = MaxHealth; BaseMoney = 20; base._Ready(); - var enemyTween = CreateTween(); + enemyTween = CreateTween(); enemyTween.TweenProperty(Sprite, "position", Vector2.Up * 5, 1f).AsRelative(); enemyTween.TweenProperty(Sprite, "position", Vector2.Down * 5, 1f).AsRelative(); enemyTween.SetTrans(Tween.TransitionType.Quad); enemyTween.SetEase(Tween.EaseType.InOut); enemyTween.SetLoops(); enemyTween.Play(); } + +public override void _ExitTree() +{ + enemyTween?.Kill(); + base._ExitTree(); +}Scenes/NoteManager/Scripts/InputHandler.cs (1)
43-43: Consider thread safety for static field access.The static
UseArrowsfield could be accessed from multiple threads without synchronization, potentially causing race conditions.Consider making the field readonly if it's set once during initialization, or add proper synchronization if it needs to be modified at runtime.
Scenes/Puppets/Enemies/Midriff/P_Midriff.cs (1)
15-21: Consider potential tween reference issues.The tween animation assumes
Spriteproperty exists and is accessible. Ensure the baseEnemyPuppetclass properly exposes this property, or consider adding null checking.var enemyTween = CreateTween(); +if (Sprite != null) { enemyTween.TweenProperty(Sprite, "position", Vector2.Left * 5, 1f).AsRelative(); enemyTween.TweenProperty(Sprite, "position", Vector2.Right * 5, 1f).AsRelative(); enemyTween.SetTrans(Tween.TransitionType.Quad); enemyTween.SetEase(Tween.EaseType.InOut); enemyTween.SetLoops(); enemyTween.Play(); +}Globals/FunkEngineNameSpace.cs (1)
293-293: Consider thread safety for the static field.The static
ForceEliteBattlesfield could cause issues in multi-threaded scenarios or if multiple battle systems run concurrently.Consider making this field part of a game state manager or battle configuration object instead of a static field to improve encapsulation and avoid potential state conflicts.
Scenes/ChartViewport/Scripts/ChartManager.cs (1)
107-107: LGTM: Improved code clarity.Removing the redundant
this.qualifier improves readability without changing functionality.Scenes/3DWizard/3DWizard.tscn (1)
47-48: Complex 3D transform values.The Sprite3D transform uses complex rotation values that may be difficult to maintain. Consider documenting the intended visual effect or using more readable transform composition.
+# Transform creates angled viewport display - maintains current visual effect transform = Transform3D(-4.37114e-08, -1, 0, 0.617036, -2.69715e-08, 0.786935, -0.786935, 3.4398e-08, 0.617036, 0, 0, 0.138215)Scenes/CustomSong/Scripts/CustomSelection.cs (1)
78-84: Consider refining the difficulty calculation algorithm.The current difficulty calculation is quite basic and may not accurately represent song difficulty. Consider factors like:
- Note density over time
- Complex rhythm patterns
- Note placement variety
-float arbitraryDifficulty = 0; -arbitraryDifficulty += (float)song.Chart.Bpm / 120; -arbitraryDifficulty += (float)song.Chart.GetLane(ArrowType.Up).Count / 10; -arbitraryDifficulty += (float)song.Chart.GetLane(ArrowType.Right).Count / 10; -arbitraryDifficulty += (float)song.Chart.GetLane(ArrowType.Left).Count / 10; -arbitraryDifficulty += (float)song.Chart.GetLane(ArrowType.Down).Count / 10; +float arbitraryDifficulty = CalculateSongDifficulty(song.Chart); + +private float CalculateSongDifficulty(NoteChart chart) +{ + float bpmFactor = (float)chart.Bpm / 120f; + int totalNotes = chart.GetLane(ArrowType.Up).Count + + chart.GetLane(ArrowType.Right).Count + + chart.GetLane(ArrowType.Left).Count + + chart.GetLane(ArrowType.Down).Count; + float densityFactor = totalNotes / (chart.NumLoops * 60f); // notes per minute + return (bpmFactor + densityFactor) * 2f; // Adjust multiplier as needed +}Scenes/ShopScene/ShopUI.tscn (1)
216-257: Avoid duplicate “OptionsBG” node namesThe root-level NinePatchRect and its child share the exact name
OptionsBG.
While legal, it makes NodePaths verbose (OptionsBG/OptionsMargin/OptionsBG/...) and easy to break during refactors.
Consider renaming the inner panel to something unique (e.g.,OptionsPane) to keep paths short and unambiguous.Scenes/CustomSong/CustomScore.cs (1)
79-82: Drop per-frame focus forcing
_ProcesscallsGrabFocus()every frame → needless CPU & causes focus flicker.
Move the call to_Ready()or after results are rendered once.Classes/MidiMaestro/SongTemplate.cs (1)
44-75:fromUserPathparameter is unused
CreateFromPath()receivesfromUserPathbut never reads it.
Either implement the alternate path handling or drop the parameter to prevent confusion.Globals/Scribe.cs (2)
247-271: Potential lifecycle & balancing concerns in new note “PlayerBrass”.
- The “initialisation via BaseVal == 0” trick relies on the prototype instance never being reused after cloning. If a live Note instance is re-pooled, BaseVal may reset incorrectly.
- Resetting the whole combo every time the note hits could feel punitive; double-check intended UX.
- Worth extracting the sentinel value (0 → 2) into a constant or explicit ctor to avoid this hidden state.
685-715: Damage-reduction path could underflow.
dmgArgs.Dmg.ModifyDamage(-val)blindly subtracts; if incoming damage is exactly 1, the check> 1blocks, but if external code later changes that condition this could drive damage below 0. Safer to clamp after modification.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (27)
Audio/EtherDisco.oggis excluded by!**/*.oggAudio/Jammin' Forest (Charting Edition).oggis excluded by!**/*.oggAudio/Paint-the-Town-Neon-Red.oggis excluded by!**/*.oggAudio/Spooky-Forest.oggis excluded by!**/*.oggClasses/Notes/Assets/Note_PlayerBrass.pngis excluded by!**/*.pngClasses/Relics/Assets/Relic_LootersLens.pngis excluded by!**/*.pngClasses/Relics/Assets/Relic_Quito.pngis excluded by!**/*.pngClasses/Relics/Assets/Relic_Soloist'sTriangle.pngis excluded by!**/*.pngClasses/Relics/Assets/Relic_Tinsel.pngis excluded by!**/*.pngClasses/Relics/Assets/Relic_WarHorn.pngis excluded by!**/*.pngGlobals/Translations/Translations.csvis excluded by!**/*.csvScenes/BattleDirector/Assets/NoteQueue_Frame.pngis excluded by!**/*.pngScenes/ChartViewport/LaneLines.pngis excluded by!**/*.pngScenes/NoteManager/Assets/Arrow_Outline.pngis excluded by!**/*.pngScenes/NoteManager/Assets/BottomNoteOutline.pngis excluded by!**/*.pngScenes/NoteManager/Assets/LeftNoteOutline.pngis excluded by!**/*.pngScenes/NoteManager/Assets/New_Arrow.pngis excluded by!**/*.pngScenes/NoteManager/Assets/Note.pngis excluded by!**/*.pngScenes/NoteManager/Assets/NoteWIthOutline.pngis excluded by!**/*.pngScenes/NoteManager/Assets/RightNoteOutline.pngis excluded by!**/*.pngScenes/NoteManager/Assets/TopNoteOutline.pngis excluded by!**/*.pngScenes/Puppets/Enemies/Astrorat/Assets/Astrorat.pngis excluded by!**/*.pngScenes/Puppets/Enemies/Midriff/Midriff.pngis excluded by!**/*.pngSharedAssets/BattleFrame1.pngis excluded by!**/*.pngSharedAssets/Heart.pngis excluded by!**/*.pngSharedAssets/NoteArrow_Black_Outline.pngis excluded by!**/*.pngSharedAssets/SplashScreenGodot.pngis excluded by!**/*.png
📒 Files selected for processing (107)
Audio/District_Four.ogg.import(1 hunks)Audio/EtherDisco.ogg.import(1 hunks)Audio/Jammin' Forest (Charting Edition).ogg.import(1 hunks)Audio/Paint-the-Town-Neon-Red.ogg.import(1 hunks)Audio/Spooky-Forest.ogg.import(1 hunks)Audio/songMaps/Astrorat.tres(1 hunks)Audio/songMaps/BossBlood.tres(1 hunks)Audio/songMaps/CyberFoxSong.tres(2 hunks)Audio/songMaps/EcholaneSong.tres(2 hunks)Audio/songMaps/FrostWaltz.tres(1 hunks)Audio/songMaps/GWS.tres(1 hunks)Audio/songMaps/Gobbler.tres(2 hunks)Audio/songMaps/HoloRepeat.tres(2 hunks)Audio/songMaps/Jammin' Forest.tres(1 hunks)Audio/songMaps/KeythuluSong.tres(2 hunks)Audio/songMaps/Mushroom.tres(1 hunks)Audio/songMaps/ParasiflyDouble.tres(1 hunks)Audio/songMaps/ParasiflySingle.tres(1 hunks)Audio/songMaps/Shapes.tres(2 hunks)Audio/songMaps/Song2.tres(0 hunks)Audio/songMaps/Spider.tres(2 hunks)Audio/songMaps/SquirkelSong.tres(2 hunks)Audio/songMaps/TutorialBoss176_7.tres(2 hunks)Audio/songMaps/TutorialSong.tres(1 hunks)Classes/Events/EventDatabase.cs(4 hunks)Classes/MapAreas/MapLevels.cs(1 hunks)Classes/MidiMaestro/MidiMaestro.cs(3 hunks)Classes/MidiMaestro/NoteChart.cs(1 hunks)Classes/MidiMaestro/SongTemplate.cs(2 hunks)Classes/Notes/Assets/Note_PlayerBrass.png.import(1 hunks)Classes/Relics/Assets/Relic_LootersLens.png.import(1 hunks)Classes/Relics/Assets/Relic_Quito.png.import(1 hunks)Classes/Relics/Assets/Relic_Soloist'sTriangle.png.import(1 hunks)Classes/Relics/Assets/Relic_Tinsel.png.import(1 hunks)Classes/Relics/Assets/Relic_WarHorn.png.import(1 hunks)Globals/FunkEngineNameSpace.cs(2 hunks)Globals/SaveSystem.cs(6 hunks)Globals/Scribe.cs(4 hunks)Globals/StageProducer.cs(6 hunks)README.md(2 hunks)Scenes/3DWizard/3DWizard.tscn(1 hunks)Scenes/3DWizard/DimensionalWizard.cs(1 hunks)Scenes/3DWizard/DimensionalWizard.cs.uid(1 hunks)Scenes/BattleDirector/BattleScene.tscn(4 hunks)Scenes/BattleDirector/NotePlacementBar.tscn(2 hunks)Scenes/BattleDirector/Scripts/BattleDirector.cs(11 hunks)Scenes/BattleDirector/Scripts/Conductor.cs(1 hunks)Scenes/BattleDirector/Scripts/NotePlacementBar.cs(1 hunks)Scenes/BattleDirector/Scripts/NoteQueueParticlesFactory.cs(1 hunks)Scenes/BattleDirector/Tutorial/Toriel.cs(1 hunks)Scenes/BattleDirector/Tutorial/Toriel.tscn(4 hunks)Scenes/ChartViewport/ChartViewport.tscn(2 hunks)Scenes/ChartViewport/LaneLines.png.import(1 hunks)Scenes/ChartViewport/Scripts/ChartManager.cs(3 hunks)Scenes/ChestScene/ChestScene.tscn(2 hunks)Scenes/CustomSong/CustomScore.cs(1 hunks)Scenes/CustomSong/CustomScore.cs.uid(1 hunks)Scenes/CustomSong/CustomScoreScreen.tscn(1 hunks)Scenes/CustomSong/CustomSelection.tscn(1 hunks)Scenes/CustomSong/Scripts/CustomSelection.cs(1 hunks)Scenes/CustomSong/Scripts/CustomSelection.cs.uid(1 hunks)Scenes/EventScene/EventScene.tscn(2 hunks)Scenes/Maps/Scripts/Cartographer.cs(2 hunks)Scenes/NoteManager/Assets/Arrow_Outline.png.import(1 hunks)Scenes/NoteManager/Assets/BottomNoteOutline.png.import(1 hunks)Scenes/NoteManager/Assets/LeftNoteOutline.png.import(1 hunks)Scenes/NoteManager/Assets/New_Arrow.png.import(1 hunks)Scenes/NoteManager/Assets/Note.png.import(1 hunks)Scenes/NoteManager/Assets/NoteWIthOutline.png.import(1 hunks)Scenes/NoteManager/Assets/RightNoteOutline.png.import(1 hunks)Scenes/NoteManager/Assets/TopNoteOutline.png.import(1 hunks)Scenes/NoteManager/HoldArrow.tscn(1 hunks)Scenes/NoteManager/NoteArrow.tscn(1 hunks)Scenes/NoteManager/NoteManager.tscn(1 hunks)Scenes/NoteManager/Scripts/InputHandler.cs(4 hunks)Scenes/NoteManager/Scripts/NoteArrow.cs(2 hunks)Scenes/NoteManager/Scripts/NoteChecker.cs(2 hunks)Scenes/Puppets/Enemies/Astrorat/Assets/Astrorat.png.import(1 hunks)Scenes/Puppets/Enemies/Astrorat/Astrorat.cs.uid(1 hunks)Scenes/Puppets/Enemies/Astrorat/Astrorat.tscn(1 hunks)Scenes/Puppets/Enemies/Astrorat/P_Astrorat.cs(1 hunks)Scenes/Puppets/Enemies/Astrorat/P_Astrorat.cs.uid(1 hunks)Scenes/Puppets/Enemies/BossBlood/P_BossBlood.cs(2 hunks)Scenes/Puppets/Enemies/CyberFox/P_CyberFox.cs(1 hunks)Scenes/Puppets/Enemies/Effigy/P_Effigy.cs(1 hunks)Scenes/Puppets/Enemies/Holograeme/P_Holograeme.cs(3 hunks)Scenes/Puppets/Enemies/Keythulu/P_Keythulu.cs(3 hunks)Scenes/Puppets/Enemies/LWS/P_LWS.cs(1 hunks)Scenes/Puppets/Enemies/Midriff/Midriff.png.import(1 hunks)Scenes/Puppets/Enemies/Midriff/Midriff.tscn(1 hunks)Scenes/Puppets/Enemies/Midriff/P_Midriff.cs(1 hunks)Scenes/Puppets/Enemies/Midriff/P_Midriff.cs.uid(1 hunks)Scenes/Puppets/Enemies/Mushroom/P_Mushroom.cs(1 hunks)Scenes/Puppets/Enemies/Parasifly/P_Parasifly.cs(1 hunks)Scenes/Puppets/Enemies/Shapes/P_Shapes.cs(1 hunks)Scenes/Puppets/Enemies/Spider/P_Spider.cs(1 hunks)Scenes/Puppets/Enemies/Squirkel/P_Squirkel.cs(1 hunks)Scenes/Puppets/Enemies/Strawman/P_Strawman.cs(1 hunks)Scenes/Puppets/Enemies/TheGWS/P_TheGWS.cs(1 hunks)Scenes/Puppets/Enemies/TheGobbler/P_Gobbler.cs(1 hunks)Scenes/Puppets/Enemies/Turtle/P_Turtle.cs(1 hunks)Scenes/Puppets/Scripts/PlayerStats.cs(2 hunks)Scenes/ShopScene/Scripts/ShopScene.cs(7 hunks)Scenes/ShopScene/ShopScene.tscn(1 hunks)Scenes/ShopScene/ShopUI.tscn(8 hunks)Scenes/UI/Assets/DisplayButtonTheme.tres(2 hunks)Scenes/UI/Assets/GeneralTheme.tres(3 hunks)
⛔ Files not processed due to max files limit (20)
- Scenes/UI/EndScreen.tscn
- Scenes/UI/Inventory.tscn
- Scenes/UI/Options/Credits.tscn
- Scenes/UI/Options/HowToPlay.tscn
- Scenes/UI/Options/OptionsMenu.tscn
- Scenes/UI/Options/Scripts/CreditsMenu.cs
- Scenes/UI/Options/Scripts/OptionsMenu.cs
- Scenes/UI/Remapping/ControlSettings.cs
- Scenes/UI/Remapping/Remap.tscn
- Scenes/UI/ScoreScreen.tscn
- Scenes/UI/Scripts/EndScreen.cs
- Scenes/UI/Scripts/Inventory.cs
- Scenes/UI/Scripts/PauseMenu.cs
- Scenes/UI/Scripts/ScoringScreen.cs
- Scenes/UI/TitleScreen/Scripts/TitleScreen.cs
- Scenes/UI/TitleScreen/TitleScreen.tscn
- Scenes/UI/TitleScreen/TitleScreenEffects.tscn
- SharedAssets/Heart.png.import
- SharedAssets/SplashScreenGodot.png.import
- project.godot
💤 Files with no reviewable changes (1)
- Audio/songMaps/Song2.tres
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
38-38: Hard tabs
Column: 1
(MD010, no-hard-tabs)
39-39: Hard tabs
Column: 1
(MD010, no-hard-tabs)
44-44: Hard tabs
Column: 1
(MD010, no-hard-tabs)
45-45: Hard tabs
Column: 1
(MD010, no-hard-tabs)
52-52: Hard tabs
Column: 1
(MD010, no-hard-tabs)
53-53: Hard tabs
Column: 1
(MD010, no-hard-tabs)
55-55: Hard tabs
Column: 1
(MD010, no-hard-tabs)
56-56: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🔇 Additional comments (165)
Scenes/Puppets/Enemies/Effigy/P_Effigy.cs (1)
13-13: Balance update: Adjusted BaseMoney for Effigy
Reduced BaseMoney to 80 to align with the updated reward scaling across enemy puppets.Scenes/Puppets/Enemies/TheGWS/P_TheGWS.cs (1)
13-13: Balance update: Increased BaseMoney for TheGWS
Updated BaseMoney to 20 to match the new balancing standards for medium-tier enemies.Scenes/Puppets/Enemies/TheGobbler/P_Gobbler.cs (1)
14-14: Balance update: Increased BaseMoney for Gobbler
Adjusted BaseMoney to 20 to ensure consistent reward scaling.Scenes/Puppets/Enemies/Squirkel/P_Squirkel.cs (1)
14-14: Balance update: Increased BaseMoney for Squirkel
Raised BaseMoney from 5 to 10 to improve reward parity.Scenes/Puppets/Enemies/Shapes/P_Shapes.cs (1)
13-13: Balance update: Increased BaseMoney for Shapes
Set BaseMoney to 20 to align with the overall enemy reward adjustments.Scenes/Puppets/Enemies/CyberFox/P_CyberFox.cs (1)
17-17: LGTM - Balanced reward increaseThe 4x increase in BaseMoney (5→20) aligns with the broader enemy balancing effort and seems appropriate given the CyberFox's 130 HP and Dodge ability.
Scenes/Puppets/Enemies/LWS/P_LWS.cs (1)
13-13: LGTM - Conservative balance adjustmentThe modest increase in BaseMoney (8→10) is appropriate for this lower-health enemy (80 HP) and maintains proportional rewards.
Scenes/Puppets/Enemies/Mushroom/P_Mushroom.cs (1)
12-14: LGTM - Well-balanced stat adjustmentThe combination of reduced MaxHealth (200→150) and increased BaseMoney (10→20) creates a good balance - making the enemy more accessible while maintaining reward incentive.
Scenes/Puppets/Enemies/Turtle/P_Turtle.cs (1)
11-13: Verify the balance impact of these significant stat changesThe Turtle received substantial increases: MaxHealth 150→225 (+50%) and BaseMoney 10→40 (+300%). Given the Turtle's self-healing ability that drains player energy (lines 32-34), the increased health could make this enemy significantly more challenging.
The reward increase compensates for the difficulty, but consider playtesting to ensure the balance remains engaging.
Scenes/Puppets/Enemies/Strawman/P_Strawman.cs (1)
16-16: LGTM - Appropriate tutorial enemy reward adjustmentThe 5x increase in BaseMoney (1→5) brings the tutorial enemy's reward more in line with other enemies. The original value of 1 was likely too low, and this adjustment maintains the tutorial enemy's role while providing a meaningful reward.
Scenes/Puppets/Enemies/Spider/P_Spider.cs (1)
11-11: LGTM! Enemy balance adjustment looks good.The health reduction from 100 to 60 aligns with the broader enemy rebalancing effort mentioned in the milestone. The change is straightforward and maintains consistency with the overall enemy stat adjustments.
Scenes/Puppets/Enemies/Parasifly/P_Parasifly.cs (1)
11-11: LGTM! Well-balanced enemy stat adjustments.The health reduction (100→75) paired with increased money reward (5→7) creates a good risk/reward balance. These changes align with the coordinated enemy rebalancing effort across the project.
Also applies to: 13-13
Scenes/Puppets/Enemies/Midriff/Midriff.png.import (1)
1-35: LGTM! Standard texture import configuration.The import settings are appropriate for a 2D sprite texture with quality preservation (compression mode 0) and proper alpha border handling. The configuration follows Godot best practices for enemy sprite assets.
README.md (1)
5-8: LGTM! Great addition of social media links.The new Discord, YouTube, and Bluesky links will improve project visibility and community engagement.
Audio/Spooky-Forest.ogg.import (1)
1-20: LGTM! Proper audio import configuration.The Ogg Vorbis import settings are appropriate for background music with looping enabled and standard musical timing parameters. This aligns with the consistent audio import pattern used throughout the project.
Scenes/ChestScene/ChestScene.tscn (2)
4-4: Confirm new audio resource import.
Please verify that the newAudioStreamext_resource path (res://Audio/Spooky-Forest.ogg) matches the actual imported file location and that itsuidis unique within the project to avoid any resource collisions.
28-29: Review audio node configuration.
TheAudioStreamPlayernode is now using the new stream withvolume_db = -2.0dB and autoplay enabled. Ensure that looping is correctly handled by the importedAudioStreamsettings (via the.importfile) and that the volume level fits the scene’s audio balance.Audio/Paint-the-Town-Neon-Red.ogg.import (1)
1-20: Verify musical metadata accuracy.
The import config setsbpm=0.0andbeat_count=0forPaint-the-Town-Neon-Red.ogg. Please confirm these values correctly reflect the track’s tempo and structure so that song maps and timing systems function as intended.Scenes/EventScene/EventScene.tscn (2)
5-5: Validate audio resource reference.
Ensure the newAudioStreamext_resource (res://Audio/Paint-the-Town-Neon-Red.ogg) uses the correctuidand thatid="3_4prsq"does not conflict with other ext_resources in this scene or the wider project.
29-30: Check audio player settings.
TheAudioStreamPlayernow points to the updated stream and hasvolume_db = -15.0dB with autoplay enabled. Confirm that looping is driven by the imported resource parameters and that this volume aligns with the intended mix for this scene.Classes/Relics/Assets/Relic_LootersLens.png.import (1)
1-35: Import configuration looks correct.
The texture import settings (compression mode, mipmap generation disabled, alpha processing) are consistent with existing relic assets. No changes needed.Classes/Relics/Assets/Relic_Soloist'sTriangle.png.import (1)
1-35: Import configuration is consistent.
Parameters mirror those of other relic textures. Please verify that the apostrophe inSoloist'sTriangleis handled correctly across all target platforms and resource references.Classes/Relics/Assets/Relic_Tinsel.png.import (1)
1-35: Import configuration looks correct.
Auto-generated by the engine; no manual adjustments required.Scenes/CustomSong/Scripts/CustomSelection.cs.uid (1)
1-2: Auto-generated UID file – no review needed.Scenes/CustomSong/CustomScore.cs.uid (1)
1-2: Auto-generated UID file – no review needed.Scenes/3DWizard/DimensionalWizard.cs.uid (1)
1-2: Auto-generated UID file – no review needed.Scenes/NoteManager/Assets/New_Arrow.png.import (1)
5-5: UID updated – import configuration is correct.Scenes/NoteManager/Assets/Arrow_Outline.png.import (1)
5-5: Standard asset UID update.This is a typical Godot asset reimport change where only the unique identifier is updated. No functional import settings were modified.
Scenes/Puppets/Enemies/Astrorat/P_Astrorat.cs.uid (1)
1-2: Standard Godot script UID file.This is an auto-generated Godot metadata file containing the unique identifier for the P_Astrorat.cs script. No functional review required.
Scenes/Puppets/Enemies/Midriff/P_Midriff.cs.uid (1)
1-2: Standard Godot script UID file.This is an auto-generated Godot metadata file containing the unique identifier for the P_Midriff.cs script. No functional review required.
Classes/MapAreas/MapLevels.cs (1)
139-139: Verify that the new song IDs are properly configured.The addition of song IDs 16 and 17 to the normal battle songs looks good. Please ensure these IDs are properly defined in the global song dictionary and have their corresponding assets and note charts available.
#!/bin/bash # Description: Verify that song IDs 16 and 17 exist in the song dictionary and have required assets # Search for song ID 16 and 17 definitions in the codebase echo "Searching for song ID 16 (Astrorat):" rg -A 3 -B 1 "16.*Astrorat|Astrorat.*16" echo -e "\nSearching for song ID 17 (CatGirl):" rg -A 3 -B 1 "17.*CatGirl|CatGirl.*17" # Check if song template definitions exist echo -e "\nSearching for SongTemplate or SongDictionary entries:" rg -A 5 "SongTemplate.*16|SongTemplate.*17" # Verify related asset files exist echo -e "\nChecking for Astrorat asset files:" fd -t f "Astrorat" --exec ls -la {} echo -e "\nChecking for CatGirl asset files:" fd -t f -i "catgirl" --exec ls -la {}Scenes/Puppets/Enemies/Astrorat/Astrorat.cs.uid (1)
1-2: Standard Godot script UID file.This is an auto-generated Godot metadata file containing the unique identifier for the Astrorat.cs script. No functional review required.
Audio/District_Four.ogg.import (1)
15-17: LGTM! Audio configuration changes support looping functionality.The changes to enable looping with appropriate timing parameters are consistent with the custom song support objectives and align with similar changes mentioned in the AI summary.
Scenes/BattleDirector/Scripts/NotePlacementBar.cs (1)
51-52: LGTM! Appropriate exposure of combo multiplier for external access.Making the
ComboMultproperty public while keeping it read-only maintains encapsulation while enabling other components to query the current combo multiplier state, which aligns with the broader architectural improvements mentioned in the PR.Scenes/ChartViewport/ChartViewport.tscn (1)
33-39: LGTM! Lane lines addition enhances chart visualization.The new TextureRect node for lane lines is properly positioned and integrated into the SubViewport, providing visual enhancement for chart display. The positioning coordinates align well with the existing camera setup.
Scenes/Puppets/Enemies/BossBlood/P_BossBlood.cs (2)
11-13: Balance changes look reasonable, but note the significant money reward increase.The health increase from 225 to 250 provides modest durability improvement, while the money reward increase from 15 to 50 represents a substantial 233% increase in player progression benefit.
29-29: Healing effect increase balances the improved rewards.The healing increase from 20 to 30 makes this boss more challenging, which helps balance the significant money reward increase.
Scenes/NoteManager/HoldArrow.tscn (2)
1-1: We should confirm if there’s a sub_resource (which counts toward load_steps) in this scene:#!/bin/bash # Count sub_resource entries in the scene file rg -c "sub_resource" Scenes/NoteManager/HoldArrow.tscn # Show each sub_resource line with its line number rg "sub_resource" -n Scenes/NoteManager/HoldArrow.tscn
15-16: ```shell
#!/bin/bashInspect HoldArrow.cs for any references to OutlineSprite or Outline node
rg -C5 "OutlineSprite" Scenes/NoteManager/Scripts/HoldArrow.cs
rg -C5 "Outline" Scenes/NoteManager/Scripts/HoldArrow.cs</details> <details> <summary>Scenes/BattleDirector/Scripts/NoteQueueParticlesFactory.cs (1)</summary> `7-8`: **Excellent refactoring to extract magic string.** This change improves code maintainability by centralizing the resource path in a named constant, making it easier to update and reuse across the codebase. </details> <details> <summary>Scenes/NoteManager/Assets/BottomNoteOutline.png.import (1)</summary> `1-35`: **Standard texture import configuration looks good.** The import settings are appropriate for a 2D game texture, with proper compression and alpha border fixing enabled for clean rendering. </details> <details> <summary>Scenes/ShopScene/ShopScene.tscn (1)</summary> `58-61`: **Player reference correctly established.** The addition of the `_player` NodePath properly connects the ShopUI to the PlayerPuppet, enabling shop interactions like healing and inventory management. The path `"../PlayerMarker/PlayerPuppet"` correctly navigates the scene hierarchy. </details> <details> <summary>Classes/Notes/Assets/Note_PlayerBrass.png.import (1)</summary> `5-6`: **Asset replacement properly configured.** The texture import file has been correctly updated to reference the new `Note_PlayerBrass.png` asset with appropriate UID and path changes while maintaining consistent import settings. Also applies to: 13-14 </details> <details> <summary>Audio/songMaps/SquirkelSong.tres (1)</summary> `1-1`: **LGTM! Song map standardization looks good.** The addition of UIDs and standardized metadata properties (Bpm, NumLoops, SongSpeed, SongMapLocation) aligns well with the broader song map refactoring effort. The values are reasonable and the resource structure is properly maintained. Also applies to: 3-4, 88-91 </details> <details> <summary>Scenes/BattleDirector/Tutorial/Toriel.cs (1)</summary> `123-124`: **LGTM! Tutorial timing adjustment looks appropriate.** Moving the selector visibility to Dialogue3 appears to better align with the tutorial narrative flow. The positioning logic is consistent with other selector positioning throughout the file. </details> <details> <summary>Scenes/Puppets/Scripts/PlayerStats.cs (2)</summary> `19-19`: **Good addition for dynamic chart speed control.** The `ChartSpeedMultiplier` field with a default value of 1f provides a clean foundation for relic effects and dynamic speed adjustments mentioned in the AI summary. --- `83-87`: **Useful overload for note removal by reference.** The new `RemoveNote(Note nSelection)` method provides a convenient way to remove notes by reference rather than index. The implementation correctly handles multiple instances as documented. </details> <details> <summary>Scenes/NoteManager/NoteArrow.tscn (2)</summary> `1-1`: **LGTM! Texture transition from arrows to notes is well-executed.** The change from `New_Arrow.png` to `Note.png` aligns with the broader refactoring to note-based visuals. Reducing load steps and moving to dynamic texture assignment for outlines is a good architectural improvement. Also applies to: 3-3, 7-7 --- `12-12`: **Dynamic texture assignment approach looks good.** The Outline node without a static texture assignment suggests it will be set dynamically in code, which provides more flexibility for directional outline textures as mentioned in the AI summary. </details> <details> <summary>Scenes/NoteManager/Assets/TopNoteOutline.png.import (1)</summary> `1-35`: **LGTM! Texture import configuration is properly set up.** The import settings are appropriate for a UI texture asset: - No compression preserves crisp edges for outline graphics - VRAM texture disabled is suitable for UI elements - Alpha border fixing prevents edge artifacts - Mipmaps disabled is correct for UI textures </details> <details> <summary>Classes/Relics/Assets/Relic_WarHorn.png.import (1)</summary> `1-35`: **LGTM! Standard texture import configuration.** The texture import settings are properly configured for a 2D sprite asset with appropriate compression and processing parameters. </details> <details> <summary>Scenes/NoteManager/Scripts/NoteArrow.cs (3)</summary> `37-38`: **LGTM! Texture assignments look correct.** The texture assignments properly utilize the parent checker's texture and outline texture, which aligns with the updated NoteChecker structure. --- `41-42`: **LGTM! Conditional vertical scroll rotation is well-implemented.** The conditional rotation adjustment for vertical scroll mode is appropriately guarded and only applies when the feature is enabled. --- `2-2`: **Verify namespace change doesn't break existing references.** The change from `namespace FunkEngine;` to `using FunkEngine;` moves this class out of the FunkEngine namespace. Ensure all existing references to `FunkEngine.NoteArrow` are updated accordingly. ```shell #!/bin/bash # Description: Search for references to FunkEngine.NoteArrow to verify they still work after namespace change. # Expected: Find any references that might need updating. # Search for qualified references to NoteArrow in the FunkEngine namespace rg -A 3 "FunkEngine\.NoteArrow" # Search for using statements that might import NoteArrow from FunkEngine rg -A 3 "using.*NoteArrow"Audio/songMaps/HoloRepeat.tres (2)
1-4: LGTM! Standard resource UID updates.The addition of UIDs for the resource and script references follows Godot's resource management best practices for better reference tracking.
128-131: LGTM! New song metadata properties are well-defined.The added metadata properties (BPM, loops, speed, and audio location) provide essential song information with reasonable values that align with the broader song map standardization effort.
Scenes/Puppets/Enemies/Holograeme/P_Holograeme.cs (3)
23-23: LGTM! Vertical scroll rotation management is properly implemented.The rotation is correctly set to 0 degrees on battle start and restored to 90 degrees on exit, ensuring clean state transitions for the vertical scroll feature.
Also applies to: 47-47
31-31: LGTM! Money reward increase aligns with balancing updates.The increase from 20 to 40 base money is consistent with similar adjustments made across multiple enemy types for improved game balance.
97-100: ```shell
#!/bin/bash
set -e
echo "Comparing OnDamageInstance cases for Holograeme, Keythulu, and Strawman..."
for f in
Scenes/Puppets/Enemies/Holograeme/P_Holograeme.cs
Scenes/Puppets/Enemies/Keythulu/P_Keythulu.cs
Scenes/Puppets/Enemies/Strawman/P_Strawman.cs; do
echo "=== $f ==="
rg -C5 'OnDamageInstance' "$f" || true
done</details> <details> <summary>Scenes/Maps/Scripts/Cartographer.cs (2)</summary> `77-81`: **LGTM! Well-structured conditional vertical scroll initialization.** The logic properly checks the FirstTime configuration setting and only enables vertical scrolling for new players, with appropriate config persistence. The early return pattern keeps the code clean and efficient. --- `211-211`: **LGTM! Explicit win state setting improves clarity.** Setting `HasWon = true` explicitly ensures the EndScreen displays the correct winning state rather than relying on default values, improving code clarity and reliability. </details> <details> <summary>Audio/songMaps/Gobbler.tres (3)</summary> `1-4`: **LGTM! Good addition of UIDs for resource management.** The addition of unique identifiers for the main resource and external script references aligns with Godot's best practices for resource management. --- `218-221`: **Excellent addition of song metadata properties.** The new `Bpm`, `NumLoops`, `SongSpeed`, and `SongMapLocation` properties provide essential metadata for the custom song support feature. The values appear reasonable for gameplay (120 BPM, 6 loops, 200.0 speed). --- `222-225`: **Verify compatibility with the simplified lane data array format.** The change from typed `ExtResource` arrays to plain `SubResource` arrays simplifies the data structure. Ensure that all consuming code (like `NoteChart` class and `MidiMaestro`) properly handles this format change. ```shell #!/bin/bash # Description: Verify that NoteChart and MidiMaestro classes handle SubResource arrays properly # Expected: Find code that processes lane data arrays and confirm it works with SubResource format # Search for lane data processing in NoteChart class ast-grep --pattern 'class NoteChart { $$$ $LANE_DATA_FIELD LaneData $$$ $$$ }' # Search for MidiMaestro usage of lane data rg -A 5 -B 5 "LaneData"Audio/songMaps/KeythuluSong.tres (3)
1-4: LGTM! Consistent UID implementation.The UID additions match the pattern used in other song map files, maintaining consistency across the resource system.
208-211: Good boss-appropriate song metadata.The metadata values (170 BPM, 9 loops) suggest this is designed for a more intense boss encounter compared to other songs. The higher BPM and loop count align well with Keythulu being a significant enemy.
212-215: Consistent array format change.The lane data array simplification matches the pattern used in
Gobbler.tresand other song maps, ensuring consistency across the resource system.Scenes/Puppets/Enemies/Keythulu/P_Keythulu.cs (4)
17-17: Reasonable boss reward increase.Increasing BaseMoney from 50 to 80 provides better incentive for defeating this boss enemy, which aligns with typical game balance progression.
35-35: Good balance adjustment for boss encounter.Reducing the OnBattleStart trigger threshold from 7 to 6 makes the MindCrush effect more impactful, appropriate for a boss-level enemy.
68-72: Excellent integration with custom song system.The condition properly handles the new Custom room type by early returning, preventing boss-specific effects from interfering with custom battles. The pattern matching for type checking is well-implemented.
79-79: Good game completion tracking.Setting the
HasWonflag when Keythulu is defeated properly tracks game completion state in the save system.Audio/EtherDisco.ogg.import (1)
1-20: Standard and properly configured audio import.The OGG Vorbis import configuration is correct with appropriate settings:
- Looping enabled for game music
- Standard compression and timing parameters
- Proper UID and file path mappings
The BPM being set to 0 is appropriate since the actual BPM will be defined in the corresponding song map resource.
Scenes/NoteManager/Assets/RightNoteOutline.png.import (1)
1-35: Properly configured texture import for UI elements.The texture import configuration is appropriate with:
- No compression (mode=0) ensuring quality for note outline graphics
- Standard parameters for game UI assets
- Proper UID and file path mappings
- Alpha border fixing enabled for clean edges
This supports the enhanced note outline system mentioned in the broader updates.
Audio/songMaps/GWS.tres (3)
1-4: Resource header and script references updated correctly.
The gd_resource declaration and ext_resource entries align with the new NoteChart integration and maintain the proper UID and script paths.
292-296: Verify metadata property names and resolution.
Ensure that the exported properties (Bpm,NumLoops,SongSpeed,SongMapLocation) exactly match the PascalCase names inNoteChart.csand confirm thatSongMapLocationpoints to the correct resource path (e.g., includeres://Audio/...if your loader expects a full path).
297-301: Consistent conversion to SubResource arrays for lane data.
The lane data arrays now correctly use plainSubResourcereferences, matching the standardized format across all song maps.Audio/Jammin' Forest (Charting Edition).ogg.import (3)
3-7: Importer and target path configuration are correct.
Theoggvorbisstrimporter,AudioStreamOggVorbistype, UID, and import path syntax match the project’s conventions for audio streaming resources.
10-11: Dependency block correctly references source and destination.
Thesource_fileanddest_filespaths align with the remap section, ensuring the imported resource is tracked properly.
15-19: Default timing parameters are appropriate.
Settingbpm=0.0andbeat_count=0here is consistent with deferring musical timing to the chart data;bar_beats=4follows the standard time signature.Scenes/NoteManager/Assets/LeftNoteOutline.png.import (3)
3-9: Texture import settings look good.
TheCompressedTexture2Dtype, VRAM flag, and import path are correct for a 2D outline asset.
13-15: Dependency references are accurate.
Thesource_fileanddest_filesentries match the new asset location under Scenes/NoteManager/Assets.
18-35: Compression and processing parameters are consistent.
All compression modes, mipmap settings, and alpha fixes align with project-wide texture import conventions.Scenes/NoteManager/Assets/NoteWIthOutline.png.import (2)
13-15: Dependency entries are correct.
The import’ssource_fileanddest_fileslines properly reference the new outline texture.
18-35: Import parameters follow established patterns.
Compression quality, mipmap disabling, and alpha border fixes mirror other note-outline imports.Scenes/ChartViewport/LaneLines.png.import (3)
3-9: Lane lines texture import setup is correct.
TheCompressedTexture2Dtype, VRAM setting, and import remap path fit the ChartViewport asset pipeline.
13-15: Dependency block is accurate.
Thesource_fileanddest_filesentries correspond precisely to the new LaneLines asset.
18-35: Compression and processing parameters are well-defined.
The settings here are consistent with other UI overlay textures in the project.Audio/songMaps/Spider.tres (2)
138-141: Verify SongSpeed standardization across song maps.The new metadata properties look good and align with the NoteChart class refactoring. However, all song maps in this PR use the same
SongSpeed = 200.0value. Please verify this is intentional standardization rather than a copy-paste oversight, as different songs might benefit from different speed settings.
142-145: LGTM! Lane data format simplification.The removal of explicit type annotations and simplification to plain SubResource lists aligns with the broader resource format standardization mentioned in the summary.
Audio/songMaps/FrostWaltz.tres (2)
98-101: Consistent metadata addition with appropriate song-specific values.The new metadata properties are properly configured with song-specific BPM (99) and loop count (5), while maintaining the standardized SongSpeed (200.0). This confirms the pattern established across song map resources.
102-105: LGTM! Consistent lane data format update.The lane data array simplification matches the format used in other updated song map resources.
Scenes/NoteManager/Assets/Note.png.import (1)
1-35: LGTM! Standard texture import configuration.The import settings are appropriate for a note texture asset - lossless compression maintains visual quality while
detect_3d/compress_to=1provides reasonable optimization for the 3D pipeline integration mentioned in the summary.Audio/songMaps/TutorialSong.tres (2)
108-111: Appropriate tutorial song configuration.The metadata is well-configured for a tutorial: slower BPM (90), single loop, and standardized speed. This should provide a good learning experience for new players.
112-115: LGTM! Tutorial-appropriate lane configuration.The simplified lane data format is consistent with other song maps. The empty
RightLaneDataarray is likely intentional to keep the tutorial focused on basic directional inputs.Scenes/NoteManager/Scripts/NoteChecker.cs (2)
36-36: Consistent modulation property usage.The update to use
ModulateinSetColoris consistent with the changes in_Process, maintaining the new modulation behavior throughout the class.
18-23: Verify the change from SelfModulate to Modulate.The switch from
SelfModulatetoModulatechanges how child nodes are affected by color modulation.Modulateaffects both the node and its children, whileSelfModulateonly affects the node itself. Please verify this change aligns with the intended visual behavior for the new outline system.#!/bin/bash # Search for other usages of SelfModulate vs Modulate in NoteChecker-related code echo "Searching for SelfModulate usage in note-related files:" rg -A 3 -B 3 "SelfModulate" --type cs echo -e "\nSearching for Modulate usage in note-related files:" rg -A 3 -B 3 "\.Modulate\s*=" --type csScenes/UI/Assets/DisplayButtonTheme.tres (2)
52-96: New button style sub-resources look well-configured.The new StyleBoxTexture sub-resources for the "nopress" button style are properly configured with consistent texture margins and appropriate modulate colors for different button states.
104-110: "nopress" button style properly inherits from Button.The new button style category correctly sets "Button" as its base type and properly maps all required button states (disabled, focus, hover, hover_pressed, normal, pressed) to their respective StyleBoxTexture sub-resources.
Classes/MidiMaestro/MidiMaestro.cs (1)
23-23: Constructor signature change improves API design.Accepting a
NoteChartobject directly instead of a file path makes the API cleaner and more testable. This change aligns well with the broader refactor to useNoteChartobjects throughout the codebase.Scenes/BattleDirector/Tutorial/Toriel.tscn (4)
13-13: Node path correctly updated for new hierarchy.The dialogue label path has been properly updated to reflect the new nested MarginContainer structure:
"Control/CenterContainer/VBoxContainer/MarginContainer/MarginContainer/TextLabel".
45-53: Proper margin container structure added.The additional MarginContainer wrapping provides better spacing control for the UI elements. The margin constants (6px on all sides) create consistent padding around the dialogue box.
73-73: Autowrap mode change improves text handling.The autowrap mode change to
3(AUTOWRAP_WORD_SMART) will provide better text wrapping behavior for the dialogue label.
115-147: Position adjustments align with UI restructuring.The marker and sprite position adjustments appear to be coordinated with the UI hierarchy changes to maintain proper visual alignment in the tutorial interface.
Scenes/Puppets/Enemies/Astrorat/Assets/Astrorat.png.import (1)
1-35: Standard texture import configuration is appropriate.The import settings are well-configured for a 2D game texture asset:
- Compression mode 0 (Lossless) is appropriate for game sprites
- HDR compression enabled for better quality
- Mipmaps disabled (standard for 2D sprites)
- Alpha border fixing enabled to prevent texture bleeding
Audio/songMaps/Mushroom.tres (2)
78-81: New metadata properties enhance song map functionality.The addition of
Bpm(100),NumLoops(4),SongSpeed(200.0), andSongMapLocation("Mushroom.ogg") provides essential metadata for song playback and aligns with the broader refactor to standardize song map resources.
82-85: Lane data structure simplification improves maintainability.The simplification from typed
Array[ExtResource("1_n1l5w")]to plain lists ofSubResourceelements makes the resource format cleaner and easier to maintain while preserving all existing note data.Audio/songMaps/BossBlood.tres (2)
1-4: Resource format updated correctly.The UID-based resource referencing improves resource identification and reduces conflicts. The external script references are properly updated to use UIDs.
248-251: New metadata properties align with system refactor.The addition of
Bpm,NumLoops,SongSpeed, andSongMapLocationproperties supports the enhanced song map system described in the AI summary.Audio/songMaps/CyberFoxSong.tres (1)
118-125: Metadata properties and array format updated consistently.The new properties align with the song map refactor, and the conversion from typed arrays to plain SubResource arrays maintains consistency with the updated format.
Audio/songMaps/Jammin' Forest.tres (2)
83-86: Song metadata properties configured appropriately.The BPM (150), loop count (4), song speed (200.0), and file location are properly configured for this new song map.
87-90: Inconsistent array format compared to other song maps.The lane data arrays use
Array[ExtResource("1_0pymh")]syntax, while other updated song map files (likeBossBlood.tresandCyberFoxSong.tres) use plain arrays of SubResources. This inconsistency could cause issues with resource loading.Verify if this array format difference is intentional or should be updated for consistency:
#!/bin/bash # Check array format patterns across all song map files echo "Checking array formats in song map files..." rg -A 1 "LaneData = " Audio/songMaps/Audio/songMaps/ParasiflyDouble.tres (1)
51-130: Verify duplicate beat timings across lanes.The note chart contains potential overlapping beats that might cause simultaneous note hits:
- Beat 11.0 appears in both LeftLaneData (Resource_xq1dl) and RightLaneData (Resource_1vvgc)
- Beat 20.0 appears in both DownLaneData (Resource_86xei) and RightLaneData (Resource_ejow0)
- Beat 8.0 appears in both LeftLaneData (Resource_rnhdg) and RightLaneData (Resource_pbett)
- Beat 22.0 appears in both LeftLaneData (Resource_of7dy) and RightLaneData (Resource_gru0r)
Ensure this is intentional for chord-like gameplay or consider adjusting beat timings to avoid unintended simultaneous inputs.
#!/bin/bash # Description: Check for duplicate beat timings in song map files to verify if this pattern is consistent across other charts. # Search for duplicate beat values in other song map files fd -e tres -x grep -l "Beat =" {} \; | head -5 | xargs -I {} sh -c 'echo "=== {} ==="; grep "Beat =" "{}" | sort | uniq -d'Scenes/NoteManager/NoteManager.tscn (1)
18-64: Verify outline texture assignments match directional expectations.The outline texture assignments appear to have potential mismatches:
arrowLeftusesTopNoteOutline.png(line 26) - should this beLeftNoteOutline.png?arrowUpusesLeftNoteOutline.png(line 38) - should this beTopNoteOutline.png?arrowDownusesRightNoteOutline.png(line 50) - should this beBottomNoteOutline.png?arrowRightusesBottomNoteOutline.png(line 62) - should this beRightNoteOutline.png?Audio/songMaps/ParasiflySingle.tres (1)
126-126: Verify audio synchronization with different BPM settings.Both ParasiflySingle.tres (BPM=60) and ParasiflyDouble.tres (BPM=120) reference the same audio file "Song2.ogg". Ensure the game engine properly handles BPM differences for the same audio track, or consider using separate audio files optimized for each BPM.
Scenes/Puppets/Enemies/Midriff/P_Midriff.cs (1)
23-38: LGTM! Well-structured battle event implementation.The battle event properly checks for level progression and applies reasonable scaling (health increases by effect value, money increases by 10% of that). The level check prevents unintended scaling at early levels.
Scenes/BattleDirector/Scripts/Conductor.cs (1)
21-35: LGTM! Clean refactoring to support NoteChart architecture.The Initialize method properly accepts the new parameters and correctly initializes the components:
- Direct assignment of the injected
ChartManager- Updated
MidiMaestroconstructor call withNoteChart- Proper initialization of
ChartManagerwith new parametersThe removal of the
[Export]attribute from theCMfield (line 10) is appropriate since it's now dependency-injected rather than configured in the editor.Scenes/BattleDirector/NotePlacementBar.tscn (3)
1-1: LGTM - Resource optimization.Reducing load steps from 15 to 14 indicates removal of an unused texture resource, which is good for performance optimization.
129-129: Note positioning adjustment looks appropriate.The NoteQueueSprite position adjustment aligns with the broader UI updates for vertical scroll mode support.
133-138: Sprite updates support new note system.The repositioning and texture updates to use
Note_PlayerDouble.pngfor both CurrentNote and NextNote sprites align with the note system enhancements mentioned in the AI summary.Globals/SaveSystem.cs (6)
18-21: Verify keyboard input default changes are intentional.The default keyboard mappings have been updated:
- Up: S (83) - unchanged
- Down: K (75) - changed
- Right: L (76) - changed
Ensure these new defaults provide a better user experience and don't conflict with existing user configurations.
31-35: New configuration constants are well-defined.The new boolean constants for TypeIsArrow, VerticalScroll, and HasWon have appropriate default values and follow the existing naming convention.
54-58: Enum additions maintain proper order.The new ConfigSettings enum values are appropriately placed and maintain consistency with the existing structure.
82-86: Initialization properly includes new settings.All new configuration settings are correctly initialized in the InitConfig method, ensuring they have default values on first run.
145-159: Save logic correctly implements new settings.The UpdateConfig method properly handles the new settings with correct sectioning:
- TypeIsArrow and VerticalScroll in "Options" section
- HasWon in "Game" section
This follows the established pattern for similar settings.
295-304: Load logic matches save implementation.The GetConfigValue method correctly retrieves the new settings with appropriate default fallbacks, maintaining consistency with the save logic.
Classes/Relics/Assets/Relic_Quito.png.import (1)
1-35: Standard texture import configuration looks appropriate.The import settings are properly configured for a relic texture asset:
- CompressedTexture2D type is appropriate for UI elements
- Compression settings are reasonable for game assets
- VRAM texture disabled is appropriate for non-3D textures
- Alpha border fix enabled prevents edge artifacts
Scenes/Puppets/Enemies/Midriff/Midriff.tscn (1)
18-36: Enemy puppet scene structure follows established patterns.The Midriff enemy scene is well-structured with:
- Proper node paths configuration for script references
- Standard health bar with appropriate gradient (green to white)
- Status container for battle effects
- Sprite positioning that accounts for the sprite's dimensions
The scene structure is consistent with other enemy puppet scenes in the project.
Audio/songMaps/Astrorat.tres (1)
211-221: Song chart resource is well-structured.The NoteChart resource for "Astrorat" is properly configured:
- Reasonable BPM (120) and loop count (4)
- Song speed (200.0) appears appropriate for gameplay
- Note data is evenly distributed across all four lanes
- Beat timings range appropriately from 1.0 to 27.0
- All notes are single-hit (Length = 0.0) which is consistent
The resource follows the expected structure for the enhanced song management system.
Scenes/Puppets/Enemies/Astrorat/Astrorat.tscn (1)
1-35: LGTM! Well-structured enemy scene.The scene follows standard Godot patterns for enemy puppets with proper node hierarchy, external resource references, and appropriate health bar styling. The gradient texture creates a visually appealing green health bar effect.
Audio/songMaps/Shapes.tres (3)
1-4: LGTM! Proper resource identification.The addition of UIDs for the main resource and external script dependencies ensures proper resource tracking and reference management in Godot.
188-191: LGTM! Essential metadata standardization.The new exported properties (Bpm, NumLoops, SongSpeed, SongMapLocation) provide crucial song metadata that aligns with the NoteChart class enhancements. The values are reasonable and follow the established pattern across other song maps.
192-195: LGTM! Simplified array structure.The change from typed arrays of ExtResource to plain arrays of SubResource references simplifies the data structure while maintaining functionality. This standardization improves consistency across all song map files.
Audio/songMaps/TutorialBoss176_7.tres (3)
1-4: LGTM! Consistent resource management.The UID additions follow the same pattern as other song map files, ensuring proper resource identification and dependency tracking.
293-296: LGTM! Appropriate boss battle metadata.The metadata values are well-suited for a tutorial boss: higher BPM (176) for intensity, consistent loop count (7), and proper audio file reference. The values align with the standardized format across all song maps.
297-300: LGTM! Consistent array standardization.The simplified SubResource array format matches the pattern applied across all song map files, improving consistency and maintainability while preserving all note timing data.
Classes/MidiMaestro/NoteChart.cs (1)
11-24: LGTM! Well-designed metadata properties.The new exported properties are properly structured with:
- Appropriate data types for each metadata field
- Sensible default values (SongSpeed = -1 for auto-calculation)
- Clear documentation about SongSpeed behavior
- Perfect alignment with the standardized song map resource format
The properties enable consistent metadata management across the song system.
Audio/songMaps/EcholaneSong.tres (3)
1-4: LGTM! Completes standardization pattern.The UID additions complete the consistent resource identification pattern applied across all song map files in this update.
223-226: LGTM! Appropriate song metadata.The metadata values suit this song well: moderate BPM (120), fewer loops (4), and proper audio file reference. This completes the standardization of metadata properties across all song maps.
227-230: LGTM! Final array standardization.The simplified SubResource array format completes the consistent data structure standardization across all song map files, improving maintainability and reducing redundancy.
Scenes/UI/Assets/GeneralTheme.tres (3)
1-1: LGTM: Load steps correctly updated for new resources.The load steps increase from 12 to 22 aligns with the addition of new external resources and sub-resources in the theme.
55-130: LGTM: Well-structured theme expansion for new UI components.The new StyleBoxTexture and StyleBoxFlat resources provide comprehensive styling for various UI states (focus, hover, normal) with consistent margin, color, and corner radius properties. The integration supports the custom song selection and 3D wizard components mentioned in the AI summary.
170-177: LGTM: Proper style assignments for enhanced UI controls.The new style assignments for CheckBox, HSlider, PopupMenu, and TabContainer components are correctly mapped to the corresponding StyleBox resources, providing consistent visual feedback across the interface.
Globals/FunkEngineNameSpace.cs (2)
262-262: LGTM: Custom stage enum addition supports new functionality.The addition of
Customto theStagesenum aligns with the custom song support mentioned in the AI summary and integrates well with the existing stage system.
301-311: LGTM: Well-implemented conditional room type logic.The property implementation correctly handles the ForceEliteBattles logic while maintaining backward compatibility. The private backing field pattern is appropriate here.
Scenes/BattleDirector/BattleScene.tscn (2)
23-33: LGTM: Node paths correctly updated for new 3D wizard integration.The node paths properly reference both the existing chart manager (CM) and new dimensional wizard (DW) components, supporting the dual-mode battle system described in the AI summary.
68-81: LGTM: Well-structured 3D viewport integration.The SubViewportContainer setup with the initially hidden 3DWizard instance allows for clean switching between 2D and 3D modes. The viewport configuration with
handle_input_locally = falseandrender_target_update_mode = 4is appropriate for this embedded scenario.Scenes/ChartViewport/Scripts/ChartManager.cs (2)
77-100: LGTM: Clean refactoring to NoteChart-based initialization.The method signature change and updated initialization logic properly handle the transition from SongData to NoteChart. The chart length calculations correctly incorporate song speed and player stats while maintaining proper bounds checking.
284-288: LGTM: Proper vertical scroll support for combo text.The conditional logic correctly adjusts text rotation and position for vertical scroll mode, maintaining visual consistency with the new gameplay feature.
Classes/Events/EventDatabase.cs (3)
26-34: LGTM: Excellent localization enhancement for note loss events.The dynamic string formatting properly extracts the note name, converts it to the localization key format, and uses
string.FormatwithTranslationServer.Translatefor proper internationalization support.
42-50: LGTM: Consistent localization pattern for relic loss events.The implementation follows the same successful pattern as the note events, ensuring consistency across different event types.
94-153: LGTM: Sophisticated spinner event localization.The introduction of the
eventEffectvariable and the comprehensive switch statement with proper localization for each outcome greatly improves the user experience. The dynamic content insertion for notes, relics, and money amounts is well implemented.Scenes/CustomSong/CustomScoreScreen.tscn (1)
1-202: Well-structured UI scene with consistent theming.The scene file demonstrates good UI design practices:
- Proper use of anchors and responsive layout
- Consistent theming with forest background and crystal frames
- Well-organized container hierarchy
- Clear node paths for script connections
The structure effectively displays score metrics in an organized manner with appropriate visual styling.
Scenes/3DWizard/3DWizard.tscn (1)
24-26: ```shell
#!/bin/bashShow context around the VPContainer node in ChartViewport.tscn
rg -n -C3 'node name="VPContainer"' --glob 'Scenes/ChartViewport/ChartViewport.tscn'
Show context around the ChartManager script assignment (ExtResource id="1_ruh2l")
rg -n -C3 'script = ExtResource("1_ruh2l")' --glob 'Scenes/ChartViewport/ChartViewport.tscn'
</details> <details> <summary>Globals/StageProducer.cs (4)</summary> `40-45`: **Excellent performance optimization with scene preloading.** The addition of scene preloading during initialization will significantly improve user experience by reducing load times during transitions. This is a well-implemented performance enhancement. --- `62-67`: **Good integration of new configuration options.** The initialization of `InputHandler.UseArrows` and `BattleDirector.VerticalScroll` from configuration properly centralizes settings management and ensures consistent state across the application. --- `242-257`: **Well-designed custom song transition method.** The `TransitionToCustom` method properly: - Resets game state for custom battles - Creates appropriate battle configuration - Handles scene transition cleanly The implementation maintains consistency with existing transition patterns. --- `339-346`: **Useful fullscreen toggle feature.** The F11 fullscreen toggle is a standard and expected feature that enhances user experience. The implementation correctly toggles between fullscreen and windowed modes. </details> <details> <summary>Scenes/CustomSong/CustomSelection.tscn (2)</summary> `8-14`: **Well-organized node path configuration.** The exported node paths are clearly defined and follow a logical hierarchy. The focus management setup with `_focused` pointing to the return button provides good default behavior. --- `76-85`: **Excellent scrollable container setup.** The ScrollContainer configuration with `follow_focus = true` and disabled horizontal scrolling creates an optimal user experience for navigating the song list with keyboard/controller input. </details> <details> <summary>Scenes/CustomSong/Scripts/CustomSelection.cs (3)</summary> `30-37`: **Good directory initialization and error handling.** The code properly ensures the user song directory exists and handles the case where directory access fails with appropriate warnings. --- `57-76`: **Excellent graceful handling of missing resources.** The code properly handles two error conditions: - Songs with null charts (disabled with explanatory text) - Songs with missing audio files (disabled with file path shown) This provides clear feedback to users about why songs are unavailable. --- `145-145`: **Verify LastStage assignment purpose.** Setting `StageProducer.LiveInstance.LastStage = Stages.Title` seems to force the last stage to Title. Ensure this is the intended behavior and doesn't interfere with other navigation flows. ```shell #!/bin/bash # Description: Check how LastStage is used throughout the codebase # Expected: Should understand the purpose of this assignment rg -A 3 -B 3 "LastStage"Scenes/ShopScene/Scripts/ShopScene.cs (1)
348-355: Guard against a null_playerreference
_playeris injected via the editor; if the node path is ever mis-wired the first heal press will NRE-crash.
Add a debug assertion or early-exit null check around_player.Heal(...).Globals/Scribe.cs (2)
558-565: Null source now passed toDealDamage– confirm nullable-handling.
DealDamage(Targetting.All, val, null)assumes the method copes with anullsource. If any downstream logic dereferencessource, this change will crash at battle start. Please verify or add a null-guard insideDealDamage.
575-588: Balance tweak looks OK.Changing Blood Money’s value 10 → 5 is purely numeric; no functional issues spotted.
| StageProducer.PlayerStats.RemoveNote( | ||
| Array.IndexOf(StageProducer.PlayerStats.CurNotes, _toRemove) | ||
| ); | ||
| _selectedRemoveButton.QueueFree(); |
There was a problem hiding this comment.
Remove notes by reference, not by index
Array.IndexOf(...) can return –1 (not found) or point to the wrong duplicate, leading to an out-of-range error inside RemoveNote.
Expose / use an overload like PlayerStats.RemoveNote(Note note) to avoid index math entirely.
🤖 Prompt for AI Agents
In Scenes/ShopScene/Scripts/ShopScene.cs around lines 335 to 338, the code
removes notes by index using Array.IndexOf, which can cause errors if the note
is not found or duplicates exist. To fix this, modify or use an existing
overload of PlayerStats.RemoveNote that accepts a Note object directly, and pass
_toRemove to it instead of calculating the index. This avoids index-related
errors and ensures the correct note is removed by reference.
There was a problem hiding this comment.
There is an existing overload like the one you mention, but it had the issue where, how it was implemented, would remove all notes of the same type, instead of the specific note reference, this will be cleaned up later.
There was a problem hiding this comment.
@LifeHckr, thank you for the clarification! That makes perfect sense - using Array.IndexOf as a workaround to target a specific note instance rather than removing all notes of the same type is a reasonable approach given the current limitation of the overload.
I understand this is temporary technical debt that will be addressed later when the RemoveNote(Note note) overload can be improved to handle single instance removal properly.
/| /|
( ._. )
o_(")(")
Understanding the context!
✏️ Learnings added
Learnt from: LifeHckr
PR: Project-Funk-Engine/ProjectFunkEngine#239
File: Scenes/ShopScene/Scripts/ShopScene.cs:335-338
Timestamp: 2025-06-18T18:27:35.437Z
Learning: In Scenes/ShopScene/Scripts/ShopScene.cs, the RemoveNote method uses Array.IndexOf to find and remove a specific note by index instead of using the existing RemoveNote(Note note) overload because that overload removes all notes of the same type rather than the specific note reference. This is temporary until the overload can be fixed to handle single instance removal.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Refactor
Chores