eternagame / EternaJS

Eterna game/RNA design interface
Other
12 stars 10 forks source link

We should get rid of UndoBlock's _paramsArray #499

Open everyday847 opened 4 years ago

everyday847 commented 4 years ago

Leaving aside the larger question of how caching should be handled, I can say for sure this particular architecture isn't the right way to go. In the status quo, we have a _paramsArray that holds onto very disparate data. For each parameter, it holds two lists, depending on whether we are permitting pseudoknots in folding. It's not exactly clear whether this is necessary -- it's sort of futureproofing -- but there are (bad, perhaps architectural code smell) reasons why it makes sense.

Point is, there is zero benefit to having the _paramsArray rather than separately tracking a number of different values across these states. I am very confident about this. The reason is that the _paramsArray has to be typed very generously: it used to be any and I was able to narrow it to number | number[] | null. This is pretty horrible! You may agree that there is no relationship between "the number of GC pairs" and "A BITMAP OF THE DOTPLOT" -- why are they the same sum type

It also requires a ton of brutal type casts. If you set NUM_GC to a number[], it would just forgive you on the way back out. Terrible.

This breaks save games, probably. I don't like that, but...

luxaritas commented 4 years ago

So, a few thoughts here:

everyday847 commented 4 years ago

Yeah, I think I agree with you on all fronts. My one question is how to elegantly implement an EternaSaveGameManager -- that doesn't just turn into an enormous branching demon (i.e., read version (or absence of version for pre-ESGM saves), then branch off in one of 8 directions, then 9 directions, then...) -- maybe this does not matter long term. Certainly, we should build the ESGM with support only for 'version-free' saves first and confirm nothing breaks.

(It may just be that we want to support effectively one year's worth of save versions at all times.)

luxaritas commented 4 years ago

You could build this in such a way that it "upgrades" a structure from one version to another, maybe? Seems at least somewhat cleaner, with one handler per version

everyday847 commented 4 years ago

Maybe. But let's say we start writing out baz in version 9. Now we have to read in a null baz for version 8 saves, meaning that we need a null baz handler elsewhere -- sort of square one. It might be a bit neater, but far from all the way, probably.

As far as I've ever seen, data versioning's just miserable no matter what you do.