QuickStart : Editor ship autosave can result in corrupt ship save

[gotmachine]

This :

QuickMods/QuickStart/QS_Persistent.cs

Lines 115 to 130 in ccc6923

	IEnumerator autoSaveShip() {
	QDebug.Log ("autoSaveShip: start", "QPersistent");
	while (HighLogic.LoadedSceneIsEditor && QSettings.Instance.enableEditorAutoSaveShip) {
	QDebug.Log("autoSaveShip: before WaitForSeconds(" + QSettings.Instance.editorTimeToSave + "", "QPersistent");
	yield return new WaitForSeconds (QSettings.Instance.editorTimeToSave);
	List<Part> parts = EditorLogic.fetch.ship != null ? EditorLogic.fetch.ship.Parts : new List<Part>();
	if (parts.Count > 0)
	{
	QDebug.Log("autoSaveShip: before saveShip", "QPersistent");
	ShipConstruction.SaveShip(shipFilename);
	QDebug.Log("autoSaveShip: after saveShip", "QPersistent");
	}
	}
	QDebug.Log ("autoSaveShip: end", "QPersistent");
	}

I'm not sure exactly why, by the call to ShipConstruction.SaveShip() occasionally result in a borked ship save. My guess is that yield return new WaitForSeconds (QSettings.Instance.editorTimeToSave); result in the method being called at a time where the ship / editor isn't a steady state.

Instead of saving the current ship instance, maybe you could get the last backup. Try replacing :

List<Part> parts = EditorLogic.fetch.ship != null ? EditorLogic.fetch.ship.Parts : new List<Part>();

if (parts.Count > 0)
{
  QDebug.Log("autoSaveShip: before saveShip", "QPersistent");
  ShipConstruction.SaveShip(shipFilename);
  QDebug.Log("autoSaveShip: after saveShip", "QPersistent");
}

by

if (ShipConstruction.backups.Count > 0)
{
	ShipConstruction.backups[ShipConstruction.backups.Count - 1].Save(shipFilename);
}

Also, instead of using a coroutine, you could subscribe to GameEvents.onEditorSetBackup.

[linuxgurugamer]

Regarding the coroutine, the GameEvents doesn't do regular backups. The whole idea behind this is to preserve the current vessel.
Using the last backup has the same issue, it isn't current.

So, what is borked? The current vessel, as saved automatically, or the vessel as you saved it.
Might be helpful as well to provide a log file, and to post in the forum where others besides myself can help

[gotmachine]

Using the last backup has the same issue, it isn't current

ShipConstruction.backups is the list of ship saves that KSP use for undo/redo, a new entry is added on all modification events, so it is as current as can be. Moreover, using it is more efficient, as you will only write the serialized node to disk, instead of triggering a full serialization and writing it to disk.
Using the GameEvent might actually be a bit heavy on disk I/O (this is called a lot), so my suggestion would be to keep the 30s coroutine, but saving the last backup instead of creating a new save every time.

So, what is borked?

I wish I knew.
I'm using QuickStart on my dev KSP install for Kerbalism. I'm quite brutal with that one, usually killing the process instead of closing KSP. We have a few hooks on the ShipConstruct.Save() stock methods, which don't get properly called for the QuickStart backup, sometimes. It is very occasional, I would say it happens in 1 out of 5 cases.

It might also be an issue on my side (that Kerbalism code is a bit order-of-execution sensitive), but I have tested/debugged that code a lot, including with various mods that are calling it in non-stock ways, and this only happens with the craft saved by QuickStart. My guess is that the coroutine occasionally execute while the editor/ship is in a transient state (shipIds affectations maybe ?).

That's why the coroutine smells fishy to me.
yield return new WaitForSeconds (QSettings.Instance.editorTimeToSave);
runs in between FixedUpdate and Update. I would suggest at least adding a
yield return new WaitForEndOfFrame();
line after that, to make sure everything is in a steady state.

[linuxgurugamer]

Don't take this wrong, you are the first one to ever report this. So I have to suspect kerbalism in this.
Your suggestion about adding a new yield is reasonable, if you like, I can make a quick build with that in there for you to test
Let me know, I can get to it this afternoon if you want it

[linuxgurugamer]

If I don't hear anything from you about this in the next few days, I'll assume it's been resolved.

[gotmachine]

It's resolved on my end by just disabling the editor autosave feature, as I don't want that mod to mess with the vanilla editor state when I'm testing my code (even if it does nothing wrong).

I was just providing some feedback : I think that feature is badly implemented, and I'm pretty sure it's the cause of the issue I had.

I doubt that issue will cause any problem in stock, but mods relying on the vanilla editor state when saving may eventually encounter some similar weirdness. I would indeed, at least, add the yield return new WaitForEndOfFrame();, yup.

[linuxgurugamer]

I've reread your whole comment, and have made the two changes suggested.
Would you like to test them, to see if it solves the problem? It isn't a problem for me, so I don't know if this would fix it.
I'd appreciate it if you could, the file is here:
https://www.dropbox.com/s/lpv2sf66cs5bgbz/QuickStart.zip?dl=0

Let me know, thanks