-
Notifications
You must be signed in to change notification settings - Fork 0
Refined saves #248
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refined saves #248
Conversation
Moved HasWon and FirstTime(renamed TutorialDone) to StageProducer
Removed some commented code Added _ to specify ignored parameters
WalkthroughThis update introduces a new persistent save system by adding the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Game
participant Configkeeper
participant Savekeeper
User->>Game: Start game / perform actions
Game->>Configkeeper: Get/Update config values (e.g., input, language)
Game->>Savekeeper: RecordSave() / LoadFromFile()
Savekeeper-->>Game: Save/load persistent and run data
Game->>Savekeeper: Update persistent values (e.g., tutorial, win state)
Possibly related PRs
Suggested reviewers
Poem
✨ 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
Classes/Events/EventDatabase.cs (1)
52-61: Potential bug: Missing outcome description assignment.The lambda calculates
localizedNamebut no longer assigns it toself.OutcomeDescriptions[0]. This means the outcome message won't show which item was stolen, only the money amount will be displayed.Was this assignment removal intentional? If not, restore it:
(self, _) => { string stolenMoney = (StageProducer.PlayerStats.Money / 2).ToString(); StageProducer.PlayerStats.Money /= 2; + self.OutcomeDescriptions[0] = string.Format( + TranslationServer.Translate("EVENT_EVENT1_OUTCOME1"), + stolenMoney + ); self.OutcomeDescriptions[2] = string.Format( TranslationServer.Translate("EVENT_EVENT1_OUTCOME3"), stolenMoney ); },Globals/Configkeeper.cs (1)
149-149: Fix error message to use correct class name.The error message still references the old class name.
- GD.PushError("SaveSystem.UpdateConfig: Invalid config setting passed. " + setting); + GD.PushError("Configkeeper.UpdateConfig: Invalid config setting passed. " + setting);
🧹 Nitpick comments (2)
Scenes/EventScene/EventScene.cs (1)
43-44: Optional cleanup: Remove redundant null check.Since
EventPoolis now initialized to an empty list at declaration, the null check is redundant.- if (EventPool == null || EventPool.Count == 0) + if (EventPool.Count == 0) RefreshPool();Globals/Configkeeper.cs (1)
8-9: Update class documentation to reflect new responsibilities.The comment still mentions "save files", but this class now only handles configuration.
/** - * <summary>SaveSystem: Manages FileI/O of configs and save files.</summary> + * <summary>Configkeeper: Manages FileI/O of user configuration settings.</summary> */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Scenes/NoteManager/Assets/NoteWithOutline.pngis excluded by!**/*.png
📒 Files selected for processing (31)
Classes/Events/EventDatabase.cs(4 hunks)Classes/MidiMaestro/MidiMaestro.cs(0 hunks)Globals/Configkeeper.cs(1 hunks)Globals/Configkeeper.cs.uid(1 hunks)Globals/Savekeeper.cs(1 hunks)Globals/Savekeeper.cs.uid(1 hunks)Globals/Scribe.cs(2 hunks)Globals/StageProducer.cs(6 hunks)Scenes/BattleDirector/Scripts/BattleDirector.cs(1 hunks)Scenes/BattleDirector/Scripts/NotePlacementBar.cs(1 hunks)Scenes/BattleDirector/Tutorial/Toriel.cs(2 hunks)Scenes/CustomSong/CustomScore.cs(2 hunks)Scenes/EventScene/EventScene.cs(1 hunks)Scenes/Maps/Scripts/Cartographer.cs(2 hunks)Scenes/NoteManager/Assets/NoteWithOutline.png.import(1 hunks)Scenes/NoteManager/Scripts/InputHandler.cs(3 hunks)Scenes/Puppets/Enemies/BossBlood/P_BossBlood.cs(1 hunks)Scenes/Puppets/Enemies/CyberFox/P_CyberFox.cs(1 hunks)Scenes/Puppets/Enemies/Effigy/P_Effigy.cs(3 hunks)Scenes/Puppets/Enemies/Keythulu/P_Keythulu.cs(3 hunks)Scenes/Puppets/Enemies/Strawman/P_Strawman.cs(7 hunks)Scenes/Puppets/Enemies/Turtle/P_Turtle.cs(1 hunks)Scenes/ShopScene/Scripts/ShopScene.cs(2 hunks)Scenes/UI/Options/HowToPlay.tscn(1 hunks)Scenes/UI/Options/Scripts/HowToPlay.cs(1 hunks)Scenes/UI/Options/Scripts/LanguageSelection.cs(1 hunks)Scenes/UI/Options/Scripts/OptionsMenu.cs(4 hunks)Scenes/UI/Remapping/ControlSettings.cs(4 hunks)Scenes/UI/TitleScreen/Scripts/TitleScreen.cs(2 hunks)Scenes/UI/TitleScreen/TitleScreenEffects.tscn(1 hunks)project.godot(1 hunks)
💤 Files with no reviewable changes (1)
- Classes/MidiMaestro/MidiMaestro.cs
🔇 Additional comments (44)
Globals/Scribe.cs (2)
718-718: Good standardization of resource path.Adding the "res://" prefix aligns with Godot's resource loading conventions and makes the path explicit and deployment-agnostic.
803-804: Good consistency improvement.Simplifying the constructor calls to use positional arguments makes these entries consistent with all other
SongTemplateentries in the array, reducing visual noise while maintaining the same functionality.Also applies to: 808-809
Scenes/NoteManager/Assets/NoteWithOutline.png.import (2)
6-6: Fixed import path casing.
Updatedpathto correctly use "NoteWithOutline.png", matching the actual filename and preventing missing asset errors during import.
13-14: Updated asset dependency entries.
Adjustedsource_fileanddest_filesto reference the corrected "NoteWithOutline.png" path, ensuring dependencies remain in sync with the import update.Globals/Configkeeper.cs.uid (1)
1-1: Add unique identifier for Configkeeper.
The new UID fileGlobals/Configkeeper.cs.uidcorrectly associates with the refactoredConfigkeeperclass for configuration management.Globals/Savekeeper.cs.uid (1)
1-1: Add unique identifier for Savekeeper.
The new UID fileGlobals/Savekeeper.cs.uidenables global registration of theSavekeepersingleton for game state persistence.Scenes/UI/TitleScreen/TitleScreenEffects.tscn (1)
4-4: Correct resource path for outline texture.
Fixed capitalization in thepathattribute to match the actualNoteWithOutline.pngasset, preventing lookup failures at runtime.Scenes/UI/Options/Scripts/LanguageSelection.cs (1)
28-28: Switched config update to new Configkeeper API.
ReplacedSaveSystem.UpdateConfigwithConfigkeeper.UpdateConfig. Confirm thatConfigkeeperis properly namespaced and thatConfigSettings.LanguageKeyaligns with the new config key.Run the following script to ensure no legacy
SaveSystem.UpdateConfigcalls remain:#!/bin/bash # Verify removal of old SaveSystem.UpdateConfig references rg "SaveSystem\\.UpdateConfig"project.godot (1)
27-27: LGTM!The autoload entry correctly registers the new Savekeeper class as a global singleton following standard Godot conventions.
Scenes/UI/Options/HowToPlay.tscn (1)
10-10: Good catch fixing the filename capitalization.Correcting "NoteWIthOutline.png" to "NoteWithOutline.png" ensures compatibility with case-sensitive filesystems and maintains consistency.
Classes/Events/EventDatabase.cs (1)
20-20: Good use of underscore for unused parameters.Replacing unused lambda parameters with
_improves code clarity and follows C# conventions.Also applies to: 36-36, 52-52, 179-179, 186-186, 190-190
Scenes/UI/Options/Scripts/HowToPlay.cs (1)
49-49: ```shell
#!/bin/bashLocate the StageProducer class and inspect method definitions
rg "class StageProducer" -n
rg -C2 "UpdatePersistantValues" -n
rg -C2 "UpdatePersistentValues" -n</details> <details> <summary>Scenes/EventScene/EventScene.cs (1)</summary> `29-29`: **Good defensive programming with list initialization.** Initializing `EventPool` to an empty list ensures it's never null, preventing potential null reference exceptions. </details> <details> <summary>Scenes/Puppets/Enemies/BossBlood/P_BossBlood.cs (1)</summary> `40-40`: **LGTM: Good use of discard parameters.** The lambda parameter simplification correctly uses underscore placeholders for unused parameters, improving code readability and following C# conventions. </details> <details> <summary>Scenes/BattleDirector/Scripts/BattleDirector.cs (1)</summary> `391-391`: **LGTM: API migration to new save system.** The replacement of `SaveSystem.ClearSave()` with `Savekeeper.ClearRun()` correctly aligns with the new architecture that separates configuration management from game state persistence. </details> <details> <summary>Scenes/Puppets/Enemies/Effigy/P_Effigy.cs (1)</summary> `23-23`: **LGTM: Consistent lambda parameter simplification.** All three lambda expressions correctly use underscore placeholders for unused parameters, maintaining consistency with the refactoring pattern applied across enemy puppet files. Also applies to: 38-38, 47-47 </details> <details> <summary>Scenes/Puppets/Enemies/CyberFox/P_CyberFox.cs (1)</summary> `39-39`: **LGTM: Proper use of discard parameters.** The lambda parameter simplification correctly identifies and marks unused parameters with underscores, consistent with the refactoring pattern applied across all enemy puppet files. </details> <details> <summary>Scenes/BattleDirector/Tutorial/Toriel.cs (1)</summary> `61-63`: **LGTM: Consistent API migration to Configkeeper.** Both instances correctly migrate from `SaveSystem` to `Configkeeper` for configuration access, maintaining the same functionality while aligning with the new architecture that separates configuration management from save data persistence. Also applies to: 73-75 </details> <details> <summary>Scenes/Puppets/Enemies/Turtle/P_Turtle.cs (2)</summary> `29-29`: **Good practice: Using underscore for unused parameter.** The change from `eff` to `_` clearly indicates this parameter is unused, improving code readability and following C# conventions. --- `34-34`: **Clean simplification of method call.** Removing the explicit `this` reference makes the code cleaner while maintaining the same functionality. </details> <details> <summary>Scenes/Puppets/Enemies/Keythulu/P_Keythulu.cs (3)</summary> `45-45`: **Good practice: Using underscores for all unused parameters.** All lambda parameters are correctly marked as unused, improving code clarity. --- `66-66`: **Appropriate parameter usage in lambda.** Only the first parameter `e` is used, while the others are correctly marked as unused. --- `79-79`: ```shell #!/bin/bash # Search for definition of PersistKeys enum in StageProducer rg -n "enum PersistKeys" -A 10 # Search for the UpdatePersistantValues method signature rg -n "UpdatePersistantValues" -C 5 # Search for any occurrences of HasWon across the repo rg -n "HasWon"Scenes/UI/TitleScreen/Scripts/TitleScreen.cs (2)
51-52: Consistent migration to persistent values system.The change from config-based
HasWonflag to persistent value lookup is consistent with the pattern used throughout the codebase. The integer comparison== 1aligns with how the value is set in other files.
33-34: Appropriate migration to new save/config system.The changes correctly separate save data clearing (
Savekeeper.ClearRun()) from configuration clearing (Configkeeper.ClearConfig()), aligning with the new system architecture.#!/bin/bash # Description: Verify Savekeeper and Configkeeper classes exist with expected methods # Expected: Find class definitions and ClearRun/ClearConfig methods # Search for Savekeeper class and ClearRun method ast-grep --pattern 'class Savekeeper { $$$ }' ast-grep --pattern 'ClearRun($_) { $$$ }' # Search for Configkeeper class and ClearConfig method ast-grep --pattern 'class Configkeeper { $$$ }' ast-grep --pattern 'ClearConfig($_) { $$$ }'Scenes/ShopScene/Scripts/ShopScene.cs (2)
346-346: Excellent improvement: Dynamic heal amount calculation.Converting from a fixed field to a calculated property ensures the heal amount stays proportional to the player's current max health, making the healing more responsive to gameplay progression.
369-369: Proper usage of the new dynamic property.The heal method correctly uses the new
HealAmountproperty, ensuring the healing scales with the player's current max health.Scenes/Puppets/Enemies/Strawman/P_Strawman.cs (2)
25-25: Consistent lambda parameter simplification across battle effects.All unused parameters are properly marked with underscores, improving code readability and following C# conventions consistently across all battle effect handlers.
Also applies to: 35-35, 56-56, 70-70, 86-86, 95-95
104-107: Proper migration to persistent values system for tutorial completion.The change from
SaveSystem.UpdateConfigwith "FirstTime" flag toStageProducer.UpdatePersistantValueswithTutorialDonekey is consistent with the new save system architecture. The achievement call remains appropriately unchanged.#!/bin/bash # Description: Verify TutorialDone persistent key is defined and used consistently # Expected: Find TutorialDone key definition and related usage # Search for TutorialDone key definition in StageProducer ast-grep --pattern 'TutorialDone' # Search for all tutorial-related persistent value operations rg -A 3 -B 3 "TutorialDone"Scenes/UI/Options/Scripts/OptionsMenu.cs (3)
47-49: LGTM!Configuration retrieval correctly updated to use
Configkeeper.
59-61: Good refactoring of persistent game state.The tutorial completion check correctly uses the new persistent value system via
StageProducerinstead of storing game state in configuration.
121-121: LGTM!All configuration updates correctly migrated to use
Configkeeper.Also applies to: 135-135, 141-141, 147-147
Scenes/BattleDirector/Scripts/NotePlacementBar.cs (1)
233-235: LGTM!Configuration retrieval correctly updated to use
Configkeeper.Scenes/UI/Remapping/ControlSettings.cs (3)
154-157: LGTM!Configuration retrieval correctly updated to use
Configkeeper.
254-257: LGTM!Configuration updates correctly migrated to use
Configkeeper.Also applies to: 480-480
393-440: Good type consistency update.The
ConfigMapdictionary type correctly updated to useConfigkeeper.ConfigSettingsenums throughout.Scenes/Maps/Scripts/Cartographer.cs (2)
41-41: LGTM!Save recording correctly migrated to use
Savekeeper.
77-80: Good separation of concerns.Tutorial completion check correctly uses the persistent value system, and configuration update uses
Configkeeper.Scenes/CustomSong/CustomScore.cs (1)
31-63: Good refactoring of event handler lifecycle management!Storing the event handlers as fields and properly unsubscribing them in
_ExitTreeprevents memory leaks from dangling subscriptions. This is a solid improvement to the code.Scenes/NoteManager/Scripts/InputHandler.cs (2)
68-74: Configuration system migration looks good.The migration from
SaveSystemtoConfigkeeperis correctly implemented and consistent with the project-wide refactoring.Also applies to: 97-97
132-143: Nice refactoring of arrow sprite loading logic!Loading textures once and applying rotations via a loop reduces code duplication and improves maintainability.
Globals/StageProducer.cs (1)
37-40: Proper integration of the new save system.Good use of event subscription pattern for save serialization. Loading from file on initialization ensures persistent data is available early.
Globals/Savekeeper.cs (2)
47-51: Good use of deferred execution for save operations.Using
CallDeferredensures the save operation doesn't interfere with the current frame processing, which is a best practice in Godot.
104-112: I've not yet located theDelimiterdefinition to confirm its intended value and usage. Let’s find where it’s declared:#!/bin/bash # Locate the Delimiter constant across the repo rg -n "Delimiter" -C3 .
| private bool DeserializeRun() //TODO: This is really verbose and bad. | ||
| { | ||
| if (!Savekeeper.GameSaveObjects.ContainsKey(Savekeeper.DefaultRunSaveHeader)) | ||
| return false; | ||
| int idx = 0; | ||
| string loadRun = Savekeeper.GameSaveObjects[Savekeeper.DefaultRunSaveHeader]; | ||
|
|
||
| var ulongSuccess = Savekeeper.Parse<ulong>( | ||
| RunSaveValues.RngSeed.ToString(), | ||
| loadRun, | ||
| idx, | ||
| ulong.TryParse | ||
| ); | ||
| if (!ulongSuccess.Success) | ||
| return false; | ||
| GlobalRng.Seed = ulongSuccess.Value; | ||
| idx = ulongSuccess.NextIdx; | ||
|
|
||
| var intSuccess = Savekeeper.Parse<int>( | ||
| RunSaveValues.Area.ToString(), | ||
| loadRun, | ||
| idx, | ||
| int.TryParse | ||
| ); | ||
| if (!intSuccess.Success) | ||
| return false; | ||
| CurLevel = MapLevels.GetLevelFromId(intSuccess.Value); | ||
| idx = intSuccess.NextIdx; | ||
|
|
||
| var bPoolSuccess = Savekeeper.ParseArray<int>( | ||
| RunSaveValues.BattlePool.ToString(), | ||
| loadRun, | ||
| idx, | ||
| int.TryParse | ||
| ); | ||
| if (bPoolSuccess.Success) | ||
| { | ||
| BattlePool = bPoolSuccess.Value.ToList(); | ||
| idx = bPoolSuccess.NextIdx; | ||
| } | ||
| else | ||
| { | ||
| GD.PushWarning("Could not parse battle pool!"); | ||
| BattlePool = []; | ||
| } | ||
|
|
||
| var ePoolSuccess = Savekeeper.ParseArray<int>( | ||
| RunSaveValues.EventPool.ToString(), | ||
| loadRun, | ||
| idx, | ||
| int.TryParse | ||
| ); | ||
| if (ePoolSuccess.Success) | ||
| { | ||
| EventScene.EventPool = ePoolSuccess.Value.ToList(); | ||
| idx = ePoolSuccess.NextIdx; | ||
| } | ||
| else | ||
| { | ||
| GD.PushWarning("Could not parse event pool!"); | ||
| EventScene.EventPool = []; | ||
| } | ||
|
|
||
| GenerateMapConsistent(); | ||
|
|
||
| ulongSuccess = Savekeeper.Parse<ulong>( | ||
| RunSaveValues.RngState.ToString(), | ||
| loadRun, | ||
| idx, | ||
| ulong.TryParse | ||
| ); | ||
| if (!ulongSuccess.Success) | ||
| return false; | ||
| GlobalRng.State = ulongSuccess.Value; | ||
| idx = ulongSuccess.NextIdx; | ||
|
|
||
| intSuccess = Savekeeper.Parse<int>( | ||
| RunSaveValues.LastRoomIdx.ToString(), | ||
| loadRun, | ||
| idx, | ||
| int.TryParse | ||
| ); | ||
| if (!intSuccess.Success) | ||
| return false; | ||
| CurRoom = intSuccess.Value; | ||
| idx = intSuccess.NextIdx; | ||
|
|
||
| Scribe.InitRelicPools(); | ||
| PlayerStats = new PlayerStats(); | ||
| PlayerStats.CurNotes = []; | ||
|
|
||
| var noteSuccess = Savekeeper.ParseArray<int>( | ||
| RunSaveValues.NoteIds.ToString(), | ||
| loadRun, | ||
| idx, | ||
| int.TryParse | ||
| ); | ||
| if (!noteSuccess.Success) | ||
| return false; | ||
| foreach (int noteId in noteSuccess.Value) | ||
| { | ||
| PlayerStats.AddNote(Scribe.NoteDictionary[noteId]); | ||
| } | ||
| idx = noteSuccess.NextIdx; | ||
|
|
||
| var relicSuccess = Savekeeper.ParseArray<int>( | ||
| RunSaveValues.RelicIds.ToString(), | ||
| loadRun, | ||
| idx, | ||
| int.TryParse | ||
| ); | ||
| if (!relicSuccess.Success) | ||
| return false; | ||
| foreach (int relicId in relicSuccess.Value) | ||
| { | ||
| PlayerStats.AddRelic(Scribe.RelicDictionary[relicId]); | ||
| } | ||
| idx = relicSuccess.NextIdx; | ||
|
|
||
| intSuccess = Savekeeper.Parse<int>( | ||
| RunSaveValues.PlayerHealth.ToString(), | ||
| loadRun, | ||
| idx, | ||
| int.TryParse | ||
| ); | ||
| if (!intSuccess.Success) | ||
| return false; | ||
| PlayerStats.CurrentHealth = intSuccess.Value; | ||
| idx = intSuccess.NextIdx; | ||
|
|
||
| intSuccess = Savekeeper.Parse<int>( | ||
| RunSaveValues.Money.ToString(), | ||
| loadRun, | ||
| idx, | ||
| int.TryParse | ||
| ); | ||
| if (!intSuccess.Success) | ||
| return false; | ||
| PlayerStats.Money = intSuccess.Value; | ||
| idx = intSuccess.NextIdx; | ||
|
|
||
| intSuccess = Savekeeper.Parse<int>( | ||
| RunSaveValues.Shortcuts.ToString(), | ||
| loadRun, | ||
| idx, | ||
| int.TryParse | ||
| ); | ||
| if (!intSuccess.Success) | ||
| return false; | ||
| PlayerStats.Shortcuts = intSuccess.Value; | ||
| idx = intSuccess.NextIdx; | ||
|
|
||
| intSuccess = Savekeeper.Parse<int>( | ||
| RunSaveValues.PlayerMaxCombo.ToString(), | ||
| loadRun, | ||
| idx, | ||
| int.TryParse | ||
| ); | ||
| if (!intSuccess.Success) | ||
| return false; | ||
| PlayerStats.MaxComboBar = intSuccess.Value; | ||
|
|
||
| return true; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider refactoring the verbose deserialization logic.
The DeserializeRun method contains highly repetitive parsing patterns. Each field follows the same pattern: parse, check success, update index. This could be significantly simplified using a helper method or reflection-based approach.
Consider creating a helper method to reduce repetition:
+private bool ParseAndAssign<T>(string key, string data, ref int idx, TryParseHandler<T> parser, Action<T> assign)
+{
+ var result = Savekeeper.Parse<T>(key, data, idx, parser);
+ if (!result.Success)
+ return false;
+ assign(result.Value);
+ idx = result.NextIdx;
+ return true;
+}Then use it like:
-var ulongSuccess = Savekeeper.Parse<ulong>(
- RunSaveValues.RngSeed.ToString(),
- loadRun,
- idx,
- ulong.TryParse
-);
-if (!ulongSuccess.Success)
- return false;
-GlobalRng.Seed = ulongSuccess.Value;
-idx = ulongSuccess.NextIdx;
+if (!ParseAndAssign<ulong>(RunSaveValues.RngSeed.ToString(), loadRun, ref idx, ulong.TryParse, v => GlobalRng.Seed = v))
+ return false;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In Globals/StageProducer.cs between lines 457 and 620, the DeserializeRun method
is very verbose due to repetitive parsing and success checking for each field.
Refactor by creating a helper method that encapsulates the parse-check-update
pattern to reduce duplication. Replace each individual parse block with calls to
this helper, passing the appropriate parameters, to simplify the code and
improve readability.
| if (value is string s && !s.IsValidFileName()) | ||
| return valName + Delimiter + InvalidFormatString + Delimiter; | ||
|
|
||
| return valName + Delimiter + value + Delimiter; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential issue with invalid string handling.
When a string is invalid (!s.IsValidFileName()), you're storing InvalidFormatString but there's no corresponding handling during deserialization to convert it back or flag it as invalid.
Consider adding validation during parse operations to handle InvalidFormatString:
public static bool StringParse(string value, out string result)
{
+ if (value == InvalidFormatString)
+ {
+ result = null;
+ return false;
+ }
result = value;
return true;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (value is string s && !s.IsValidFileName()) | |
| return valName + Delimiter + InvalidFormatString + Delimiter; | |
| return valName + Delimiter + value + Delimiter; | |
| } | |
| public static bool StringParse(string value, out string result) | |
| { | |
| if (value == InvalidFormatString) | |
| { | |
| result = null; | |
| return false; | |
| } | |
| result = value; | |
| return true; | |
| } |
🤖 Prompt for AI Agents
In Globals/Savekeeper.cs around lines 121 to 125, the code stores
InvalidFormatString when a string is invalid but lacks corresponding handling
during deserialization. To fix this, update the parsing or deserialization logic
to detect InvalidFormatString and handle it appropriately, either by flagging
the value as invalid or converting it back to a recognizable invalid state.
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Style
Chores