Skip to content

Conversation

@minacode
Copy link
Contributor

This PR aims to implement a simple interface to store and load program state to the memory.

Why is this a good idea?

Because it provides a simple interface for developers and reduces the amount of IO-code people have to write. Writing persistent apps would only be a matter of defining the persistent data struct, inheriting from Persistent and loading/saving the data in the constructor/desconstructor (and maybe we can automate this away, not sure though).

What is already there?

In utility/Persistent.h is a new template class Persistent<T>.
It holds a versioned struct of data which it can save to and load from the memory. The code is taken from the settings implementation.
For testing/reference I changed the score of the Paddle game to be persistent.

What is missing?

Currently, I have a linker error I cannot get rid of. It's probably easy for someone who knows more about C++ than me.
Therefore it's not tested either.

this aims at providing a simple interface to store and load program state to the memory.
@github-actions
Copy link

Build checks have not completed. Possible reasons for this are:

  1. The checks need to be approved by a maintainer
  2. The branch has conflicts
  3. The firmware build has failed

@mark9064
Copy link
Member

Love this concept! I'll see if I can look at the linker error sometime soon, this week is looking busy for me though :(

@mark9064 mark9064 added the maintenance Background work label Nov 24, 2024
@owenfromcanada
Copy link
Contributor

owenfromcanada commented Jun 25, 2025

Hey there, I was asking similar questions (#2321), and was pointed here. I think I found the build error:

Line 32 of Persistent.h attempts to invoke a virtual method in the constructor of the template class. It looks like you can't do that there. I'm not a C++ expert, so I can't say for sure, but commenting out that line allowed the code to build (and the load method is called during the subclass instantiation, so it shouldn't make any functional difference).

I don't know if I can submit a change to this PR (I'm not a git/hub expert either), so hopefully this helps.

A couple other things that I found while looking at this:

  1. If a class defines GetPersistencePath() to return a path with a subdirectory that doesn't exist, the load and save functions don't work. I'm guessing you'd need to explicitly create any subdirectories first.
  2. You might want to change the return type of LoadPersistentState() to bool to indicate whether the file was found and was valid. That way the subclass can use default values or take some other action (technically you could do this by assigning default values before calling LoadPersistentState() but that seems less clear). While you're at it, it might be worth making SavePersistentState() return a boolean as well.
  3. The function definitions should probably be in a separate C file rather than in the header.

Another thing to consider: does this make more sense as a class template, or should the persistent data itself be the templated class? I ask because I can imagine a situation where an app might need access to more than one set of persistent data, or conceivably two apps might want to reference the same persisted data.

@minacode
Copy link
Contributor Author

Thank you, @owenfromcanada! That's a lot of good feedback 🙂

I will think about it and come back with a cleaner solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance Background work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants