SaveLoad: validate save data before applying it - #81
Open
Goober5000 wants to merge 2 commits into
Open
Conversation
Goober5000
added
fix
The save format is positional, so a truncated or hand-edited line used to cause nil arithmetic inside the splitters. They now log the problem and return nil, and shipdataset skips the ship. An empty weapon class name is not an error: it means the bank was empty when the ship was saved. Saved names are checked before use, since a failed table lookup yields an invalid handle rather than nil. Whatever depends on a name that no longer resolves is skipped rather than applied to the wrong target: an unusable ship class skips the subsystem and weapon sections, which are indexed by position within that class, and an unusable weapon class skips the ammunition, which is scaled by the mounted weapon's cargo size. shipdatacollect consults mn.ShipRegistry first, so an unknown name is skipped instead of reaching the SEXP parser. Every isValid() call site uses the "x and x:isValid()" form, for mods that make invalid handles nil. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Such a ship still has exactly its mission file attributes, so there is nothing to save or restore. It could not be represented either: only live ship handles are read, so it fell through both status probes and was saved as vanished, which vanished it on load if it had arrived by then. lua-spawn-and-load-ship hit this every time, making the ship arrive and then immediately vanishing it. shipdatacollect now returns nothing for a not-yet-present registry entry, and load_ship says so when a ship has no saved data. A file written before this change may still hold the vanished status for such a ship, and will still vanish it. The status enum is read through its FSO 26.0 name where the build has one, since the old name raises an error in a mod that targets 26.0 or later. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Goober5000
force-pushed
the
fix/saveload_robustness
branch
3 times, most recently
from
September 5, 2026 07:32
c773837 to
29f76ce
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The save format is positional, so a truncated or hand-edited line used to
cause nil arithmetic inside the splitters. They now log the problem and
return nil, and shipdataset skips the ship. An empty weapon class name is
not an error: it means the bank was empty when the ship was saved.
Saved names are checked before use, since a failed table lookup yields an
invalid handle rather than nil. Whatever depends on a name that no longer
resolves is skipped rather than applied to the wrong target: an unusable
ship class skips the subsystem and weapon sections, which are indexed by
position within that class, and an unusable weapon class skips the
ammunition, which is scaled by the mounted weapon's cargo size.
shipdatacollect consults mn.ShipRegistry first, so an unknown name is
skipped instead of reaching the SEXP parser. Every isValid() call site
uses the "x and x:isValid()" form, for mods that make invalid handles nil.
The save format is positional, so a truncated or hand-edited line used to
cause nil arithmetic inside the splitters. They now log the problem and
return nil, and shipdataset skips the ship. An empty weapon class name is
not an error: it means the bank was empty when the ship was saved.
Saved names are checked before use, since a failed table lookup yields an
invalid handle rather than nil. Whatever depends on a name that no longer
resolves is skipped rather than applied to the wrong target: an unusable
ship class skips the subsystem and weapon sections, which are indexed by
position within that class, and an unusable weapon class skips the
ammunition, which is scaled by the mounted weapon's cargo size.
shipdatacollect consults mn.ShipRegistry first, so an unknown name is
skipped instead of reaching the SEXP parser. Every isValid() call site
uses the "x and x:isValid()" form, for mods that make invalid handles nil.