[DRAFT] Autosave by default

[shihan42]

These are the first steps to solve #22
Right now, I'd like to get some feedback on two possible pitfalls/problems. That's why this is a draft PR.

The first problem arises in regard to proper async methods:
All the methods that are responsible for saving the project are converted to support async I/O, but we can't have that yet. The whole UI framework doesn't use the async/await pattern. This pattern commonly needs "async all the way", starting at the UI events and ending in the method that for example use async file I/O.

I'm not sure if it is feasible or even a good idea at this point to rewrite the UI system. That could be a lot of work with possibly low benefits. But on the other hand, saving even big production sheets never exceeded a handful of milliseconds on my machine. So we could just ignore this (as I do in Project.QueueAutosave()). But tests with big real life examples would be nice.

The second problem is the need to manually include the call to autosave at every occasion where we want the autosave to trigger (if it is enabled).

At first I tried to attach the autosave to the undo system. But that didn't really make sense, as the undo system registers the status before a change. With a typical call such as project.RecordUndo().pages.Add(page); the status of the displayed pages is only changed after the undo.

Shenanigans with deferred execution after a call to RecordUndo() are more a hack than an actual solution and introduce synchronization issues over threads.

Any ideas how we should proceed?

[shihan42]

No comments? No ideas? No opinions?

[shpaass]

Since @veger and @SWeini are busy, I'll share what I know.

I'm not sure if it is feasible or even a good idea at this point to rewrite the UI system.

It would take a ton of effort to rewrite the whole UI, so we should consider thoroughly if we want to do it. The chances that we want to do it are very low.

we could just ignore [the async problem]

I'm okay with this. If the current implementation didn't bother with that, and it works, then it will likely continue to work. Although that's a bit risky, true.

Regarding to where attach the save-trigger, I don't know. It needs futher investigation.

[SWeini]

1. Just don't use async/await

Yes, there is an async API for file access, and there are certainly cases when it will help keeping the UI responsive (large files, accessing network drives, spin-up of idle hard disks), but none of that are relevant for YAFC autosave in my opinion.

Introducing async in UI is not an easy task. It's complex to keep the UI responsive while still maintaining integrity of the data and preventing the user from making conflicting changes. the very low-level UI programming done all over the place will probably make this even more complex than it would be otherwise. Doing so is equivalent to a full rewrite. Definitely don't recommend doing anytime soon.

2. There are a few possible solutions for auto-saving, all of them have their pros and cons.

a) Add an explicit method call at every place that modifies the data. This shouldn't be too many places. Search for usages of the undo system and you'll have all of them. This keeps undo/auto-save two separate features. However, in future changes bugs are more likely to happen if doing changes to one system and forgetting to keep the other in sync.

b) Change the API of the undo system to something more similar to an transaction system. The classes UndoSnapshot/UndoBatch are a good start, just needs a different interface to use.

var transaction = model.StartTransaction();
model.MakeSomeChanges(foo, bar);
transaction.Commit();

Then it's extremely easy to add the auto-save in the commit step.

c) As hinted by you, trigger the auto-save by the undo system, but defer it. There is UI.DispatchInMainThread, which should do the trick for thread synchronization.

d) Auto-save based on a timer, instead of after each change. For large files it might be the better solution performance-wise. Especially if the user is just undoing/redoing a lot of changes quickly, an immediate auto-save is a waste of performance, but required to meet user expectations.

3. Additional Notes

Personally, I don't care. I won't use the auto-save feature because I regularly just play around and explicitly don't want to save my changes. So any solution that doesn't massively deteriorate the code quality will get my approval.

Make sure to have a proper "backup" system for autosave. Don't overwrite the original file, instead create separate autosave files. At least write to a new file and perform a rename afterwards. Otherwise it's too easy to lose the data.

[SWeini]

FYI: I can give you my 3.5 MB pyanodon yafc file for testing performance. I'm still on some older version, so getting it to run flawlessly needs some more detailed instructions.

[veger]

Holidays here, so the kids take quite a bunch of my time...

I am not a C# expert (I only did a pet project here and there with it), so these kind of low-level, language specific things are not for me to comment on. I do have some generic advice/feedback:

Are save times even a bottleneck? When I save my py yafc file I don't even notice it (must admit it is an order of magnitude smaller than SWeini's one). If it is not an issue: don't try to fix it by throwing complexity to the (non existing) problem.

2 There are a few possible solutions for auto-saving, all of them have their pros and cons.

e) Use some event/trigger system (again not a C# expert but I would assume there is some build-in off-the-self solution) with 'pre-change' and 'post-change' events. Then other parts of the code can benefit from this: undo system, recalculation triggers, auto-save, etc.

Similar as for 2a, use the places of the undo system to add the triggers to fire the events. Maybe accept some lambda input so it is easier to fire the pre and post triggers in a single go.

Personally, I don't care. I won't use the auto-save feature because I regularly just play around and explicitly don't want to save my changes.

Same, so lets agree to make this optional as it would mess up my way of trying things with YAFC: for pyanodon I need to try a lot before I find something that makes sense, and often it is revamping half of my recipes to make it work.

[shihan42]

@SWeini I'd love to make some tests with your large project.

@veger
I'm not sure if there is such an event/trigger system in C#. Can you provide an example in other languages so that I can do some research?

In general, all the points of you both are something to think about. Give me some time for that ;-)

[veger]

I remember seeing such a system in Unity (also C#), but it looks like it is part of their library: UnityEvent

There is 'some event', listeners can be added and whenever something triggers, a function (Invoke() in this case) is called, causing all subscribed listeners to be called.

I know I saw this scheme in other languages, but I do not have a more active memory of using them. But this is what I (quickly) found:

• https://www.geeksforgeeks.org/node-js-events/
• https://stackoverflow.com/questions/605169/how-to-do-events-in-ruby

Basically it just keeps a list of interested parties (listeners) and call them all in a fire/invoke/emit function. This way the system is (more) decoupled as the 'fire logic' does not need to know what needs to get triggered.

[shihan42]

Ok, the event handling won't solve the problem either, since it can't detect when and what happens in a line like this after the call to RecordUndo: settings.RecordUndo().milestones.Add(obj);

I have another approach in mind, but that needs some bigger refactoring (same direction as @SWeinis idea about transactions).

PS: We had another use case for this in the Discord. Someone had a power outage IRL :/
So at least recurring autosaves would be a nice thing. Likes Factorio's, rolling around a ringbuffer...

[shihan42]

What do you people think about this: Autosave is supported in both flavors. Either every x minutes or after each meaningful operation (whatever that is).
Plus the option of no autsave whatsoever...

[shpaass]

Sounds good to me.