search

Synodic-Software / Soul-Engine

Physically based renderer and simulation engine for real-time applications.
GNU General Public License v3.0
42 stars 24 forks source link

Settings Refactor #90

Closed aferritto closed 7 years ago

aferritto commented 7 years ago

Addresses issue #82. I have the core design implemented, but still need to implement ArchiveXML and ArchiveBinary. This should not take too long.

@Behemyth please let me know if you have any requests for me to change the design before I finish and clean up the code. Please do not merge this yet.

Behemyth commented 7 years ago

I added a single comment to the files changed tab. I'm not sure if it would be relevant for this issue fix, but the enum{ null, text, binary etc and the requisite code should be removed from settings and placed in a Transput/Serialization.cpp/.h as the choice should be present here, and not in the creation of user modifiable .ini files. Probs the next issue though.

aferritto commented 7 years ago

Maybe, depending on if you plan to store archives similarly in other modules of the engine in which case I may end up just making an archive wrapper class (and putting the enum there). You can directly access a text archive by using ArchiveText, I just kept the enum in Settings to preserve the API.

So the two main ways I see right now to handle this problem across the engine are:

  1. Create a archive wrapper class which stores a archive base * and creates new archives as necessary (this is what I do in settings).

  2. Change the API so that you have something like Settings::Read<ArchiveText>(filename) instead of Settings(filename,TEXT). This is likely better if you are always switching archive files/types (since it is simple and always makes a new archive file). However if you plan on reusing the same archive file frequently option 1 is likely the way to go since you can just keep the same archive object constructed.

Also, would you want a Settings::DeleteArchive() which deletes the underlying archive, since as implemented after the first read/write call there will always be an archive object in memory?

Behemyth commented 7 years ago

Lets add the delete. The only two modules/applications I can imagine are in serialization and configuration. If you are content with how you already did it, I am too.

aferritto commented 7 years ago

@Behemyth I've finished everything on my end. Now just have to override the original settings tests with the new ones and we're good to merge.