Refactoring Save Slot Controllers to work more with Save Slot Views #1014
Conversation
This file contains 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
This reverts commit d4e4808.
Updating develop branch on my end
This reverts commit e03289b.
…Say dialog entries" This reverts commit 5cad400.
Updating the branch on my end
Also did a little reorganizing of what's in the Fungus Save script folder
Thanks to the new view system, they're no longer necessary
All the tests have passed at the time of this writing
CG-Tespy
changed the title
|
@stevehalliwell Finished. Removed some legacy fields and added tests to ensure that the slots do their thing right on their own. I've added to the view classes as well, based on the things the unit tests needed to check for. |
…to SaveDataCreationTesting
Added an Encode func to NumericVarSaveEncoder that can take an IList of vars, returning an IList of said vars encoded. I know that Fungus would normally return just a regular List, but I felt it'd be better to go with IList here due to how it's good practice to code more to interfaces than to concrete implementations. Especially in this case. If the client really wants a regular List of the encoded results, they can easily make their own by passing said results to the regular List's constructor.
One that lets subclasses decide which var types they can support. This should cut down on boilerplate code we'd otherwise make when working on other var encoders.
Also deleted the sound tests, since we're quite a ways off from testing that stuff
FlowchartSavingTests was getting a bit big, and I felt that testing other var type encodings would take similar amounts of code. Hence, I made FlowchartSavingTests an abstract class for various suites that each focus on certain var types or groups. Hence the new FlowchartNumSavingTests suite.
To cut down on code duplication, I created a testing extensions script. I may have to do something about the code duplication in the string var tests as they are now...
Will need to add back in the tests that check to make sure that invalid inputs get rejected
That test suite was set up only for var-encoding, so naming it "FlowchartSavingTests" was a bit misleading
As jsons don't work with all var types, I refactored the encoders to be more flexible about whether their values get encoded as json strings or regular strings
Renamed a couple flowcharts to reduce confusion, too
I can't save the state of the Say Commands' execution counts when they're inaccessible from outside code, so I changed it into an auto-property
This definitely matters for the Menu Command's HideIfVisited field, since that there depends on block execution counts
Hide If Visited procs based on the blocks' execution counts, and as making sure that is saved should depend on the Block-specific unit tests...
What with how different Commands' states may need to be saved in different ways.
Before, it didn't properly check the block state before deciding whether the test was successful. And to make sure that the test passed, I had to alter the Block and Say Savers so they... well, properly had the command indexes saved.
Since the stuff for saving the state of say commands should be in CommandSavingTests. And that stuff for checking the execution counts of blocks should be in BlockSavingTests
Since they can potentially hold thousands of bytes (the GameSaveData and UISaveData most likely will), which is too much for a struct
Any client code can also set the typenames in case there's any need to differentiate instances beyond just their class, like with VariableSaveUnit
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
It's a refactor.
I've noticed that the source for the new SaveSlotController class has yet to be fully adapted to the new SlotView system. I figured I might as well take a crack at making it so, even writing unit tests to make sure things are as intended. At the time of this writing, I've only managed to confirm that the slots do indeed have their base views registered as intended.
Later on, I'd like to make changes such as removing the old nameText, descText, and timeStampText fields; the new view system renders them unnecessary.
I hope to later on also help clean up other UI-related parts of the save system, though that's for another time.
Important Notes