Save logging + Fixes#473
Merged
Merged
Conversation
Added admin verb to the server tab to allow changing at runtime the error tolerance for saving the world. Allow skipping over recoverable and unrecoverable error if wanted. Meant to be a sort of last-resort tool to force a save if something on the map breaks the save in a limited way.
Renamed and moved the code for saving in SSpersistence over to it's own file since it was pretty massive and hard to navigate.
* Also renamed "DeserializeOneOff" to "LoadFromLimbo" to match the other limbo proc names.
PsyCommando
added
🆕 enhancement
PsyCommando
force-pushed
the
save-logging
branch
from
0511188 to
c9bfaf2
Compare
* Renamed and duplicated exception filters, one for saving, and one for loading. Since loading doesn't need to unblock entering or restart subsystems on an exception. * Cut up loading code after applying some of the fixes upstream.
This reverts commit cee6ea4.
PsyCommando
force-pushed
the
save-logging
branch
from
c9bfaf2 to
1d476d8
Compare
NataKilar
reviewed
Sep 25, 2023
|
|
||
| var/datum/V | ||
| try | ||
| V = _list[key] //#FIXME: We really need to get rid of this awful way to check list types. |
| //!! - HOT CODE BELOW - !! | ||
| //!!!!!!!!!!!!!!!!!!!!!!!! | ||
| for(var/z in saved_levels) | ||
| var/datum/persistence/load_cache/z_level/z_level = z_transform["[z]"] //#FIXME: String concatenation are extremely slow!!! |
Collaborator
There was a problem hiding this comment.
True, but this is only in the saved_levels loop. Creating the string as a key here isn't going to cause an issue.
| if(last_area_type != TA.type || last_area_name != TA.name) | ||
| if(area_turf_count > 0) | ||
| z_level.areas += list(list("[last_area_type]", sanitize_sql(last_area_name), area_turf_count)) | ||
| last_area_type = TA.type //#FIXME: If last_area_type is only ever used as a string, might as well concat it here, instead of doing it thousands of time above? |
Collaborator
There was a problem hiding this comment.
The string concatenation should only occur at the boundaries of two areas, when last_area_type changes. Changing last_area_type to a string would be slower, as you'd need to convert TA.type to a string for comparison, which would occur thousands of times.
| var/atom/old_loc // Where the ship was prior to saving. Used to relocate the ship following saving, not on load. | ||
|
|
||
| //#FIXME: Don't store this here. | ||
| //#FIXME: Don't store this here. Those are meant to be strictly just temporary objects just to show a position on the overmap! |
Collaborator
There was a problem hiding this comment.
I don't think these should be regarded as temporary, since ship velocity etc. are stored on this object.
Collaborator
Author
There was a problem hiding this comment.
The thing is, that's what neb has been going for last time I asked. That's part of why most of the planet code was decoupled from the markers.
Collaborator
There was a problem hiding this comment.
Not blocking, but it's not like Nebula has saving either. There's nothing in game that creates and destroys these objects, so I'm not sure how they could be regarded as temporary.
| . = ..() | ||
| CRASH("obj/debug/error_on_save: Crashing save!") | ||
|
|
||
| // |
level_data.setup_level_data() is not called after loading level data from a save. But that's where it was adding itself to the list of saved levels. (which really should have stayed inside ssmapping for this very reason)
Silently catching it means we don't get a call stack and nice things like that. So, I'm rethrowing it for the runtime log to pick it up.
* Added the persistence config options to the example config file. * Renamed an ambiguous config options for persistence.
Collaborator
Author
|
Added some more things while waiting. Now persistence config options are actually parsed from the config files via hooks to the modpack. |
Removes dependency on the persistence modpack.
NataKilar
approved these changes
Sep 30, 2023
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
2 participants
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.
Description of changes
Fixes:
Changes: