Skip to content

Consider missing vanilla savefile corrupted (#649)#899

Merged
Wartori54 merged 3 commits into
EverestAPI:devfrom
DashingCat:649-consider-missing-vanilla-corrupted
Jul 15, 2025
Merged

Consider missing vanilla savefile corrupted (#649)#899
Wartori54 merged 3 commits into
EverestAPI:devfrom
DashingCat:649-consider-missing-vanilla-corrupted

Conversation

@DashingCat

Copy link
Copy Markdown
Contributor

Commit fa9b476 always delete modded save data when starting a new game.

This PR changes this behavior:

  • Modded save data is no longer deleted when starting a new save file.
  • Instead, save slots without vanilla save data and with modded save data are considered corrupted, making the slot unusable until all save data (vanilla and modded) is deleted.

This allows the Everest.Events.FileSelectSlot.OnCreateButtons event to work properly on new save files, as modded save data is no longer deleted after being created.

Fixes #649.

@Wartori54

Copy link
Copy Markdown
Member

I have a minor issue with this, which is: in the case where users have deleted their savefiles in vanilla, once they hop on Everest again, they would see like their deleted savefiles appear as corrupted. Which could lead to confusion, especially once this hits stable, where potentially multiple blank savefiles may become corrupted unexpectedly.
I think it could be better to convey to the user what's actually happening, given that the files aren't really corrupted, just partially deleted. So replacing the text when a savefile is partially deleted from "Corrupted" to something like "Partially deleted" would be much better.

@maddie480 maddie480 added the 1: review needed This PR needs 2 approvals to be merged (bot-managed) label May 18, 2025
@DashingCat

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback, I clarified the text in the latest commit, here is a screenshot below:

image

@Wartori54 Wartori54 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested it and it seems to work, it just needs a few logging changes.

Comment thread Celeste.Mod.mm/Patches/OuiFileSelect.cs Outdated

for (int i = 0; i < Slots.Count() - 1; i++) { // - 1, last slot is always empty
if (!Slots[i].Exists && Directory.GetFiles(saveFilePath, $"{i}-mod*.celeste").Length > 0) {
Logger.Warn("save", $"Save slot {i} have modded data but no save file, flagging as corrupted");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence is a bit wrong, something like "Save slot {i} has modded data but no vanilla save data, flagging as corrupted" would be better.

Also using save as the log id seems a bit vague, OuiFileSelect or SaveData could be better.

Comment thread Celeste.Mod.mm/Patches/SaveData.cs
Comment thread Celeste.Mod.mm/Patches/SaveData.cs Outdated
if (!orig_TryDelete(slot))
return false;
else
Logger.Warn("save", $"Deleting save data for slot {slot} which have no vanilla data");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above for save. And here the "have" should be a "has".

@maddie480-bot maddie480-bot added 2: changes requested This PR cannot be merged because changes were requested (bot-managed) and removed 1: review needed This PR needs 2 approvals to be merged (bot-managed) labels Jun 1, 2025
@DashingCat

Copy link
Copy Markdown
Contributor Author

Thanks for the review, I fixed the if statement and log wording in the latest commit.

@maddie480-bot maddie480-bot added 1: review needed This PR needs 2 approvals to be merged (bot-managed) and removed 2: changes requested This PR cannot be merged because changes were requested (bot-managed) labels Jun 12, 2025
@maddie480-bot

Copy link
Copy Markdown
Member

The pull request was approved and entered the 3-day last-call window.
If no further reviews happen, it will end on Jul 13, 2025, 12:00 AM UTC, after which the pull request will be able to be merged.

@maddie480-bot maddie480-bot added 3: last call window This PR was approved, and is in the 5-day last-call window before getting merged (bot-managed) and removed 1: review needed This PR needs 2 approvals to be merged (bot-managed) labels Jul 7, 2025
@maddie480-bot

Copy link
Copy Markdown
Member

The last-call window for this pull request ended. It can now be merged if no blockers were brought up.

@maddie480-bot maddie480-bot added 4: ready to merge This PR was approved and the last-call window is over (bot-managed) and removed 3: last call window This PR was approved, and is in the 5-day last-call window before getting merged (bot-managed) labels Jul 12, 2025
@Wartori54 Wartori54 merged commit 2ec0d93 into EverestAPI:dev Jul 15, 2025
1 check passed
maddie480 added a commit to maddie480/Everest that referenced this pull request Aug 9, 2025
maddie480 added a commit that referenced this pull request Aug 9, 2025
Revert "Consider missing vanilla savefile corrupted (#649) (#899)"
maddie480 added a commit that referenced this pull request Aug 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4: ready to merge This PR was approved and the last-call window is over (bot-managed)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Events.FileSelectSlot.OnCreateButtons not applying settings for new saves

5 participants